| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Three of these share a shape worth naming: a throw inside an event-emitter callback is an uncaught exception, not a failed operation. The surrounding try/catch or promise looks like it covers the code, and doesn't. - The websocket envelope was JSON.parsed and zod-parsed inline in a 'message' listener, so one malformed frame ended the process. Per-event payloads were already parsed safely; only the envelope wasn't. - Websocket handlers are async and throw freely — findSession throws for an unknown id, which a reconnecting client with a stale session triggers on its own. They were called unawaited, so that was an unhandled rejection, and Node exits on those by default. - tsserver's stdout parser throws on framing it doesn't recognise, inside a 'data' listener. Now logged, with the buffer dropped; tsserver output is a best-effort feature, not worth ending the process over. Also: - Running a cell twice before the first exits used to corrupt the process registry, because the first process's exit handler deleted the key now owned by the second. The handler now only clears the key if it still holds the same process. What double-exec *should* do is left open in task 0005 — the registry no longer corrupts itself, but the first process is orphaned rather than killed, and that's a product question. - tsserver request promises never settled if the server died or never replied, hanging the caller and leaking the resolver. They now time out at 10s and reject on process exit. 10s because these are interactive requests — an answer that late is no more useful than none; what matters is that they settle. 24 new tests. ws-client.mts had no coverage at all despite being the dispatch path for every notebook operation, so this covers topic matching and wildcard extraction, subscribe/broadcast/unsubscribe, and each crash case. Suite is 62 tests, up from 11 at the start of the stack.
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
packages/api/ server/ ws-client.mts +38/-3 guard envelope parse, catch handler rejections tsserver/ tsserver.mts +58/-15 request timeouts, reject on exit, guard stdout parse processes.mts +12/-1 only delete the key you still own; add has() test/ ws-client.test.mts +232/-0 16 tests — this file had zero coverage processes.test.mts +120/-0 8 tests incl. the double-exec regression tasks/0003-...md +24/-2 close tasks/0005-...md +15/-1 partial close, record what's left open .changeset/ +18/-0Stacked on #508. Closes task 0003, partially closes task 0005.
Last PR in the stack. Several inputs could take the whole server down, which matters more than each one's individual severity — the server holds notebook state that hasn't been flushed.
The shape worth naming
Three of these are the same mistake: a throw inside an event-emitter callback is an uncaught exception, not a failed operation. The surrounding try/catch or promise looks like it covers the code, and doesn't.
The websocket case is the sharpest: per-event payloads were already safeParsed, but the envelope was JSON.parsed and zod.parsed inline. So the careful validation was one line below the unguarded one.
Also fixed
Unhandled handler rejections. Websocket handlers are async and throw freely — findSession throws for an unknown id, which a reconnecting client with a stale session triggers by itself. They were invoked unawaited, so that was an unhandled rejection, and Node exits on those by default.
Process registry clobbering. Running a cell twice before the first exits replaced the map entry, and then the first process's exit handler deleted the key now owned by the second — leaving a process running that cell:stop insisted didn't exist. The handler now only clears the key if it still holds the same process reference.
tsserver promises that never settled. A resolver was stored by seq with no timeout and no reject path, so if tsserver died or never answered, hover/completions/go-to-definition hung forever and the resolver map leaked. Now a 10s timeout plus rejection on process exit. 10s because these are interactive — an answer that late is no more useful than none; what matters is that they settle at all.
Tests
24 new. ws-client.mts had no coverage at all despite being the dispatch path for every notebook operation, so this adds topic matching and wildcard extraction, subscribe/broadcast/unsubscribe, and each crash case. Suite is 62, up from 11 at the start of the stack.
The handler-error tests silence console.error — they deliberately exercise the logging path, and a passing run shouldn't print stack traces that look like failures.
Left open on purpose
What double-exec should do. The registry no longer corrupts itself, but the first process is orphaned rather than killed or refused. That's a product question — replace, refuse, queue? — and it's coupled to the execution-model decision in task 0006, since a stateful kernel would change the answer entirely.
Whether stop actually kills TypeScript cells. SIGTERM goes to the tsx wrapper, not the process group. If tsx forks a child node process, stop is only cosmetic for TypeScript cells. Needs a running notebook and a long-running cell to check; noted in task 0005 rather than quietly closed.
A rejected tsserver request still leaves the client hanging, because responses are broadcasts with no request correlation. Task 0009.
No top-level unhandledRejection handler: with the handler-level catch there's no known path to one, and a blanket handler would hide the next one rather than surface it.
🤖 Generated with Claude Code