fix: flush tracer on WS responses final chunk#3017
fix: flush tracer on WS responses final chunk#3017thiscantbeserious wants to merge 1 commit intomaximhq:mainfrom
Conversation
The postHookRunner closure built in RunStreamPreHooks called drainAndAttachPluginLogs on the final chunk but never called tracer.CompleteAndFlushTrace. The logging plugin accumulates completed entries in pendingLogsToInject keyed by traceID; that map is only drained to the store when CompleteAndFlushTrace runs. As a result every native-WS response left its log entry stuck in memory and no row was written to the logs table. All early-exit paths in the same function and the sibling RunRealtimeTurnPreHooks already call CompleteAndFlushTrace correctly. This adds the missing call in the happy-path branch, mirroring the existing pattern, and adds two regression tests. Closes maximhq#2999
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Superseded by #3018 (merged 2026-04-24), which bundles the fix for this issue and several other native WS reliability bugs. Closing this draft as redundant. |
Summary
For native WebSocket streaming, the
postHookRunnerclosure returned byRunStreamPreHookscalleddrainAndAttachPluginLogson the final chunk but never calledtracer.CompleteAndFlushTrace. The logging plugin accumulates completed entries in an in-memorypendingLogsToInjectmap keyed by trace ID. That map is only drained to the store whenCompleteAndFlushTraceruns. Every native-WS response therefore left its log entry stuck in memory and no row was written to thelogstable.Root cause: a single missing
CompleteAndFlushTracecall in the happy-path branch ofRunStreamPreHooks. All early-exit paths in the same function and the siblingRunRealtimeTurnPreHooksalready call it correctly. The HTTP streaming path also flushes correctly.Changes
core/bifrost.go: addtracer.CompleteAndFlushTrace(traceID)inside thepostHookRunnerclosure whenIsFinalChunk(ctx)is true, afterdrainAndAttachPluginLogs. Mirrors the pattern used on every other exit path in the same function.core/bifrost_test.go: addTestRunStreamPreHooks_PostHookRunner_FlushesTracerOnFinalChunkandTestRunStreamPreHooks_PostHookRunner_NoFlushOnNonFinalChunkusing a recording tracer to assert the call happens exactly once on the final chunk and not on mid-stream chunks.Type of change
Affected areas
How to test
go build ./core/... ./transports/... go test ./core/... -run TestRunStreamPreHooks go vet ./core/...Expected: both new tests pass, build and vet are clean.
End-to-end: start Bifrost, route a WebSocket Responses request through a mock upstream that sends a terminal
response.completedframe, then check thelogstable. Before this fix the row was never written. After this fix a row appears.Screenshots/Recordings
N/A
Breaking changes
Related issues
Closes #2999
This bug was discovered while working on PR #2775 (OpenAI OAuth passthrough). The PR branch contains the fix that is being extracted here.
Security considerations
None. This change only adds a missing trace flush call.
Checklist
docs/contributing/README.mdand followed the guidelines