Repository navigation
serve-instrument exits when an instrument directory is deleted or a request URL has a malformed percent-escape #1804
Description
serve-instrument exits with an unhandled error when an instrument directory is deleted or renamed while it runs, or when it receives a URL with a malformed percent-escape. In both cases an exception escapes an async function that is called with void: this.handler.handle(...) for requests, and this.updateEncodedBundle() in the fs.watch callback. Node turns the unhandled rejection into a process exit. In updateEncodedBundle, only the bundle() call is inside the try; the readdir/readFile before it are not. In --all mode, deleting or renaming one instrument folder kills the server for all the others, and so does a git checkout that removes one. An author has to notice that the terminal died and restart it.
Where
packages/serve-instrument/src/server.tsx:45-48, 62-79 and 243-247, 312:
this.watcher = fs.watch(target, { recursive: false }, () => {
// ...
void this.updateEncodedBundle();
});
// ...
private async updateEncodedBundle(): Promise<void> {
const inputs: BundlerInput[] = [];
const filepaths = await fs.promises
.readdir(this.target, 'utf-8')
// ...
for (const filepath of filepaths) {
const loader = inferLoader(filepath);
const content: string | Uint8Array = await fs.promises.readFile(filepath, /* ... */);
// ...
}
const start = Date.now();
try {
// ...
const decodedName = decodeURIComponent(name!);
// ...
this.server = http.createServer((...args) => void this.handler.handle(...args));Reproduce
- Build it (
pnpm --filter @opendatacapture/serve-instrument build) and runnode dist/cli.js --all <dir>on a directory containingforms/FOO/index.tsandforms/BAR/index.ts. - Open
/forms/BAR, thenrm -rf <dir>/forms/BAR.
Actual:↺ [forms/BAR] File changed, rebuilding..., thenError: ENOENT: no such file or directory, scandir '…/forms/BAR'and the process exits (verified onmain)./forms/FOOis no longer served. - Restart, then
curl 'http://localhost:3000/forms/%E0%A4%A'.
Actual:URIError: URI malformedatdecodeURIComponentand the process exits (verified onmain).
Expected: the server logs the failure and keeps running. A deleted instrument answers 404 and a malformed URL answers 400.
Tests
packages/serve-instrument/src/__tests__/server.test.ts:
it('should keep serving other instruments after an instrument directory is deleted, so a rebuild failure does not kill the server'): fire the captured watch callback after removing the temp directory, and assert that the remaining instrument still returns 200.it('should answer 400 to a malformed percent-escape in an instrument URL rather than crashing')
No e2e test applies: serve-instrument is not part of the stack under testing/.
Suggested fix
Move the directory listing and file reads inside the existing try in updateEncodedBundle. Wrap handle in Server's request callback with a .catch that logs the error and ends the response with 500 if headers are not yet sent. Map a URIError from decodeURIComponent to 400.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). Neitherserver.tsxnorserver.test.tshas changed since this was filed, and no open PR touches them.fs.watchandhttp.createServerappear nowhere else in the repo, so this file is the whole fix.The suggested fix does not give the expected 404
Moving the reads inside the
trystops the exit, but thecatchnever clearsencodedBundle(server.tsx:84-89).getEncodedBundleonly rebuilds when it is null (:55-60), so after step 2/forms/BARkeeps returning 200 with the old bundle. Setthis.encodedBundle = nullin thecatch. A deleted instrument then answers 404 in--allmode and 503 in single mode.This also changes what a broken edit does: the page errors instead of serving the last good bundle. That is what an instrument that never compiled already gets (
server.test.ts:254-261,:385-393). There is no live reload, so an author who refreshes should see the failure, not their previous version.Two more gaps
EPERMerror instead of a change (Nodefs.watchcaveats). TheFSWatcheremits it as'error', andInstrumentLoaderhas no'error'listener (:45-48), so the process still exits there after the fix. Addthis.watcher.on('error', ...)that logs and setsencodedBundle = null. It does not need to callclose(): Node closes the handle before it emits'error'(watchers.js).res.writeHead(200)beforeawait this.renderPage(...)(:163-164,:238-239,:259-260), andres.headersSentis true as soon aswriteHeadreturns. A.catchthat sends 500 only when headers are not sent skips arenderPagefailure, such as a missingdist/client.js, and the request hangs. Render first, then callwriteHeadandend. Everyawaitin bothhandlemethods then runs beforewriteHead, so the.catchcan always send 500 and needs noheadersSentbranch that no test could reach.Catch the
URIErrorwhere it is thrown, at:246, and answer 400 there. Do not match onURIErrorin the generic.catch.Tests
In
server.test.ts:node:fsmock (:18-29) discards the watch target and returns{ close }. Record the target with each callback, so a test firesforms/BAR's callback instead of relying onreaddirorder. Return anEventEmitterwithclose, or the new.on('error', ...)throws.forms/BAR, fire its callback andvi.waitFortheFailed to compile forms/BARlog. Then assert FOO answers 200 and BAR answers 404. The 404 is what catches the stale bundle./forms/%E0%A4%Aanswers 400 and/still answers 200.fetchsends that path unchanged.client.jsread reject once. Assert a 500, a logged error, and that the next request is served.'error'on its mocked watcher, and assert the error is logged and the next request callsbundleagain..agents/docs/architecture/testing-strategy.md:53describes this mock wrongly. It says the mock is intest/and is never closed byServer.stop(), but the file issrc/__tests__/server.test.tsandServer.stop()closes every watcher (:362-363). #1660 rewrites that table and keeps the old text, so fix it there.Out of scope:
InstrumentLoaderMap.initbuilds the instrument list and loaders once (:120-138). The index keeps linking to a deleted instrument, and one added after startup is not served until a restart.Triage note written with Claude Code. Code references point at
main@d86740e.