From 6fe4af2546b428ce65db72c061ef66eb858defc4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 4 May 2026 20:05:09 -0400 Subject: [PATCH] refactor: split subprocess timeout and failure error seams --- sdk/src/query-gsd-tools-runtime.ts | 8 +++--- sdk/src/query-subprocess-adapter.test.ts | 4 ++- sdk/src/query-subprocess-adapter.ts | 32 +++++++++++++----------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/sdk/src/query-gsd-tools-runtime.ts b/sdk/src/query-gsd-tools-runtime.ts index 07a9e9063..3ea879768 100644 --- a/sdk/src/query-gsd-tools-runtime.ts +++ b/sdk/src/query-gsd-tools-runtime.ts @@ -33,10 +33,10 @@ export function createGSDToolsRuntime(opts: { gsdToolsPath: opts.gsdToolsPath, timeoutMs: opts.timeoutMs, workstream: opts.workstream, - createToolsError: (message, command, args, exitCode, stderr, classification) => - classification?.kind === 'timeout' - ? GSDToolsError.timeout(message, command, args, stderr, classification.timeoutMs, { exitCode }) - : GSDToolsError.failure(message, command, args, exitCode, stderr), + createTimeoutError: (message, command, args, stderr, timeoutMs) => + GSDToolsError.timeout(message, command, args, stderr, timeoutMs), + createFailureError: (message, command, args, exitCode, stderr) => + GSDToolsError.failure(message, command, args, exitCode, stderr), }); const nativeDirectAdapter = new QueryNativeDirectAdapter({ diff --git a/sdk/src/query-subprocess-adapter.test.ts b/sdk/src/query-subprocess-adapter.test.ts index c2ab850c3..fae7c4a5b 100644 --- a/sdk/src/query-subprocess-adapter.test.ts +++ b/sdk/src/query-subprocess-adapter.test.ts @@ -41,7 +41,9 @@ describe('QuerySubprocessAdapter', () => { projectDir: dir, gsdToolsPath, timeoutMs: 2_000, - createToolsError: (message, command, args, exitCode, stderr) => + createTimeoutError: (message, command, args, stderr) => + new FakeToolsError(message, command, args, null, stderr) as never, + createFailureError: (message, command, args, exitCode, stderr) => new FakeToolsError(message, command, args, exitCode, stderr) as never, }); } diff --git a/sdk/src/query-subprocess-adapter.ts b/sdk/src/query-subprocess-adapter.ts index 474892dd7..ce940212d 100644 --- a/sdk/src/query-subprocess-adapter.ts +++ b/sdk/src/query-subprocess-adapter.ts @@ -1,20 +1,26 @@ import { execFile } from 'node:child_process'; import { readFile } from 'node:fs/promises'; import { timeoutMessage } from './query-failure-classification.js'; -import type { GSDToolsError, GSDToolsErrorClassification } from './gsd-tools-error.js'; +import type { GSDToolsError } from './gsd-tools-error.js'; export interface QuerySubprocessAdapterDeps { projectDir: string; gsdToolsPath: string; timeoutMs: number; workstream?: string; - createToolsError: ( + createTimeoutError: ( + message: string, + command: string, + args: string[], + stderr: string, + timeoutMs: number, + ) => GSDToolsError; + createFailureError: ( message: string, command: string, args: string[], exitCode: number | null, stderr: string, - classification?: GSDToolsErrorClassification, ) => GSDToolsError; } @@ -41,20 +47,19 @@ export class QuerySubprocessAdapter { if (error) { if (error.killed || (error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { reject( - this.deps.createToolsError( + this.deps.createTimeoutError( timeoutMessage(command, args, this.deps.timeoutMs), command, args, - null, stderrStr, - { kind: 'timeout', timeoutMs: this.deps.timeoutMs }, + this.deps.timeoutMs, ), ); return; } reject( - this.deps.createToolsError( + this.deps.createFailureError( `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, command, args, @@ -71,7 +76,7 @@ export class QuerySubprocessAdapter { resolve(parsed); } catch (parseErr) { reject( - this.deps.createToolsError( + this.deps.createFailureError( `Failed to parse gsd-tools output for "${command}": ${parseErr instanceof Error ? parseErr.message : String(parseErr)}\nRaw output: ${raw.slice(0, 500)}`, command, args, @@ -84,7 +89,7 @@ export class QuerySubprocessAdapter { ); child.on('error', (err) => { - reject(this.deps.createToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); + reject(this.deps.createFailureError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); }); }); } @@ -108,19 +113,18 @@ export class QuerySubprocessAdapter { if (error) { if (error.killed || (error as NodeJS.ErrnoException).code === 'ETIMEDOUT') { reject( - this.deps.createToolsError( + this.deps.createTimeoutError( timeoutMessage(command, args, this.deps.timeoutMs), command, args, - null, stderrStr, - { kind: 'timeout', timeoutMs: this.deps.timeoutMs }, + this.deps.timeoutMs, ), ); return; } reject( - this.deps.createToolsError( + this.deps.createFailureError( `gsd-tools exited with code ${error.code ?? 'unknown'}: ${command} ${args.join(' ')}${stderrStr ? `\n${stderrStr}` : ''}`, command, args, @@ -135,7 +139,7 @@ export class QuerySubprocessAdapter { ); child.on('error', (err) => { - reject(this.deps.createToolsError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); + reject(this.deps.createFailureError(`Failed to execute gsd-tools: ${err.message}`, command, args, null, '')); }); }); }