Stop a broken stdin pipe from terminating the server - #212
Merged
Merged
Conversation
runProcessWithInputTimeout writes the caller's payload to the child's stdin and never awaits the result. Bun's FileSink returns number | Promise<number>, and returns a promise whenever the payload cannot be flushed synchronously. A child can exit before it reads stdin. git check-ignore --stdin outside a repository exits 128 immediately, which is the path taken by file.list on any non-repository directory. The pending flush then rejects with EPIPE. Nothing holds that promise, so it surfaces as an unhandled rejection and terminates the whole server process, dropping every connected browser session at once. The existing try/catch in the caller cannot help: it wraps the awaited result, while the rejection comes from a promise the caller never receives. Wrapping the write in a local try/catch does not help either, for the same reason. Absorb the broken pipe where it originates instead. The child's exit code and stderr still describe the failure, so the callers keep behaving as before. Observed in production on 0.7.4: 52 such exits over five days, up to 23 in a single day, each one restarting the service and disconnecting every client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
crandrosoff
marked this pull request as draft
September 18, 2026 19:51
crandrosoff
marked this pull request as ready for review
September 18, 2026 20:02
crandrosoff
marked this pull request as draft
September 18, 2026 22:46
powerfooI
marked this pull request as ready for review
September 19, 2026 00:49
Owner
|
Thanks for tracking this down and providing the fix and reproduction details! I’ll take it from here and handle the remaining checks and merge. |
Contributor
Author
|
No problem! Keep up the good work. Love the tool! |
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.
The bug
An unhandled
EPIPEterminates the whole server process. Every connected browsersession drops at once. Under systemd the service restarts about two seconds later,
so it reads as a flaky connection rather than a crash, and it is invisible unless
you read the journal.
Measured on one 0.7.4 host over five days: 52 such exits, up to 23 in a single
day.
It is not historical. It recurred on v0.7.5 at 2026-09-18 14:38:09 MDT on the same
host, while this pull request was open:
Two caller frames are byte-identical to the v0.7.4 trace above,
498:5507and498:7824. Mappingzoagainst the shipped v0.7.5 binary lands on the sameBun.spawn(..., stdin: "pipe")followed byr.stdin.write(t). Same defect, same path.Mapping those offsets against the published
roamgate-linux-x64v0.7.4 binary givesthe exact chain:
jorunProcessWithInputTimeout(server/src/workspace/process.ts)_gcollectIgnoredNames(server/src/workspace/git-ignore.ts)flistWorkspaceFiles(server/src/workspace/files.ts)qBfile.listRoot cause
runProcessWithInputTimeoutwrites the payload to the child's stdin and neverawaits the result:
Bun's
FileSink.write()and.end()both returnnumber | Promise<number>, andreturn a promise whenever the payload cannot be flushed synchronously.
git check-ignore --stdinoutside a work tree exits128immediately and neverreads stdin.
collectIgnoredNameshas already handled that exit code, but thepayload is still in flight. The pending flush rejects with
EPIPE, nothing holdsthat promise, and Bun terminates the process.
Why it fires in practice
collectIgnoredNamesruns onlist.entries, which is already capped atLIST_LIMIT = 1000. Payload size therefore depends on entry-name length, and thefailure is a race between the flush and the child's exit. Measured, 40 runs each:
That matches the production frequency: roughly one crash an hour on a host that
browses many non-repository directories, not one per listing.
End-to-end verification
Not just a unit test. Both binaries were compiled with
bun run buildon Bun 1.4.1,started as real servers against a real Herdr 0.9.1, and driven through the real
websocket bridge with a genuine
file.listRPC on a non-repository directory of1100 files.
main(unfixed)The unfixed server printed exactly the production error before exiting:
Note that the RPC itself succeeds in both cases. The request returns a correct
listing and then the process dies, which is why this looks like a flaky connection
rather than a failing request.
Why the two obvious fixes do not work
Both were measured, not assumed.
try/catchcannot catch it.collectIgnoredNamesalready wraps its
awaitintry { ... } catch { return new Set(); }. That wrapsthe awaited result, while the rejection comes from a promise the caller never
receives. In the reproduction the caller sees a clean
code: 128and theprocess still dies.
try/catchdoes not work either, for the samereason. Measured: still
escaped=EPIPE.Only attaching a handler to the returned value suppresses it.
The fix
Absorb the broken pipe where it originates. The child's exit code and stderr still
describe the failure, so every caller keeps behaving exactly as before, including the
existing
code === 128branch incollectIgnoredNames.Validation
Run with Bun 1.4.1, matching CI.
bun run format:checkbun run lintbun run typecheckbun run build:sitemainprocess.ts:42bun testmainThe suite is not fully green in my environment either before or after: a handful of
SSH, port-binding and process-control tests fail for environmental reasons. I compared
failing test names before and after, and the set after this change is a strict
subset of the baseline. No test fails that did not already fail.
A separate finding, deliberately not fixed here
On the same host, v0.7.5 logs a large volume of
terminal.link.resolveerrors thatv0.7.4 did not: 1708 in 76 minutes, against 0 in the preceding 24 hours. 1050 of
them are
this endpoint is still processing another command. Over the same window theclosing slow websocketbackpressure disconnects ran at a higher rate than before theupgrade. The sample is short and I am not claiming a regression, only reporting it.
Happy to open a separate issue with the full numbers if that is useful.
Update: the backpressure half of that observation is now #213, against the same
c0014ce Release 0.7.5base. It replaces the 8 MB disconnect with per-terminal framecoalescing. Neither branch contains the other and they merge cleanly. The
terminal.link.resolveerror volume is untouched by both and remains unexplained.🤖 Generated with Claude Code