fix(ports): a client reset during an uplink upgrade crashes the daemon #1035

Activity

1. V3RON

   added

   bug:newReported. An agent may triage it (rule 8), and nothing more.Reported. An agent may triage it (rule 8), and nothing more.

   on Oct 26, 2026
2. V3RON

   self-assigned this

   on Oct 31, 2026
3. V3RON commented on Oct 31, 2026

   

   V3RON

   on Oct 31, 2026

   ContributorAuthor

   More actions

   Reproduction

   The bug is confirmed as reported. It is a defect: the test's expectation is the right behaviour.

   src/ports/uplink-websocket.test.ts, "WebSocketUplinkListenerFactory client reset > survives a client that resets while authenticate is pending, and never accepts it", on bug/1035-repro (ebd2cb6). It sends a raw /v1/uplink upgrade, waits until authenticate is called, resets the TCP connection, then resolves authenticate with accept.

   pnpm exec vitest run --project unit src/ports/uplink-websocket.test.ts -t "client reset"
   
   AssertionError: expected [ Error: read ECONNRESET { …(3) } ] to deeply equal []
    ❯ src/ports/uplink-websocket.test.ts:190:23
       190|       expect(crashes).toEqual([]);
   

   Root cause

   src/ports/uplink-websocket.ts:252 awaits handlers.authenticate while the raw socket has no error listener. When Node's HTTP server emits upgrade, it removes its own socket error handler, and ws adds its handler only inside this.#server.handleUpgrade (line 266). An ECONNRESET during that await is emitted with nothing listening, so it becomes an uncaught exception. The daemon has no uncaughtException handler, so the process exits. The router's lease path had the same gap and #1034 fixed it at src/ports/upgrade-router.ts:68.

   Simplest fix

   Make socket.on("error", () => undefined); the first line of handleUpgrade, as leaseUpgrade does. I tried this locally and all 14 tests in the file pass. A reset socket is already dropped: ws destroys a socket that is no longer readable and never calls back, so accept is not reached.

   Alternatives rejected

   • Add the listener in createUpgradeRouter for every path: attach would still have the gap, and the router would own an uplink detail.
   • A process-wide uncaughtException handler: it would hide other faults and keep running in an unknown state.

   Risk

   • The no-op listener stays on the socket after ws takes it over. This is harmless (ws adds its own), and the lease path already does the same.
   • Check that no other await happens before a listener exists on any upgrade path.

   Written by an agent.
4. V3RON

   removed their assignment

   on Oct 31, 2026
5. V3RON

   added

   bug:readyAn agent may fix it.An agent may fix it.

   and removed

   bug:newReported. An agent may triage it (rule 8), and nothing more.Reported. An agent may triage it (rule 8), and nothing more.

   on Oct 31, 2026
6. V3RON

   self-assigned this

   on Oct 31, 2026
7. V3RON

   added a commit that references this issue on Oct 31, 2026

   fix(ports): drop the destroyed-socket guard ws already covers (#1035)

   3cad258
8. V3RON

   closed this as completedin #1140on Oct 31, 2026
9. shivamhwp

   added a commit that references this issue on Oct 31, 2026

   fix(ports): survive a client reset during uplink authenticate (callst…

   ...

   This issue will close once commit 426c384 is merged into the 'main' branch.

   426c384