Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 47 additions & 22 deletions server/modules/providers/services/gjc-session-watcher.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,30 @@ const FAILURE_MESSAGE = 'GJC session watcher failed.';
const CALLBACK_FAILURE_MESSAGE = 'GJC session watcher callback failed.';
const STDERR_MESSAGE = 'GJC session watcher emitted diagnostics.';

/**
* Fixed failure vocabulary. Every member is a compile-time constant that carries
* no transcript path, frame content, or callback detail, so a reason may be
* logged without breaking the invariant that watcher diagnostics never expose
* session data. Without it a restart loop reports thousands of identical,
* unactionable lines.
*/
export type GjcSessionWatcherFailureReason =
| 'spawn-failed'
| 'ready-timeout'
| 'stdin-error'
| 'child-error'
| 'child-exit'
| 'oversized-frame'
| 'invalid-utf8'
| 'malformed-json'
| 'protocol-violation'
| 'queue-overflow';

/** Exit status is numeric/signal-name only, so it is safe to attach to a reason. */
function exitDetail(code: unknown, signal: unknown): string {
return `code=${typeof code === 'number' ? code : 'none'} signal=${typeof signal === 'string' ? signal : 'none'}`;
}

export type GjcSessionWatchEvent = {
kind: 'add' | 'change';
path: string;
Expand Down Expand Up @@ -135,21 +159,21 @@ export class GjcSessionWatcher {
this.child = child;
child.stdout.on('data', (chunk) => this.onStdout(chunk));
child.stderr?.on('data', () => this.diagnose(STDERR_MESSAGE));
child.stdin.on?.('error', () => this.fail());
child.on('error', () => this.fail());
child.on('exit', () => this.onExit());
child.on('close', () => this.onExit());
child.stdin.on?.('error', () => this.fail('stdin-error'));
child.on('error', () => this.fail('child-error'));
child.on('exit', (code, signal) => this.onExit(code, signal));
child.on('close', (code, signal) => this.onExit(code, signal));
} catch {
this.fail();
this.fail('spawn-failed');
}
return this.starting;
}

private async waitForReady(): Promise<void> {
await Promise.race([this.started.promise, timeout(this.options.readyTimeoutMs).then(() => {
if (!this.ready) {
this.fail();
throw new Error(FAILURE_MESSAGE);
this.fail('ready-timeout');
throw new Error(FAILURE_MESSAGE, { cause: 'ready-timeout' });
}
})]);
}
Expand All @@ -163,36 +187,36 @@ export class GjcSessionWatcher {
const frame = this.input.subarray(0, newline);
this.input = this.input.subarray(newline + 1);
if (frame.length > MAX_FRAME_BYTES) {
this.fail();
this.fail('oversized-frame');
return;
}
let text: string;
try {
text = new TextDecoder('utf-8', { fatal: true }).decode(frame).replace(/\r$/u, '');
} catch {
this.fail();
this.fail('invalid-utf8');
return;
}
this.decode(text);
if (this.failed) return;
}
if (this.input.length > MAX_FRAME_BYTES) this.fail();
if (this.input.length > MAX_FRAME_BYTES) this.fail('oversized-frame');
}

private decode(text: string): void {
let frame: unknown;
try {
frame = JSON.parse(text);
} catch {
this.fail();
this.fail('malformed-json');
return;
}
if (frame === null || typeof frame !== 'object' || Array.isArray(frame)) return this.fail();
if (frame === null || typeof frame !== 'object' || Array.isArray(frame)) return this.fail('protocol-violation');
const record = frame as Record<string, unknown>;
const keys = Object.keys(record);
if (record.protocolVersion !== 1 || typeof record.kind !== 'string') return this.fail();
if (record.protocolVersion !== 1 || typeof record.kind !== 'string') return this.fail('protocol-violation');
if (record.kind === 'ready') {
if (this.ready || keys.length !== 2 || !keys.includes('protocolVersion') || !keys.includes('kind')) return this.fail();
if (this.ready || keys.length !== 2 || !keys.includes('protocolVersion') || !keys.includes('kind')) return this.fail('protocol-violation');
this.ready = true;
this.started.resolve();
return;
Expand All @@ -210,9 +234,9 @@ export class GjcSessionWatcher {
record.path.length === 0 ||
record.path.includes('\0')
) {
return this.fail();
return this.fail('protocol-violation');
}
if (!this.pending.has(record.path) && this.pending.size >= MAX_QUEUED_PATHS) return this.fail();
if (!this.pending.has(record.path) && this.pending.size >= MAX_QUEUED_PATHS) return this.fail('queue-overflow');
this.pending.set(record.path, { kind: record.event, path: record.path });
void this.drain();
}
Expand Down Expand Up @@ -241,28 +265,29 @@ export class GjcSessionWatcher {
safeCall(() => this.options.diagnostic(message));
}

private fail(): void {
private fail(reason: GjcSessionWatcherFailureReason, detail?: string): void {
if (this.failed || this.closed) return;
this.failed = true;
this.drainCancelled = true;
this.pending.clear();
this.drainAbort.abort();
this.drainDone.resolve();
this.started.reject(new Error(FAILURE_MESSAGE));
this.diagnose(FAILURE_MESSAGE);
safeCall(() => this.options.onFailure(new Error(FAILURE_MESSAGE)));
const cause = detail === undefined ? reason : `${reason} ${detail}`;
this.started.reject(new Error(FAILURE_MESSAGE, { cause }));
this.diagnose(`${FAILURE_MESSAGE} (${cause})`);
safeCall(() => this.options.onFailure(new Error(FAILURE_MESSAGE, { cause })));
try {
this.child?.kill('SIGKILL');
} catch {
// The process may already be gone.
}
}

private onExit(): void {
private onExit(code?: unknown, signal?: unknown): void {
if (this.exitedOnce) return;
this.exitedOnce = true;
this.exited.resolve();
if (!this.closed) this.fail();
if (!this.closed) this.fail('child-exit', exitDetail(code, signal));
}

async close(): Promise<void> {
Expand Down
18 changes: 14 additions & 4 deletions server/modules/providers/services/sessions-watcher.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ const gjcWatcherStartTasks = new Set<Promise<void>>();
const gjcWatcherStartAbortControllers = new Set<AbortController>();
let gjcWatcherRestartTimer: ReturnType<typeof setTimeout> | null = null;
let gjcWatcherRestartDelayMs = 1_000;
// Restarts are capped at GJC_WATCH_RESTART_MAX_MS, so a permanently broken watcher
// otherwise logs one indistinguishable line every 30s forever. The run length makes
// a stuck loop visible in the log without needing to count timestamps by hand.
let gjcWatcherConsecutiveFailures = 0;
let gjcWatcherGeneration = 0;
let sessionWatchersClosing = false;
const GJC_WATCH_RESTART_MAX_MS = 30_000;
Expand Down Expand Up @@ -435,12 +439,16 @@ async function runGjcSessionWatcherStart(
if (signal.aborted || sessionWatchersClosing || gjcWatcher || gjcWatcherStarting) return;
const generation = ++gjcWatcherGeneration;
let failureReported = false;
const reportFailure = (): void => {
const reportFailure = (error?: Error): void => {
if (failureReported || generation !== gjcWatcherGeneration || sessionWatchersClosing) return;
failureReported = true;
gjcWatcherConsecutiveFailures += 1;
controller.abort();
if (gjcWatcher === watcher) gjcWatcher = null;
console.error('GJC native session watcher failed.');
const reason = typeof error?.cause === 'string' ? error.cause : 'unreported';
console.error(
`GJC native session watcher failed. (${reason}; consecutive ${gjcWatcherConsecutiveFailures})`
);
void watcher.close()
.catch(() => {})
.finally(() => {
Expand Down Expand Up @@ -498,8 +506,9 @@ async function runGjcSessionWatcherStart(
return;
}
gjcWatcherRestartDelayMs = 1_000;
} catch {
reportFailure();
gjcWatcherConsecutiveFailures = 0;
} catch (error) {
reportFailure(error instanceof Error ? error : undefined);
await watcher.close();
}
}
Expand Down Expand Up @@ -680,6 +689,7 @@ export async function closeSessionsWatcher(): Promise<void> {
]);
watchers.length = 0;
gjcWatcherRestartDelayMs = 1_000;
gjcWatcherConsecutiveFailures = 0;
pendingWatcherUpdate = null;
pendingWatcherUpdateStartedAt = null;
watcherRefreshInFlight = false;
Expand Down
47 changes: 47 additions & 0 deletions server/modules/providers/tests/gjc-session-watcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,50 @@ test('close before readiness rejects the pending start without reporting a runti
await rejected;
assert.equal(failures.length, 0);
});

test('each failure mode reports its own reason code and nothing from the frame', async () => {
for (const [frame, reason] of [
['{"protocolVersion":1,"kind":"event","event":"add","path":"secret-path"}\n', 'protocol-violation'],
['{"protocolVersion":1,"kind":"unknown"}\n', 'protocol-violation'],
['secret-path is not json\n', 'malformed-json'],
[`${'x'.repeat(64 * 1024 + 1)}\n`, 'oversized-frame'],
] as const) {
const { watcher, child, failures } = setup();
const started = watcher.start();
child.output(frame);

await assert.rejects(started, /GJC session watcher failed\./u);
assert.equal(failures.length, 1);
assert.equal(failures[0].cause, reason, frame.slice(0, 24));
assert.doesNotMatch(failures[0].message, /secret-path/u);
}
});

test('ready timeout and child exit stay distinguishable, exit status included', async () => {
const timedOut = setup({ readyTimeoutMs: 1 });
await assert.rejects(timedOut.watcher.start(), /GJC session watcher failed\./u);
assert.equal(timedOut.failures[0].cause, 'ready-timeout');

const exited = setup();
await ready(exited.watcher, exited.child);
exited.child.emit('exit', 3, null);
exited.child.emit('close', 3, null);
assert.equal(exited.failures.length, 1);
assert.equal(exited.failures[0].cause, 'child-exit code=3 signal=none');

const signalled = setup();
await ready(signalled.watcher, signalled.child);
signalled.child.emit('exit', null, 'SIGKILL');
assert.equal(signalled.failures[0].cause, 'child-exit code=none signal=SIGKILL');
});

test('failure diagnostics name the reason so a restart loop is diagnosable from logs alone', async () => {
const diagnostics: string[] = [];
const { watcher, child } = setup({ diagnostic: (message) => diagnostics.push(message) });
const started = watcher.start();
child.output('{"protocolVersion":1,"kind":"event","event":"add","path":"secret-path"}\n');

await assert.rejects(started, /GJC session watcher failed\./u);
assert.deepEqual(diagnostics, ['GJC session watcher failed. (protocol-violation)']);
assert.doesNotMatch(diagnostics.join(' '), /secret-path/u);
});