Skip to content

Commit 022e1b6

Browse files
authored
feat(diagnostics): bridge generate failures to diagnose() hypotheses (#130) (#165)
## Summary Test-connection 失败已经走 \`diagnose()\` 给出可操作 hypothesis(如 404 → \`missingV1\` + 自动加 \`/v1\`),但**真实生成失败完全没接这套系统**,用户只看到原始 IPC 错误文案,毫无方向。 本 PR 把 generate 失败接到 diagnostic hypothesis 系统,覆盖 404/4xx/5xx 多种场景,并为 404 missing-\`/v1\` 加自动 fix 按钮。 ## What changed ### main 进程 - \`apps/desktop/src/main/index.ts\`:generate catch 块给 thrown error 附加 \`upstream_status\` / \`upstream_provider\` / \`upstream_baseurl\` / \`upstream_wire\` metadata - 新增 helper \`extractUpstreamHttpStatus()\` ### shared - 新增 \`diagnoseGenerateFailure()\` in \`packages/shared/src/diagnostics.ts\`: - 400 + \"instructions\" 关键词 → \`openaiResponsesMisconfigured\` + \`switchWire\` 提示 - 5xx + \"not implemented\" / \"page not found\" → \`gatewayIncompatible\` + \`switchWire\` 提示 - 通用 5xx → \`serverError\` + \`waitAndRetry\` - 404(含裸 \"404 page not found\" 文案,#130 原始形态)→ \`missingV1\` + \`/v1\` 自动 fix - 401/403/429/402 → 委托现有 \`diagnose()\` - 其它 → \`unknown\`(toast 跳过避免噪音) ### renderer - \`apps/desktop/src/renderer/src/store.ts\` \`applyGenerateError\`:调用 \`diagnoseGenerateFailure\` 把 cause sentence 拼到 toast description;404 missing-\`/v1\` case 加 **\"Apply fix\" action button** 通过 \`config:v1:update-provider\` 自动加 \`/v1\` ### i18n(en + zh-CN) - \`diagnostics.cause.{gatewayIncompatible,openaiResponsesMisconfigured,serverError}\` - \`diagnostics.fix.switchWire\` - \`notifications.{generationFailedApplyFix,generationFailedBaseUrlUpdated}\` ## Design trade-offs - 新增 \`diagnoseGenerateFailure\` 而非扩展 \`diagnose()\` —— connection-test 传 \`ErrorCode\`,generate 传 \`{status, message, wire, ...}\`,重载会很别扭 - status 提取:先读 \`err.upstream_status\`,fallback 到 message regex —— Electron IPC 在某些版本会剥掉 custom Error props - 404 auto-fix 按钮只对 missing-\`/v1\` 实现,其它 hypothesis 只有提示文本(updateKey/switchWire/addCredits 需要用户判断) ## Test plan - [x] 10 new tests in \`diagnostics.test.ts\`(25/25 pass) - [x] 870/870 desktop tests green - [x] \`pnpm typecheck\` + \`pnpm lint\` clean ## Follow-up - 403 \"Your request was blocked\" / Cloudflare ray id 的 \`gatewayWafBlocked\` 分支(issue #124 专用)将在后续 commit 追加 - 其它 hypothesis 的 auto-fix 按钮(如 switchWire one-click) Closes #130 Refs #124, #134, #158 --------- Signed-off-by: hqhq1025 <1506751656@qq.com>
1 parent 63fa316 commit 022e1b6

9 files changed

Lines changed: 483 additions & 5 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@open-codesign/desktop': patch
3+
'@open-codesign/shared': minor
4+
'@open-codesign/i18n': patch
5+
---
6+
7+
feat(diagnostics): bridge generate failures to diagnose() hypotheses (#130)
8+
9+
- Main process `codesign:v1:generate` catch block now tags the thrown error with `upstream_status` / `upstream_provider` / `upstream_baseurl` / `upstream_wire` so the renderer can reason about the failure without re-parsing `err.message`.
10+
- New `diagnoseGenerateFailure()` in `@open-codesign/shared` maps generate-time failures to the same `DiagnosticHypothesis` shape the connection-test path already uses: 404 / "404 page not found" → missing `/v1`; 5xx with "not implemented" or "page not found" body → gateway does not implement the provider API; 400 with "instructions" body → openai-responses wire misconfigured; 401/403/429 reuse existing hypotheses.
11+
- Renderer `applyGenerateError` now appends the most-likely-cause sentence to the failure toast description and, for the missing-`/v1` case, surfaces an "Apply fix" action that updates the provider's baseUrl via `config:v1:update-provider` — addressing the Win11 relay-gateway failure in #130 with a one-click fix rather than a dead-end error message.
12+
- Adds new i18n cause keys (`gatewayIncompatible`, `openaiResponsesMisconfigured`, `serverError`) and fix keys (`switchWire`) in en + zh-CN.

apps/desktop/src/main/index.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,35 @@ function createWindow(): void {
160160

161161
type Database = BetterSqlite3.Database;
162162

163+
/**
164+
* Pull an HTTP status code out of a caught provider error. Mirrors
165+
* `packages/providers/src/retry.ts::extractStatus` intentionally — we don't
166+
* import from retry.ts to avoid coupling main to a retry-internal helper
167+
* that might get reshaped. Used by the generate catch block to tag the
168+
* thrown err with `upstream_status` so the renderer's diagnose pipeline
169+
* can pick up a hypothesis.
170+
*/
171+
function extractUpstreamHttpStatus(err: unknown): number | undefined {
172+
if (typeof err !== 'object' || err === null) return undefined;
173+
const candidates: unknown[] = [
174+
(err as { status?: unknown }).status,
175+
(err as { statusCode?: unknown }).statusCode,
176+
(err as { upstream_status?: unknown }).upstream_status,
177+
(err as { response?: { status?: unknown } }).response?.status,
178+
];
179+
for (const c of candidates) {
180+
if (typeof c === 'number' && Number.isFinite(c) && c >= 100 && c < 600) return c;
181+
}
182+
if (err instanceof Error) {
183+
const m = /\b(\d{3})\b/.exec(err.message);
184+
if (m?.[1]) {
185+
const n = Number(m[1]);
186+
if (n >= 400 && n < 600) return n;
187+
}
188+
}
189+
return undefined;
190+
}
191+
163192
function resolveActiveApiKeyFromState(providerId: string): Promise<string> {
164193
return resolveActiveApiKey(providerId, {
165194
getCodexAccessToken: () => getCodexTokenStore().getValidAccessToken(),
@@ -686,12 +715,35 @@ function registerIpcHandlers(db: Database | null): void {
686715
});
687716
return result;
688717
} catch (err) {
718+
// Attach upstream metadata to the thrown err so the renderer's
719+
// diagnostic pipeline (store.ts::applyGenerateError →
720+
// diagnoseGenerateFailure) can map this failure to a "most likely
721+
// cause + suggested fix" hypothesis. Without this, renderer only
722+
// sees err.message + err.code and cannot offer actionable hints
723+
// (e.g. the #130 404-page-not-found case that needs /v1 appended).
724+
const upstreamStatus = extractUpstreamHttpStatus(err);
725+
if (err !== null && typeof err === 'object') {
726+
const errAsRec = err as Record<string, unknown>;
727+
if (upstreamStatus !== undefined && errAsRec['upstream_status'] === undefined) {
728+
errAsRec['upstream_status'] = upstreamStatus;
729+
}
730+
if (errAsRec['upstream_provider'] === undefined) {
731+
errAsRec['upstream_provider'] = active.model.provider;
732+
}
733+
if (errAsRec['upstream_baseurl'] === undefined && baseUrl !== undefined) {
734+
errAsRec['upstream_baseurl'] = baseUrl;
735+
}
736+
if (errAsRec['upstream_wire'] === undefined && active.wire !== undefined) {
737+
errAsRec['upstream_wire'] = active.wire;
738+
}
739+
}
689740
logIpc.error('generate.fail', {
690741
generationId: id,
691742
ms: Date.now() - t0,
692743
provider: active.model.provider,
693744
modelId: active.model.modelId,
694745
baseUrl: baseUrl ?? '<default>',
746+
status: upstreamStatus,
695747
message: err instanceof Error ? err.message : String(err),
696748
code: err instanceof CodesignError ? err.code : undefined,
697749
});
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { applyGenerateBaseUrlFix, useCodesignStore } from './store';
3+
4+
const initialState = useCodesignStore.getState();
5+
6+
function resetStore(): void {
7+
useCodesignStore.setState({ ...initialState, toasts: [], reportableErrors: [] });
8+
}
9+
10+
function installWindow(updateProvider: unknown): void {
11+
(globalThis as unknown as { window: { codesign: unknown } }).window = {
12+
codesign: updateProvider === undefined ? {} : { config: { updateProvider } },
13+
};
14+
}
15+
16+
beforeEach(() => {
17+
resetStore();
18+
});
19+
20+
afterEach(() => {
21+
(globalThis as unknown as { window?: unknown }).window = undefined;
22+
vi.restoreAllMocks();
23+
});
24+
25+
describe('applyGenerateBaseUrlFix — silent-fallback regressions for #130', () => {
26+
it('emits a success toast when IPC updates the provider', async () => {
27+
const next = { ...(initialState.config ?? { schemaVersion: 3 }) } as unknown;
28+
const updateProvider = vi.fn().mockResolvedValue(next);
29+
installWindow(updateProvider);
30+
31+
const { getState, setState } = useCodesignStore;
32+
await applyGenerateBaseUrlFix(getState, setState, 'openai-compat', 'https://host/v1');
33+
34+
expect(updateProvider).toHaveBeenCalledWith({
35+
id: 'openai-compat',
36+
baseUrl: 'https://host/v1',
37+
});
38+
const toasts = useCodesignStore.getState().toasts;
39+
expect(toasts).toHaveLength(1);
40+
expect(toasts[0]?.variant).toBe('success');
41+
});
42+
43+
it('surfaces an error toast when IPC throws — never silent', async () => {
44+
const updateProvider = vi.fn().mockRejectedValue(new Error('ENOENT config.toml'));
45+
installWindow(updateProvider);
46+
47+
const { getState, setState } = useCodesignStore;
48+
await applyGenerateBaseUrlFix(getState, setState, 'openai-compat', 'https://host/v1');
49+
50+
const toasts = useCodesignStore.getState().toasts;
51+
expect(toasts).toHaveLength(1);
52+
expect(toasts[0]?.variant).toBe('error');
53+
expect(toasts[0]?.description).toContain('ENOENT config.toml');
54+
// Must also be reportable so users can file it.
55+
expect(useCodesignStore.getState().reportableErrors).toHaveLength(1);
56+
expect(useCodesignStore.getState().reportableErrors[0]?.code).toBe('GENERATE_FIX_APPLY_FAILED');
57+
});
58+
59+
it('surfaces an "unavailable" toast when updateProvider IPC is missing — not silent', async () => {
60+
installWindow(undefined);
61+
62+
const { getState, setState } = useCodesignStore;
63+
await applyGenerateBaseUrlFix(getState, setState, 'openai-compat', 'https://host/v1');
64+
65+
const toasts = useCodesignStore.getState().toasts;
66+
expect(toasts).toHaveLength(1);
67+
expect(toasts[0]?.variant).toBe('error');
68+
expect(useCodesignStore.getState().reportableErrors).toHaveLength(1);
69+
expect(useCodesignStore.getState().reportableErrors[0]?.code).toBe(
70+
'GENERATE_FIX_APPLY_UNAVAILABLE',
71+
);
72+
});
73+
});

apps/desktop/src/renderer/src/store.ts

Lines changed: 156 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
CommentScope,
1111
Design,
1212
DiagnosticEventRow,
13+
DiagnosticHypothesis,
1314
LocalInputFile,
1415
ModelRef,
1516
OnboardingState,
@@ -18,6 +19,7 @@ import type {
1819
ReportableError,
1920
SelectedElement,
2021
} from '@open-codesign/shared';
22+
import { diagnoseGenerateFailure } from '@open-codesign/shared';
2123
import { computeFingerprint } from '@open-codesign/shared/fingerprint';
2224
import { create } from 'zustand';
2325
import type { StoreApi } from 'zustand';
@@ -974,6 +976,41 @@ export function extractUpstreamContext(err: unknown): Record<string, unknown> |
974976
return Object.keys(out).length > 0 ? out : undefined;
975977
}
976978

979+
/**
980+
* Pull an HTTP status code off a caught generate error. Looks at the
981+
* `upstream_status` field main/index.ts attaches first, then falls back to
982+
* common SDK locations, and finally regex-scans `err.message` for the
983+
* #130-style "404 page not found" text that arrives with no structured status.
984+
*/
985+
export function extractGenerateStatus(err: unknown): number | undefined {
986+
if (err === null || typeof err !== 'object') return undefined;
987+
const rec = err as Record<string, unknown>;
988+
const candidates: unknown[] = [
989+
rec['upstream_status'],
990+
rec['status'],
991+
rec['statusCode'],
992+
(rec['response'] as { status?: unknown } | undefined)?.status,
993+
];
994+
for (const c of candidates) {
995+
if (typeof c === 'number' && Number.isFinite(c) && c >= 100 && c < 600) return c;
996+
}
997+
if (err instanceof Error) {
998+
const m = /\b([45]\d{2})\b/.exec(err.message);
999+
if (m?.[1]) return Number(m[1]);
1000+
}
1001+
return undefined;
1002+
}
1003+
1004+
/**
1005+
* Pick an upstream-* string field off an err, guarding the "wrong type"
1006+
* and "empty string" cases so callers can use `?? fallback`.
1007+
*/
1008+
function pickUpstreamString(err: unknown, key: string): string | undefined {
1009+
if (err === null || typeof err !== 'object') return undefined;
1010+
const v = (err as Record<string, unknown>)[key];
1011+
return typeof v === 'string' && v.length > 0 ? v : undefined;
1012+
}
1013+
9771014
function applyGenerateError(
9781015
get: GetState,
9791016
set: SetState,
@@ -1009,10 +1046,21 @@ function applyGenerateError(
10091046
}
10101047
const code = extractCodesignErrorCode(err) ?? 'GENERATION_FAILED';
10111048
const upstream = extractUpstreamContext(err);
1049+
1050+
// Bridge the failure into the connection-test diagnostics system so the
1051+
// toast tells the user WHY and WHAT TO TRY instead of just dumping the
1052+
// upstream message. Fixes #130 (404 → "add /v1") and gives #158 / #134 a
1053+
// home for gateway / instructions-required hints.
1054+
const cfg = get().config;
1055+
const hypothesis = deriveGenerateHypothesis(err, cfg);
1056+
const description = buildGenerateErrorDescription(msg, hypothesis);
1057+
const action = buildGenerateFixAction(get, set, hypothesis, err, cfg);
1058+
10121059
get().pushToast({
10131060
variant: 'error',
10141061
title: tr('notifications.generationFailed'),
1015-
description: msg,
1062+
description,
1063+
...(action !== undefined ? { action } : {}),
10161064
localId: get().createReportableError({
10171065
code,
10181066
scope: 'generate',
@@ -1024,6 +1072,113 @@ function applyGenerateError(
10241072
});
10251073
}
10261074

1075+
function deriveGenerateHypothesis(
1076+
err: unknown,
1077+
cfg: OnboardingState | null,
1078+
): DiagnosticHypothesis | undefined {
1079+
const provider = pickUpstreamString(err, 'upstream_provider') ?? cfg?.provider ?? 'unknown';
1080+
const baseUrl = pickUpstreamString(err, 'upstream_baseurl') ?? cfg?.baseUrl ?? undefined;
1081+
const wire = pickUpstreamString(err, 'upstream_wire');
1082+
const status = extractGenerateStatus(err);
1083+
const message = err instanceof Error ? err.message : undefined;
1084+
const ctx = {
1085+
provider,
1086+
...(baseUrl !== undefined && baseUrl !== null ? { baseUrl } : {}),
1087+
...(wire !== undefined ? { wire } : {}),
1088+
...(status !== undefined ? { status } : {}),
1089+
...(message !== undefined ? { message } : {}),
1090+
};
1091+
const hypotheses = diagnoseGenerateFailure(ctx);
1092+
const primary = hypotheses[0];
1093+
// Skip the bare "unknown" hypothesis — appending "Unknown error" to a
1094+
// toast that already shows the upstream message is just noise.
1095+
if (primary === undefined || primary.cause === 'diagnostics.cause.unknown') {
1096+
return undefined;
1097+
}
1098+
return primary;
1099+
}
1100+
1101+
function buildGenerateErrorDescription(
1102+
originalMessage: string,
1103+
hypothesis: DiagnosticHypothesis | undefined,
1104+
): string {
1105+
if (hypothesis === undefined) return originalMessage;
1106+
const hint = tr(hypothesis.cause);
1107+
// When the i18n key was missing, tr() falls back to returning the key
1108+
// itself; don't double up "diagnostics.cause.x" in the toast.
1109+
if (hint === hypothesis.cause) return originalMessage;
1110+
return `${originalMessage}\n\n${tr('diagnostics.mostLikelyCause')} ${hint}`;
1111+
}
1112+
1113+
function buildGenerateFixAction(
1114+
get: GetState,
1115+
set: SetState,
1116+
hypothesis: DiagnosticHypothesis | undefined,
1117+
err: unknown,
1118+
cfg: OnboardingState | null,
1119+
): Toast['action'] | undefined {
1120+
const fix = hypothesis?.suggestedFix;
1121+
if (fix === undefined) return undefined;
1122+
if (fix.baseUrlTransform === undefined) return undefined;
1123+
const providerId = pickUpstreamString(err, 'upstream_provider') ?? cfg?.provider;
1124+
const baseUrl = pickUpstreamString(err, 'upstream_baseurl') ?? cfg?.baseUrl ?? null;
1125+
if (
1126+
providerId === undefined ||
1127+
providerId === null ||
1128+
baseUrl === null ||
1129+
!/^https?:\/\/\S+/i.test(baseUrl.trim())
1130+
) {
1131+
return undefined;
1132+
}
1133+
const nextBaseUrl = fix.baseUrlTransform(baseUrl);
1134+
if (nextBaseUrl === baseUrl) return undefined;
1135+
return {
1136+
label: tr('notifications.generationFailedApplyFix'),
1137+
onClick: () => {
1138+
void applyGenerateBaseUrlFix(get, set, providerId, nextBaseUrl);
1139+
},
1140+
};
1141+
}
1142+
1143+
export async function applyGenerateBaseUrlFix(
1144+
get: GetState,
1145+
set: SetState,
1146+
providerId: string,
1147+
nextBaseUrl: string,
1148+
): Promise<void> {
1149+
const api = window.codesign?.config?.updateProvider;
1150+
// Don't silently swallow "this app version lacks the IPC" — surface it as a
1151+
// reportable error so users know why the Apply-fix button did nothing and
1152+
// can fall back to editing baseUrl manually in Settings.
1153+
if (api === undefined) {
1154+
get().reportableErrorToast({
1155+
code: 'GENERATE_FIX_APPLY_UNAVAILABLE',
1156+
scope: 'generate',
1157+
title: tr('notifications.generationFailedFixUnavailable'),
1158+
description: tr('notifications.generationFailedFixUnavailableDescription'),
1159+
});
1160+
return;
1161+
}
1162+
try {
1163+
const next = await api({ id: providerId, baseUrl: nextBaseUrl });
1164+
set({ config: next });
1165+
get().pushToast({
1166+
variant: 'success',
1167+
title: tr('notifications.generationFailedBaseUrlUpdated'),
1168+
});
1169+
} catch (updateErr) {
1170+
get().reportableErrorToast({
1171+
code: 'GENERATE_FIX_APPLY_FAILED',
1172+
scope: 'generate',
1173+
title: tr('notifications.generationFailedFixApplyFailed'),
1174+
description: updateErr instanceof Error ? updateErr.message : String(updateErr),
1175+
...(updateErr instanceof Error && updateErr.stack !== undefined
1176+
? { stack: updateErr.stack }
1177+
: {}),
1178+
});
1179+
}
1180+
}
1181+
10271182
function advanceStageIfCurrent(
10281183
get: GetState,
10291184
set: SetState,

packages/i18n/src/locales/en.json

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,11 @@
558558
"designSystemCleared": "Design system cleared",
559559
"clearDesignSystemFailed": "Unable to clear design system",
560560
"generationFailed": "Generation failed",
561+
"generationFailedApplyFix": "Apply fix",
562+
"generationFailedBaseUrlUpdated": "Base URL updated. Try the request again.",
563+
"generationFailedFixUnavailable": "Auto-fix unavailable",
564+
"generationFailedFixUnavailableDescription": "This app version can't apply the fix automatically. Please edit the Base URL manually in Settings.",
565+
"generationFailedFixApplyFailed": "Could not apply fix",
561566
"cancellationFailed": "Cancellation failed",
562567
"inlineCommentFailed": "Inline comment failed",
563568
"commentNeedsSnapshot": "Generate a design first before leaving a comment",
@@ -778,6 +783,9 @@
778783
"timedOut": "Request timed out — check firewall or VPN.",
779784
"corsError": "CORS error (should not happen in main process). This is a bug.",
780785
"sslError": "SSL / certificate error (self-signed cert on relay?).",
786+
"gatewayIncompatible": "The gateway accepted the connection but does not implement this provider's API. Try switching wire (e.g. openai-chat).",
787+
"openaiResponsesMisconfigured": "The endpoint rejected the request shape. The wire may be wrong — try switching to openai-chat.",
788+
"serverError": "Upstream server error. May be transient — try again.",
781789
"unknown": "Unknown error — check the full log for details."
782790
},
783791
"fix": {
@@ -789,7 +797,8 @@
789797
"checkNetwork": "Check network / VPN",
790798
"checkVpn": "Check VPN / firewall",
791799
"reportBug": "Report this bug",
792-
"disableTls": "Disable TLS verify"
800+
"disableTls": "Disable TLS verify",
801+
"switchWire": "Switch wire in Settings"
793802
},
794803
"applyFix": "Apply this fix",
795804
"setBaseUrlFirst": "Set a Base URL first",

0 commit comments

Comments
 (0)