Skip to content

Commit 423f0dd

Browse files
committed
fix(desktop): preserve generation timeout reason and raise dropdown ceiling (#164)
When a long-running generation hits the configured timeout, the main-process AbortController was stamped with a CodesignError(GENERATION_TIMEOUT) — but the Anthropic / OpenAI SDKs catch the aborted fetch and rethrow a generic 'Request was aborted.' error, dropping signal.reason. The user ended up with an opaque message with no hint about which limit was hit or where to raise it. - Add extractGenerationTimeoutError(signal) to recover the stashed error. - Both generate IPC catches (v1 + legacy) now upgrade the SDK error back to GENERATION_TIMEOUT when our own timer fired, so the log row and the renderer toast both show the configured seconds + Settings → Advanced path. - Expand the Settings timeout dropdown to 60s / 2m / 3m / 5m / 10m / 20m / 30m / 1h / 2h (the default prefs value is 1200s but the old 60-300s list silently downgraded to 300 on save). When a stored value is outside the canonical list, inject it so the select shows the user's existing choice instead of rendering blank. Signed-off-by: hqhq1025 <1506751656@qq.com>
1 parent 7e75226 commit 423f0dd

6 files changed

Lines changed: 152 additions & 17 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@open-codesign/desktop": patch
3+
---
4+
5+
Fix: preserve the generation-timeout reason so long runs no longer surface a bare "Request was aborted." Provider SDKs rewrite aborted fetches into a generic message that drops `signal.reason`; the generate IPC now re-surfaces the stashed `GENERATION_TIMEOUT` CodesignError (with configured seconds + Settings path) when the controller was aborted by our own timer. Settings → Advanced → Generation timeout also gains 10m / 20m / 30m / 1h / 2h choices so the default 1200s and longer full-PDP runs can actually be configured without the dropdown silently downgrading the stored value.

apps/desktop/src/main/generation-ipc.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { CancelGenerationPayloadV1, CodesignError } from '@open-codesign/shared';
22
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3-
import { armGenerationTimeout, cancelGenerationRequest } from './generation-ipc';
3+
import {
4+
armGenerationTimeout,
5+
cancelGenerationRequest,
6+
extractGenerationTimeoutError,
7+
} from './generation-ipc';
48

59
function makeController() {
610
return { abort: vi.fn() } as unknown as AbortController;
@@ -182,3 +186,43 @@ describe('armGenerationTimeout', () => {
182186
expect(controller.signal.aborted).toBe(false);
183187
});
184188
});
189+
190+
describe('extractGenerationTimeoutError', () => {
191+
beforeEach(() => {
192+
vi.useFakeTimers();
193+
});
194+
afterEach(() => {
195+
vi.useRealTimers();
196+
});
197+
198+
it('returns the CodesignError stashed by armGenerationTimeout so the SDK-rewritten AbortError can be upgraded back to GENERATION_TIMEOUT', async () => {
199+
const controller = new AbortController();
200+
const logger = { warn: vi.fn() };
201+
202+
await armGenerationTimeout('gen-1', controller, async () => 3, logger);
203+
vi.advanceTimersByTime(3000);
204+
205+
const recovered = extractGenerationTimeoutError(controller.signal);
206+
expect(recovered).toBeInstanceOf(CodesignError);
207+
expect(recovered?.code).toBe('GENERATION_TIMEOUT');
208+
expect(recovered?.message).toContain('3s');
209+
expect(recovered?.message).toContain('Settings');
210+
});
211+
212+
it('returns null when the controller was aborted by a user-initiated cancel (no reason set)', () => {
213+
const controller = new AbortController();
214+
controller.abort();
215+
expect(extractGenerationTimeoutError(controller.signal)).toBeNull();
216+
});
217+
218+
it('returns null when the signal has not been aborted', () => {
219+
const controller = new AbortController();
220+
expect(extractGenerationTimeoutError(controller.signal)).toBeNull();
221+
});
222+
223+
it('returns null when the abort reason is some other CodesignError (not a timeout)', () => {
224+
const controller = new AbortController();
225+
controller.abort(new CodesignError('something else', 'PROVIDER_ABORTED'));
226+
expect(extractGenerationTimeoutError(controller.signal)).toBeNull();
227+
});
228+
});

apps/desktop/src/main/generation-ipc.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,21 @@ export async function armGenerationTimeout(
8282
}, ms);
8383
return () => clearTimeout(handle);
8484
}
85+
86+
/**
87+
* Provider SDKs (Anthropic / OpenAI) catch an aborted fetch and rethrow their
88+
* own generic `'Request was aborted.'` error, discarding `signal.reason`. When
89+
* the caught error came from our own timeout abort, we want the richer message
90+
* (with the configured seconds and the Settings path) to surface to the user.
91+
*
92+
* Returns the `CodesignError` we stashed on `signal.reason`, or `null` when the
93+
* abort was caused by something else (user-initiated cancel, upstream error).
94+
*/
95+
export function extractGenerationTimeoutError(signal: AbortSignal): CodesignError | null {
96+
if (!signal.aborted) return null;
97+
const reason = signal.reason;
98+
if (reason instanceof CodesignError && reason.code === ERROR_CODES.GENERATION_TIMEOUT) {
99+
return reason;
100+
}
101+
return null;
102+
}

apps/desktop/src/main/index.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ import { registerDiagnosticsIpc } from './diagnostics-ipc';
4343
import { makeRuntimeVerifier } from './done-verify';
4444
import { BrowserWindow, app, clipboard, dialog, ipcMain, shell } from './electron-runtime';
4545
import { registerExporterIpc } from './exporter-ipc';
46-
import { armGenerationTimeout, cancelGenerationRequest } from './generation-ipc';
46+
import {
47+
armGenerationTimeout,
48+
cancelGenerationRequest,
49+
extractGenerationTimeoutError,
50+
} from './generation-ipc';
4751
import { maybeAbortIfRunningFromDmg } from './install-check';
4852
import { registerLocaleIpc } from './locale-ipc';
4953
import { getLogPath, getLogger, initLogger } from './logger';
@@ -686,17 +690,23 @@ function registerIpcHandlers(db: Database | null): void {
686690
});
687691
return result;
688692
} catch (err) {
693+
// The SDK catches our AbortController and rethrows a generic
694+
// `'Request was aborted.'` that drops signal.reason. Prefer the
695+
// CodesignError we stashed on the signal so the user sees the
696+
// configured timeout + Settings path instead of an opaque message.
697+
const timeoutErr = extractGenerationTimeoutError(controller.signal);
698+
const rethrow = timeoutErr ?? err;
689699
logIpc.error('generate.fail', {
690700
generationId: id,
691701
ms: Date.now() - t0,
692702
provider: active.model.provider,
693703
modelId: active.model.modelId,
694704
baseUrl: baseUrl ?? '<default>',
695-
message: err instanceof Error ? err.message : String(err),
696-
code: err instanceof CodesignError ? err.code : undefined,
705+
message: rethrow instanceof Error ? rethrow.message : String(rethrow),
706+
code: rethrow instanceof CodesignError ? rethrow.code : undefined,
697707
});
698-
recordFinalError('generate', id, err);
699-
throw err;
708+
recordFinalError('generate', id, rethrow);
709+
throw rethrow;
700710
} finally {
701711
clearTimeoutGuard();
702712
inFlight.delete(id);
@@ -794,17 +804,23 @@ function registerIpcHandlers(db: Database | null): void {
794804
});
795805
return result;
796806
} catch (err) {
807+
// The SDK catches our AbortController and rethrows a generic
808+
// `'Request was aborted.'` that drops signal.reason. Prefer the
809+
// CodesignError we stashed on the signal so the user sees the
810+
// configured timeout + Settings path instead of an opaque message.
811+
const timeoutErr = extractGenerationTimeoutError(controller.signal);
812+
const rethrow = timeoutErr ?? err;
797813
logIpc.error('generate.fail', {
798814
generationId: id,
799815
ms: Date.now() - t0,
800816
provider: active.model.provider,
801817
modelId: active.model.modelId,
802818
baseUrl: baseUrl ?? '<default>',
803-
message: err instanceof Error ? err.message : String(err),
804-
code: err instanceof CodesignError ? err.code : undefined,
819+
message: rethrow instanceof Error ? rethrow.message : String(rethrow),
820+
code: rethrow instanceof CodesignError ? rethrow.code : undefined,
805821
});
806-
recordFinalError('generate', id, err);
807-
throw err;
822+
recordFinalError('generate', id, rethrow);
823+
throw rethrow;
808824
} finally {
809825
clearTimeoutGuard();
810826
inFlight.delete(id);

apps/desktop/src/renderer/src/components/Settings.test.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it, vi } from 'vitest';
2-
import { applyLocaleChange } from './Settings';
2+
import { TIMEOUT_OPTION_SECONDS, applyLocaleChange, resolveTimeoutOptions } from './Settings';
33

44
vi.mock('@open-codesign/i18n', () => ({
55
setLocale: vi.fn((locale: string) => Promise.resolve(locale)),
@@ -34,3 +34,34 @@ describe('applyLocaleChange', () => {
3434
expect(result).toBe('zh-CN');
3535
});
3636
});
37+
38+
describe('resolveTimeoutOptions', () => {
39+
it('covers the default 1200s stored value and long-generation 30m / 1h / 2h choices so users can configure what they need without hitting the old 300s ceiling', () => {
40+
expect(TIMEOUT_OPTION_SECONDS).toContain(1200);
41+
expect(TIMEOUT_OPTION_SECONDS).toContain(1800);
42+
expect(TIMEOUT_OPTION_SECONDS).toContain(3600);
43+
expect(TIMEOUT_OPTION_SECONDS).toContain(7200);
44+
});
45+
46+
it('returns the canonical options unchanged when the stored value is already present', () => {
47+
const options = resolveTimeoutOptions(1200);
48+
expect(options).toEqual([...TIMEOUT_OPTION_SECONDS]);
49+
});
50+
51+
it("merges a stored value that is not in the canonical list and keeps the list sorted so the select shows the user's existing choice instead of silently downgrading on save", () => {
52+
const options = resolveTimeoutOptions(900);
53+
expect(options).toContain(900);
54+
expect(options).toEqual([...options].sort((a, b) => a - b));
55+
// Canonical entries are preserved.
56+
for (const sec of TIMEOUT_OPTION_SECONDS) {
57+
expect(options).toContain(sec);
58+
}
59+
});
60+
61+
it('ignores non-positive or non-finite stored values rather than injecting bogus options', () => {
62+
expect(resolveTimeoutOptions(0)).toEqual([...TIMEOUT_OPTION_SECONDS]);
63+
expect(resolveTimeoutOptions(-1)).toEqual([...TIMEOUT_OPTION_SECONDS]);
64+
expect(resolveTimeoutOptions(Number.NaN)).toEqual([...TIMEOUT_OPTION_SECONDS]);
65+
expect(resolveTimeoutOptions(Number.POSITIVE_INFINITY)).toEqual([...TIMEOUT_OPTION_SECONDS]);
66+
});
67+
});

apps/desktop/src/renderer/src/components/Settings.tsx

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1593,6 +1593,29 @@ export async function applyLocaleChange(
15931593
return applied;
15941594
}
15951595

1596+
/**
1597+
* Canonical timeout choices shown in Settings → Advanced. The default prefs
1598+
* value is 1200s (20 min); long generations (full PDP runs) need 30-60 min,
1599+
* so the dropdown tops out at 2h. The old 60-300s ceiling silently clamped
1600+
* the stored value when the UI couldn't represent it.
1601+
*/
1602+
export const TIMEOUT_OPTION_SECONDS = [60, 120, 180, 300, 600, 1200, 1800, 3600, 7200] as const;
1603+
1604+
/**
1605+
* Returns the canonical timeout list with `currentSec` merged in when it is a
1606+
* positive finite value that isn't already present. Prevents the select from
1607+
* rendering with no selection when the user (or an earlier build) stored a
1608+
* custom value — and prevents a silent downgrade on the next save.
1609+
*/
1610+
export function resolveTimeoutOptions(currentSec: number): number[] {
1611+
const base: number[] = [...TIMEOUT_OPTION_SECONDS];
1612+
if (Number.isFinite(currentSec) && currentSec > 0 && !base.includes(currentSec)) {
1613+
base.push(currentSec);
1614+
base.sort((a, b) => a - b);
1615+
}
1616+
return base;
1617+
}
1618+
15961619
function AppearanceTab() {
15971620
const t = useT();
15981621
const theme = useCodesignStore((s) => s.theme);
@@ -2057,12 +2080,10 @@ function AdvancedTab() {
20572080
<NativeSelect
20582081
value={String(prefs.generationTimeoutSec)}
20592082
onChange={(v) => void updatePref({ generationTimeoutSec: Number(v) })}
2060-
options={[
2061-
{ value: '60', label: t('settings.advanced.timeoutSeconds', { value: 60 }) },
2062-
{ value: '120', label: t('settings.advanced.timeoutSeconds', { value: 120 }) },
2063-
{ value: '180', label: t('settings.advanced.timeoutSeconds', { value: 180 }) },
2064-
{ value: '300', label: t('settings.advanced.timeoutSeconds', { value: 300 }) },
2065-
]}
2083+
options={resolveTimeoutOptions(prefs.generationTimeoutSec).map((sec) => ({
2084+
value: String(sec),
2085+
label: t('settings.advanced.timeoutSeconds', { value: sec }),
2086+
}))}
20662087
/>
20672088
</Row>
20682089

0 commit comments

Comments
 (0)