recorder: don't drop fixtures when SDK closes socket after data: [DONE]#288
Open
linyijie wants to merge 1 commit into
Open
recorder: don't drop fixtures when SDK closes socket after data: [DONE]#288linyijie wants to merge 1 commit into
data: [DONE]#288linyijie wants to merge 1 commit into
Conversation
…NE]`
SDKs like the OpenAI Python SDK close the response socket the moment they consume `data: [DONE]`, before upstream fires its `end` event. The current clientRes.on("close") handler treats that as a mid-stream disconnect, destroys the upstream request, and the downstream `if (clientDisconnected) return "relayed"` branch skips fixture persistence entirely — so record mode silently produces no fixture for any OpenAI-SDK-driven traffic even though the full response was received.
Fix: stop destroying upstream on client close (the existing !clientDisconnected guard in onUpstreamData already prevents writes to the closed peer, so nothing is written to a dead socket), and remove the early return in the clientDisconnected branch — reaching that branch now means upstream ran to completion cleanly, so persisting the fixture is safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In record mode,
clientRes.on("close")currently callsreq.destroy()on the upstream request whenever the client closes its socket beforeclientRes.writableFinished, and downstream logic then skips saving the fixture entirely.That's a reasonable guard against saving a truncated body when the client really did die mid-stream, but it also fires in a common benign case: SDKs that eagerly close their HTTP response the instant they consume the terminating SSE frame. The OpenAI Python SDK does exactly this — its
Stream.__stream__breaks out of the iterator loop ondata: [DONE]and itsfinally: await response.aclose()closes the socket immediately, often before upstream has fired its ownendevent.Result: record mode silently produces zero fixtures for any OpenAI-SDK-driven traffic, even though the full response was received and rendered end-to-end by the caller. This is very easy to hit and hard to diagnose (the only signal in the logs is
Proxy request failed: aborted, which looks like an upstream problem).Fix
Two small changes in
src/recorder.ts:In the
clientRes.on("close")handler for progressive-stream responses: stop destroying the upstream request and stop discarding the already-buffered chunks. Just record that the client disconnected. Upstream is allowed to run to completion so the fixture body is complete. The existing!clientDisconnectedguard inonUpstreamDataalready prevents any further writes to the closed client socket, so no data is written to a dead peer.In the post-collapse
if (clientDisconnected)branch: remove thereturn "relayed"— since we no longer destroy upstream on client close, reaching that point means upstream did complete cleanly and the buffered body is intact, so persisting the fixture is safe. The warn message is kept (retitled) so the disconnect is still observable.Repro (before fix)
Start aimock in record mode against any OpenAI-compatible upstream:
Then drive it with the OpenAI Python SDK:
Observed logs:
No fixture is written. The SDK, however, receives the full response — the abort is aimock destroying upstream in reaction to the SDK's normal post-
[DONE]socket close.After fix
Same repro produces:
Verified locally against a live OpenAI-compatible gateway with the OpenAI Python SDK 2.30.0 + httpx 0.28.1; 714 SSE chunks streamed to the SDK, fixture written to disk, replay works.
Safety notes
res.on("error")) or upstream timeouts (req.on("timeout")→req.destroy(Error(...))), andmakeUpstreamRequestrejects with that error — control never reaches theclientDisconnectedbranch, and no fixture is written.maxProxyBufferBytes/maxProxyBufferFramescaps.