fix(watch): a watch callback that throws no longer ends the host's process (#3092)

A program that uses CodeGraph as a library passes callbacks to watch().
FileWatcher.flush() runs from a timer with no caller to catch anything, and it
called onSyncError and onDegraded (through degrade()) in its catch block. A
throw from either left flush() as an unhandled rejection, which ends the host
process with exit 1. A throw from onDegraded also skipped the stop() that
degrade() owes, so a watcher reporting itself degraded kept retrying. A throw
from onSyncComplete sat inside the try, so it counted a sync that worked as a
failed one and called onSyncError with an error that was no sync failure.

All three now go through one guard, callHost(), which logs the throw as a
warning and changes nothing else. The CLI and MCP server are not affected:
their callbacks only write to stderr. Only a host's own callback can reach this.

Validation:
- Three new tests in watcher.test.ts fail on main and pass here (64/64 in the
  file). Putting each guard back one at a time fails exactly its own test.
- A host script on the public API (watch() with a throwing onSyncComplete and
  onSyncError, then a real file edit): main exits 1, this branch logs the throw
  and keeps running, exit 0.

PR #3074 listed ten lint findings; this keeps the two watcher call sites'
purpose and leaves out the rest, which change no behaviour:
- flushOrdered(): flushChain's body catches everything, so it cannot reject.
- stop() from timers and signals: nothing stop() awaits can reject today, and
  a forced rejection already exits (exit 1, one fatal line).
- await on getCodeGraph and the upgrade functions: plain values, nothing else
  runs in that microtask.
- { cause } on the rebuild and daemon-lock errors: the rebuild message already
  carries the reason, and nothing reads or prints cause.

Reimplements PR #3074 by @johnhldiver-beep. Refs issue 2643.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-30 18:19:56 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 17d5426d63
commit 22cf7d3aa1
3 changed files with 100 additions and 3 deletions
+1
View File
@@ -78,6 +78,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- A program that uses CodeGraph as a library and indexes chosen files with `indexFiles()` now keeps other CodeGraph processes, like a `codegraph sync` run by a git hook, out of the index until those files are stored. Before, it let go of the write lock that keeps two writers apart as soon as indexing started, so another process could write to the index at the same time.
- When several CodeGraph processes start at the same moment after one of them crashed, like the MCP server, a git hook's `codegraph sync` and `codegraph index`, only one of them now takes over the write lock the crashed process left behind. Before, two of them could both take it, the second one deleting the first one's lock as it did, and both went on to write to the index at once. A write lock held by another user's CodeGraph, or on Windows by one running as administrator, is no longer mistaken for a crashed one's and taken over either. Thanks @inth3shadows for the report and @vedantb7 for the first fix. (#2979)
- A program that uses CodeGraph as a library and runs `indexAll()` again on a project it has already indexed now keeps every symbol findable by the words in its name, which the prompt hook and `codegraph_explore` use to match a question like "how does the order state machine work" to `OrderStateMachine`. Before, only the files changed since the last index kept their names' words, and a cancelled run lost them all; `codegraph index` was not affected. Thanks @rudycelekli.
- A program that uses CodeGraph as a library no longer crashes when a `watch()` callback it passed, `onSyncError` or `onDegraded`, throws: the error is logged, a failed sync still retries, and a watcher that gave up still stops. A throwing `onSyncComplete` no longer counts as a failed sync either. Thanks @johnhldiver-beep. (#2643)
- The shared background server is no longer killed with "Main thread unresponsive" while it brings another project's index up to date, such as one a session names with `projectPath`: only its own project's index counted as progress, so a long update of any other looked like a hang, and the server was stopped and restarted again and again. Every index the server keeps up to date now counts, and so does the one a session updates on its own when the background server is unavailable. Thanks @mradwankhalil for the report and the first fix. (#2404)
- A long-running session, like the background server behind your agent, now notices a framework that arrives or goes while it runs, such as Express in a new `package.json`, or one app replaced by another with as many files. Files it indexes from then on are read with the frameworks the project has now; before, they were read with the ones it had when the session started, so a new Express app's routes were missing until a full `codegraph index`. Files indexed before the change are not read again, so `codegraph status` now says which frameworks changed since the last full index and recommends running one, including when only `package.json` changed. Thanks @nk33-dev for the report and the first fix, and @inth3shadows for the `package.json` case. (#2516, #2519, #3007)
- On Windows, deleting the folder a link in your project points to, like a junction to a library kept outside the project, no longer makes CodeGraph's server use a full CPU core until it is restarted, or stop picking up your edits. The linked files now drop out of the index while the rest of the project keeps updating, and if the project folder itself disappears while watched, live updates turn off with a message that says so. Thanks @L0garithmic.
+81
View File
@@ -978,6 +978,87 @@ describe('FileWatcher', () => {
watcher.stop();
});
// These run from a timer, with no caller to catch a throw: out of flush() it
// is an unhandled rejection, which ends the host's process.
describe('one that throws', () => {
const unhandled: unknown[] = [];
const recordUnhandled = (reason: unknown) => { unhandled.push(reason); };
const hostBug = () => { throw new Error('host bug'); };
const warned = (spy: { mock: { calls: unknown[][] } }, text: string) =>
spy.mock.calls.some((c) => typeof c[0] === 'string' && c[0].includes(text));
beforeEach(() => {
unhandled.length = 0;
process.on('unhandledRejection', recordUnhandled);
});
afterEach(() => {
process.off('unhandledRejection', recordUnhandled);
});
it('leaves a failed sync to its retry when onSyncError throws', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const syncFn = vi.fn()
.mockRejectedValueOnce(new Error('disk full'))
.mockResolvedValue({ filesChanged: 1, durationMs: 1 });
const onSyncComplete = vi.fn();
const onSyncError = vi.fn(hostBug);
const watcher = newWatcher(syncFn, { debounceMs: 25, onSyncComplete, onSyncError });
watcher.start();
await watcher.waitUntilReady();
__emitWatchEventForTests(testDir, 'src/retry.ts');
await waitFor(() => onSyncComplete.mock.calls.length > 0, 8000, 10);
expect(onSyncError).toHaveBeenCalledTimes(1);
expect(syncFn).toHaveBeenCalledTimes(2);
expect(unhandled).toEqual([]);
expect(warned(warnSpy, 'onSyncError')).toBe(true);
});
it('still stops a degrading watcher when onDegraded throws', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError());
const onDegraded = vi.fn(hostBug);
const watcher = newWatcher(syncFn, { debounceMs: 25, onDegraded });
watcher.start();
await watcher.waitUntilReady();
__emitWatchEventForTests(testDir, 'src/long-lock.ts');
await waitFor(() => onDegraded.mock.calls.length > 0, 8000, 10);
await waitFor(() => !watcher.isActive(), 1000, 10);
const syncs = syncFn.mock.calls.length;
await new Promise((resolve) => setTimeout(resolve, 150));
expect(watcher.isDegraded()).toBe(true);
expect(syncFn).toHaveBeenCalledTimes(syncs); // stopped means no more retries
expect(onDegraded).toHaveBeenCalledTimes(1);
expect(unhandled).toEqual([]);
expect(warned(warnSpy, 'onDegraded')).toBe(true);
});
it('keeps a sync that worked a sync that worked when onSyncComplete throws', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const syncFn = vi.fn().mockResolvedValue({ filesChanged: 1, durationMs: 1 });
const onSyncComplete = vi.fn(hostBug);
const onSyncError = vi.fn();
const watcher = newWatcher(syncFn, { debounceMs: 25, onSyncComplete, onSyncError });
watcher.start();
await watcher.waitUntilReady();
__emitWatchEventForTests(testDir, 'src/ok.ts');
await waitFor(() => onSyncComplete.mock.calls.length > 0, 8000, 10);
await new Promise((resolve) => setTimeout(resolve, 100));
expect(onSyncError).not.toHaveBeenCalled();
expect(syncFn).toHaveBeenCalledTimes(1);
expect(watcher.isActive()).toBe(true);
expect(warned(warnSpy, 'onSyncComplete')).toBe(true);
});
});
});
describe('CodeGraph integration', () => {
+18 -3
View File
@@ -853,6 +853,21 @@ export class FileWatcher {
return this.ignoreMatcher.ignores(rel + '/');
}
/**
* Call one of the host's {@link WatchOptions} callbacks. They are diagnostics:
* one that throws is logged and changes nothing here. A sync runs from a timer,
* so a throw would have no caller and end the host's process as an unhandled
* rejection. It would also read as a failed sync, or skip the stop that a
* degrade owes.
*/
private callHost(name: string, call: () => void): void {
try {
call();
} catch (err) {
logWarn(`File watcher ${name} callback threw`, { error: err instanceof Error ? err.message : String(err) });
}
}
/**
* Disable live watching after a terminal runtime failure
* (watch-resource exhaustion, lock contention past the retry budget, or a
@@ -866,7 +881,7 @@ export class FileWatcher {
this.degradedByLock = byLock;
this.recoveringFromLock = false;
logWarn('File watcher disabled', { projectRoot: this.projectRoot, reason, ...context });
this.onDegraded?.(reason);
this.callHost('onDegraded', () => this.onDegraded?.(reason));
this.stop();
}
@@ -1114,7 +1129,7 @@ export class FileWatcher {
this.pendingFiles.delete(filePath);
}
}
this.onSyncComplete?.(result);
this.callHost('onSyncComplete', () => this.onSyncComplete?.(result));
} catch (err) {
if (!scoped) this.needsFullScan = true;
if (err instanceof LockUnavailableError) {
@@ -1145,7 +1160,7 @@ export class FileWatcher {
error: error.message,
retryCount: this.syncFailureRetryCount,
});
this.onSyncError?.(error);
this.callHost('onSyncError', () => this.onSyncError?.(error));
// A persistent (deterministic) sync failure — a broken extractor on a
// specific file, DB corruption, SQLITE_FULL, an OOM in resolution —
// would otherwise retry forever at the debounce cadence, spamming logs