Repository navigation
fix(dotnet): end the bridge socket on netcoredbg's close, not its exit (#878) - #892
Merged
Merged
Conversation
#878) Node fires `exit` when the process ends and `close` once its stdio streams have drained; the last DAP frames netcoredbg writes before exiting can still be in the stdout pipe at `exit`, and ending the proxy's socket there lost them (the #860 class). The bridge now ends the socket on `close`, drops proxy bytes aimed at a stdin with no process behind it, and logs an in-flight stdin error instead of dying on it — the rule the Dart bridge already carries. The `dotnet-utils` header still said no bridge process was needed; it now says what the bridge is for. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… bridge's close and socket errors (#878) Review of #892: Node destroys the child's stdin before it emits `exit`, so the `childEnded` flag is belt and braces — the stdin `error` listener is what keeps an in-flight EPIPE from crashing the bridge; the comment now says so. The bridge also writes one line when netcoredbg closes and one when the proxy's socket errors (both land in the proxy log through the adapter manager's sanitized stderr path), and a test pins that a child closing after the proxy has gone neither throws nor double-ends. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
Fixes #878.
packages/adapter-dotnet/src/utils/netcoredbg-bridge-core.tsended the proxy's socket on the child'sexitevent. Node firesexitwhen the process ends andcloseonce its stdio streams have drained, so anoutput/exited/terminatedframe netcoredbg wrote just before exiting could still be in the stdout pipe when the socket was closed — the same class as #860 ("last line before exit"). The Dart bridge (dap-stdio-bridge-core.ts) already carries the rule; this brings the netcoredbg bridge in line.Changes
close, never onexit(exitonly marks the child gone).errorlistener before.dotnet-utils.tsheader: replaced "netcoredbg natively supports DAP over TCP … No bridge process is needed" with what the bridge is for;packages/adapter-dotnet/docs/dotnet-adapter.mdno longer says the--serverbug is Windows-only (its own next paragraph said all platforms).changelog.d/878.fixed.md.Tests (
packages/adapter-dotnet/tests/unit/netcoredbg-bridge-core.test.ts, 9 → 12)keeps the socket open across netcoredbg exit until its streams close, then ends it (issue #878)— emitsexit, then a finalContent-Length: 2\r\n\r\n{}on stdout, thenclose; asserts the client received the frame. Red before the fix: the client received''(the frame was lost to the ended socket, which also raisedERR_STREAM_WRITE_AFTER_ENDinto the socket error path).stops forwarding socket bytes to netcoredbg stdin once it has exited(red before: the write went through).logs a netcoredbg stdin error instead of dying on it(red before: unhandlederroron stdin).cleans up on netcoredbg exitemitted onlyexitand would hang under the new rule; it now emitsexitthenclose(cleans up once netcoredbg has closed).Verification
pnpm exec vitest run --project unit packages/adapter-dotnet/tests— 170 passed.npm run typecheck:all(ratchet 0/0), eslint on the touched files,pnpm changelog:check,node scripts/check-docs.mjs— clean.Not observed live (the issue notes netcoredbg usually answers
disconnectand lingers); the unit test is the reproduction of the race.🤖 Generated with Claude Code