Skip to content

Commit 011b25d

Browse files
authored
fix(desktop): preserve generation timeout reason and raise dropdown ceiling (#169)
Fixes #164. ## Summary - **Root cause of the opaque "Request was aborted." message.** When the generation timer fires, `armGenerationTimeout` calls `controller.abort(new CodesignError('Generation aborted after {n}s (Settings → Advanced → Generation timeout).', 'GENERATION_TIMEOUT'))`. But the Anthropic and OpenAI SDKs catch the aborted fetch and rethrow their own generic `'Request was aborted.'`, which drops `signal.reason`. The main-process `catch` then logged and re-threw that opaque error, so the user never saw the configured timeout or where to change it. - **Fix at the catch boundary, not the SDK boundary.** New `extractGenerationTimeoutError(signal)` reads back the `CodesignError` we stashed. Both `codesign:v1:generate` and the legacy `codesign:generate` catch blocks now prefer it over the rewritten SDK error, so the log row and the renderer toast both carry `GENERATION_TIMEOUT` with the full message. - **Settings dropdown ceiling raised.** The default preference is 1200s (20 min) but the UI only listed 60/120/180/300, so the select rendered blank and `updatePref` silently downgraded on save (covered in `project_bug_settings_timeout_dropdown` memory — this PR resolves that). Options now run 60s / 2m / 3m / 5m / 10m / 20m / 30m / 1h / 2h, and a stored value outside the list is injected so the user's existing choice is preserved. ## Changes - `apps/desktop/src/main/generation-ipc.ts` — export `extractGenerationTimeoutError(signal)`. - `apps/desktop/src/main/index.ts` — both generate catch blocks upgrade the SDK error back to `GENERATION_TIMEOUT` when our timer fired. - `apps/desktop/src/renderer/src/components/Settings.tsx` — `TIMEOUT_OPTION_SECONDS` + `resolveTimeoutOptions(currentSec)` helper; dropdown maps from it. - Tests: new `extractGenerationTimeoutError` suite (4 cases) + `resolveTimeoutOptions` suite (4 cases). - Changeset: patch. ## PRINCIPLES §5b - Compatibility: prefs schema unchanged, only the list of UI choices widened. Any stored timeout (including the legacy 1200) renders correctly. - Upgradeability: `resolveTimeoutOptions` merges unknown stored values, so we can change the canonical list in the future without stranding user settings. - No bloat: one new exported function per concern, no new deps. - Elegance: fix is at the one place where the SDK's rewrite lands (the catch), not spread across every provider adapter. ## Test plan - [x] `pnpm typecheck` — 10/10 tasks pass. - [x] `pnpm --filter @open-codesign/desktop test -- --run` — 878/878 pass. - [x] `pnpm lint` — clean. - [x] Full pre-commit hook (turbo full test + lint) — clean. - [ ] Manual: Settings → Advanced shows the new 30m/1h/2h choices; stored 1200s renders as "1200 s"; changing to 3600s and triggering a generation that exceeds the provider turn time shows the friendly GENERATION_TIMEOUT toast instead of "Request was aborted." ## Coordination - Does not touch `retry.ts` or `remapProviderError` (out of scope, other PRs own those). - Does not change `applyGenerateError` in the renderer store — the richer error code already flows through the normal error pipeline; a follow-up can add a "Open Settings" secondary action on the toast if desired. 😇 Signed-off-by: hqhq1025 <1506751656@qq.com>
1 parent 003d81b commit 011b25d

6 files changed

Lines changed: 157 additions & 18 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';
@@ -737,18 +741,24 @@ function registerIpcHandlers(db: Database | null): void {
737741
errAsRec['upstream_wire'] = active.wire;
738742
}
739743
}
744+
// The SDK catches our AbortController and rethrows a generic
745+
// `'Request was aborted.'` that drops signal.reason. Prefer the
746+
// CodesignError we stashed on the signal so the user sees the
747+
// configured timeout + Settings path instead of an opaque message.
748+
const timeoutErr = extractGenerationTimeoutError(controller.signal);
749+
const rethrow = timeoutErr ?? err;
740750
logIpc.error('generate.fail', {
741751
generationId: id,
742752
ms: Date.now() - t0,
743753
provider: active.model.provider,
744754
modelId: active.model.modelId,
745755
baseUrl: baseUrl ?? '<default>',
746756
status: upstreamStatus,
747-
message: err instanceof Error ? err.message : String(err),
748-
code: err instanceof CodesignError ? err.code : undefined,
757+
message: rethrow instanceof Error ? rethrow.message : String(rethrow),
758+
code: rethrow instanceof CodesignError ? rethrow.code : undefined,
749759
});
750-
recordFinalError('generate', id, err);
751-
throw err;
760+
recordFinalError('generate', id, rethrow);
761+
throw rethrow;
752762
} finally {
753763
clearTimeoutGuard();
754764
inFlight.delete(id);
@@ -846,17 +856,23 @@ function registerIpcHandlers(db: Database | null): void {
846856
});
847857
return result;
848858
} catch (err) {
859+
// The SDK catches our AbortController and rethrows a generic
860+
// `'Request was aborted.'` that drops signal.reason. Prefer the
861+
// CodesignError we stashed on the signal so the user sees the
862+
// configured timeout + Settings path instead of an opaque message.
863+
const timeoutErr = extractGenerationTimeoutError(controller.signal);
864+
const rethrow = timeoutErr ?? err;
849865
logIpc.error('generate.fail', {
850866
generationId: id,
851867
ms: Date.now() - t0,
852868
provider: active.model.provider,
853869
modelId: active.model.modelId,
854870
baseUrl: baseUrl ?? '<default>',
855-
message: err instanceof Error ? err.message : String(err),
856-
code: err instanceof CodesignError ? err.code : undefined,
871+
message: rethrow instanceof Error ? rethrow.message : String(rethrow),
872+
code: rethrow instanceof CodesignError ? rethrow.code : undefined,
857873
});
858-
recordFinalError('generate', id, err);
859-
throw err;
874+
recordFinalError('generate', id, rethrow);
875+
throw rethrow;
860876
} finally {
861877
clearTimeoutGuard();
862878
inFlight.delete(id);

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { describe, expect, it, vi } from 'vitest';
2-
import { applyLocaleChange, computeModelOptions } from './Settings';
2+
import {
3+
TIMEOUT_OPTION_SECONDS,
4+
applyLocaleChange,
5+
computeModelOptions,
6+
resolveTimeoutOptions,
7+
} from './Settings';
38

49
vi.mock('@open-codesign/i18n', () => ({
510
setLocale: vi.fn((locale: string) => Promise.resolve(locale)),
@@ -34,7 +39,6 @@ describe('applyLocaleChange', () => {
3439
expect(result).toBe('zh-CN');
3540
});
3641
});
37-
3842
describe('computeModelOptions', () => {
3943
const suffix = '(active, not in provider list)';
4044

@@ -88,3 +92,34 @@ describe('computeModelOptions', () => {
8892
]);
8993
});
9094
});
95+
96+
describe('resolveTimeoutOptions', () => {
97+
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', () => {
98+
expect(TIMEOUT_OPTION_SECONDS).toContain(1200);
99+
expect(TIMEOUT_OPTION_SECONDS).toContain(1800);
100+
expect(TIMEOUT_OPTION_SECONDS).toContain(3600);
101+
expect(TIMEOUT_OPTION_SECONDS).toContain(7200);
102+
});
103+
104+
it('returns the canonical options unchanged when the stored value is already present', () => {
105+
const options = resolveTimeoutOptions(1200);
106+
expect(options).toEqual([...TIMEOUT_OPTION_SECONDS]);
107+
});
108+
109+
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", () => {
110+
const options = resolveTimeoutOptions(900);
111+
expect(options).toContain(900);
112+
expect(options).toEqual([...options].sort((a, b) => a - b));
113+
// Canonical entries are preserved.
114+
for (const sec of TIMEOUT_OPTION_SECONDS) {
115+
expect(options).toContain(sec);
116+
}
117+
});
118+
119+
it('ignores non-positive or non-finite stored values rather than injecting bogus options', () => {
120+
expect(resolveTimeoutOptions(0)).toEqual([...TIMEOUT_OPTION_SECONDS]);
121+
expect(resolveTimeoutOptions(-1)).toEqual([...TIMEOUT_OPTION_SECONDS]);
122+
expect(resolveTimeoutOptions(Number.NaN)).toEqual([...TIMEOUT_OPTION_SECONDS]);
123+
expect(resolveTimeoutOptions(Number.POSITIVE_INFINITY)).toEqual([...TIMEOUT_OPTION_SECONDS]);
124+
});
125+
});

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,29 @@ export function computeModelOptions(input: {
16441644
return base;
16451645
}
16461646

1647+
/**
1648+
* Canonical timeout choices shown in Settings → Advanced. The default prefs
1649+
* value is 1200s (20 min); long generations (full PDP runs) need 30-60 min,
1650+
* so the dropdown tops out at 2h. The old 60-300s ceiling silently clamped
1651+
* the stored value when the UI couldn't represent it.
1652+
*/
1653+
export const TIMEOUT_OPTION_SECONDS = [60, 120, 180, 300, 600, 1200, 1800, 3600, 7200] as const;
1654+
1655+
/**
1656+
* Returns the canonical timeout list with `currentSec` merged in when it is a
1657+
* positive finite value that isn't already present. Prevents the select from
1658+
* rendering with no selection when the user (or an earlier build) stored a
1659+
* custom value — and prevents a silent downgrade on the next save.
1660+
*/
1661+
export function resolveTimeoutOptions(currentSec: number): number[] {
1662+
const base: number[] = [...TIMEOUT_OPTION_SECONDS];
1663+
if (Number.isFinite(currentSec) && currentSec > 0 && !base.includes(currentSec)) {
1664+
base.push(currentSec);
1665+
base.sort((a, b) => a - b);
1666+
}
1667+
return base;
1668+
}
1669+
16471670
function AppearanceTab() {
16481671
const t = useT();
16491672
const theme = useCodesignStore((s) => s.theme);
@@ -2109,12 +2132,10 @@ function AdvancedTab() {
21092132
<NativeSelect
21102133
value={String(prefs.generationTimeoutSec)}
21112134
onChange={(v) => void updatePref({ generationTimeoutSec: Number(v) })}
2112-
options={[
2113-
{ value: '60', label: t('settings.advanced.timeoutSeconds', { value: 60 }) },
2114-
{ value: '120', label: t('settings.advanced.timeoutSeconds', { value: 120 }) },
2115-
{ value: '180', label: t('settings.advanced.timeoutSeconds', { value: 180 }) },
2116-
{ value: '300', label: t('settings.advanced.timeoutSeconds', { value: 300 }) },
2117-
]}
2135+
options={resolveTimeoutOptions(prefs.generationTimeoutSec).map((sec) => ({
2136+
value: String(sec),
2137+
label: t('settings.advanced.timeoutSeconds', { value: sec }),
2138+
}))}
21182139
/>
21192140
</Row>
21202141

0 commit comments

Comments
 (0)