Skip to content

Commit b41da76

Browse files
authored
feat(reliability): error boundaries + cancellation + retry + iframe error reporting (#11)
Adds the seven reliability gaps tracked in docs/research/09-polish-parity-backlog.md: C1 App-shell React error boundary with Reload + Copy stack actions C2 Per-pane boundaries (Sidebar / Preview / TopBar) so a single crash never blanks the window C5 AbortController threaded through core.generate -> providers.complete -> pi-ai signal; renderer races IPC against abort and shows Cancel button while generating C6 completeWithRetry wrapper in packages/providers: max 3 attempts, base 500ms with +/-20% jitter, retries only 5xx / network / 429-with-Retry-After. Every attempt is surfaced via onRetry — no silent retries (PRINCIPLES §10) C7 429 rate-limit handling: parse Retry-After, store rateLimitedUntil, show "Rate limited until HH:MM:SS" toast with countdown C10 Sandbox iframe error reporting: overlay.ts catches window.error + unhandledrejection and postMessages IFRAME_ERROR; surfaced as a slim red CanvasErrorBar above the iframe C11 Overlay defense: setInterval(reattach, 200) re-installs listeners in case generated HTML strips them; capture-phase listeners New files: packages/providers/src/retry.ts (+12 vitest cases in retry.test.ts) packages/runtime/src/iframe-errors.ts apps/desktop/src/renderer/src/components/ErrorBoundary.tsx apps/desktop/src/renderer/src/components/CanvasErrorBar.tsx No new prod dependencies. Tier 1 — no animations, no new libraries. All UI uses var(--color-*) tokens. Signed-off-by: hqhq1025 <1506751656@qq.com>
1 parent ffe81dc commit b41da76

11 files changed

Lines changed: 683 additions & 29 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* CanvasErrorBar — slim red strip shown above the preview iframe when the
3+
* sandbox runtime postMessages an IFRAME_ERROR.
4+
*
5+
* Loud surface (PRINCIPLES §10): every JS exception thrown inside the
6+
* generated HTML lands here with file + line. Users can dismiss the bar
7+
* (it clears the store slice) but errors are never auto-hidden.
8+
*/
9+
10+
import { X } from 'lucide-react';
11+
import { useCodesignStore } from '../store';
12+
13+
export function CanvasErrorBar() {
14+
const errors = useCodesignStore((s) => s.iframeErrors);
15+
const clear = useCodesignStore((s) => s.clearIframeErrors);
16+
if (errors.length === 0) return null;
17+
const latest = errors[errors.length - 1];
18+
if (!latest) return null;
19+
return (
20+
<div
21+
role="alert"
22+
className="flex items-start gap-3 px-4 py-2 border-b border-[var(--color-border)] bg-[color-mix(in_srgb,var(--color-error)_10%,var(--color-background))] text-[var(--color-text-primary)]"
23+
>
24+
<span className="mt-0.5 inline-block w-2 h-2 rounded-full bg-[var(--color-error)] shrink-0" />
25+
<div className="flex-1 min-w-0">
26+
<div className="text-xs font-semibold uppercase tracking-wide text-[var(--color-error)]">
27+
Preview runtime error{errors.length > 1 ? ` (${errors.length})` : ''}
28+
</div>
29+
<div className="text-sm text-[var(--color-text-primary)] truncate" title={latest}>
30+
{latest}
31+
</div>
32+
</div>
33+
<button
34+
type="button"
35+
onClick={clear}
36+
aria-label="Dismiss preview errors"
37+
className="shrink-0 p-1 rounded-[var(--radius-md)] text-[var(--color-text-muted)] hover:text-[var(--color-text-primary)] hover:bg-[var(--color-surface-hover)] transition-colors"
38+
>
39+
<X className="w-4 h-4" />
40+
</button>
41+
</div>
42+
);
43+
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Generic React error boundary.
3+
*
4+
* Strategy (PRINCIPLES §10): never silently swallow render exceptions.
5+
* If a child throws we render an actionable card with:
6+
* - the error message (loud, not hidden in console)
7+
* - "Reload" — re-mounts the children by bumping a key
8+
* - "Copy stack" — puts message+stack into the clipboard for bug reports
9+
*
10+
* Used both at the app shell (whole renderer) and per-pane (sidebar /
11+
* preview / topbar) so a single crash never blanks the entire window.
12+
*/
13+
14+
import { Button } from '@open-codesign/ui';
15+
import { Component, type ErrorInfo, type ReactNode } from 'react';
16+
17+
export interface ErrorBoundaryProps {
18+
children: ReactNode;
19+
/** Human-readable label used in the fallback heading: "Sidebar crashed". */
20+
scope?: string;
21+
/**
22+
* Optional custom fallback. Receives the error and a `reset` callback
23+
* that re-mounts the boundary's children.
24+
*/
25+
fallback?: (args: { error: Error; reset: () => void }) => ReactNode;
26+
}
27+
28+
interface ErrorBoundaryState {
29+
error: Error | null;
30+
resetKey: number;
31+
}
32+
33+
export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
34+
override state: ErrorBoundaryState = { error: null, resetKey: 0 };
35+
36+
static getDerivedStateFromError(error: Error): Partial<ErrorBoundaryState> {
37+
return { error };
38+
}
39+
40+
override componentDidCatch(error: Error, info: ErrorInfo): void {
41+
// Loud surface: also log so devtools shows the boundary that caught it.
42+
// Tier-1 — no remote reporting, BYOK / local-first.
43+
console.error(`[ErrorBoundary:${this.props.scope ?? 'root'}]`, error, info.componentStack);
44+
}
45+
46+
reset = (): void => {
47+
this.setState((s) => ({ error: null, resetKey: s.resetKey + 1 }));
48+
};
49+
50+
copyStack = async (): Promise<void> => {
51+
const err = this.state.error;
52+
if (!err) return;
53+
const text = `${err.name}: ${err.message}\n\n${err.stack ?? '(no stack)'}`;
54+
try {
55+
await navigator.clipboard.writeText(text);
56+
} catch {
57+
// Clipboard may be blocked in the sandbox; surface a textarea fallback
58+
// so the user can still grab the text instead of failing silently.
59+
const ta = document.createElement('textarea');
60+
ta.value = text;
61+
ta.style.position = 'fixed';
62+
ta.style.opacity = '0';
63+
document.body.appendChild(ta);
64+
ta.select();
65+
try {
66+
document.execCommand('copy');
67+
} finally {
68+
ta.remove();
69+
}
70+
}
71+
};
72+
73+
override render(): ReactNode {
74+
const { error, resetKey } = this.state;
75+
if (!error) {
76+
// Use resetKey to force remount of children after reset.
77+
return <ErrorBoundaryChildren key={resetKey}>{this.props.children}</ErrorBoundaryChildren>;
78+
}
79+
if (this.props.fallback) {
80+
return this.props.fallback({ error, reset: this.reset });
81+
}
82+
const scope = this.props.scope ?? 'this view';
83+
return (
84+
<div className="h-full w-full flex items-center justify-center p-6 bg-[var(--color-background)]">
85+
<div className="max-w-md w-full rounded-[var(--radius-2xl)] border border-[var(--color-border)] bg-[var(--color-surface)] shadow-[var(--shadow-card)] p-6">
86+
<div className="text-xs uppercase tracking-wide text-[var(--color-error)] font-semibold mb-2">
87+
{scope} crashed
88+
</div>
89+
<h2 className="text-lg font-semibold text-[var(--color-text-primary)] mb-1">
90+
{error.name}: {error.message}
91+
</h2>
92+
<p className="text-sm text-[var(--color-text-secondary)] mb-4">
93+
The rest of the app is still running. Reload this view, or copy the stack to file a bug.
94+
</p>
95+
<pre className="text-[11px] leading-snug font-mono text-[var(--color-text-muted)] bg-[var(--color-surface-active)] border border-[var(--color-border-muted)] rounded-[var(--radius-md)] p-3 max-h-40 overflow-auto whitespace-pre-wrap">
96+
{error.stack ?? '(no stack)'}
97+
</pre>
98+
<div className="mt-4 flex gap-2 justify-end">
99+
<Button
100+
type="button"
101+
variant="secondary"
102+
size="md"
103+
onClick={() => void this.copyStack()}
104+
>
105+
Copy stack
106+
</Button>
107+
<Button type="button" size="md" onClick={this.reset}>
108+
Reload
109+
</Button>
110+
</div>
111+
</div>
112+
</div>
113+
);
114+
}
115+
}
116+
117+
function ErrorBoundaryChildren({ children }: { children: ReactNode }): ReactNode {
118+
return children;
119+
}

packages/core/src/generate.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ const completeMock = vi.fn();
66

77
vi.mock('@open-codesign/providers', () => ({
88
complete: (...args: unknown[]) => completeMock(...args),
9+
completeWithRetry: (
10+
_model: unknown,
11+
_messages: unknown,
12+
_opts: unknown,
13+
_retryOpts: unknown,
14+
impl: (...args: unknown[]) => unknown,
15+
) => impl(_model, _messages, _opts),
916
}));
1017

1118
import { generate } from './index';

packages/core/src/index.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { type ArtifactEvent, createArtifactParser } from '@open-codesign/artifacts';
2-
import { complete } from '@open-codesign/providers';
2+
import { type RetryReason, complete, completeWithRetry } from '@open-codesign/providers';
33
import type { Artifact, ChatMessage, ModelRef } from '@open-codesign/shared';
44
import { CodesignError } from '@open-codesign/shared';
55
import { SYSTEM_PROMPTS } from '@open-codesign/templates';
@@ -11,6 +11,10 @@ export interface GenerateInput {
1111
apiKey: string;
1212
baseUrl?: string;
1313
systemPrompt?: string;
14+
/** Optional cancellation. Threaded through to pi-ai. */
15+
signal?: AbortSignal;
16+
/** Surfaced for every retry attempt — never silent (PRINCIPLES §10). */
17+
onRetry?: (info: RetryReason) => void;
1418
}
1519

1620
export interface GenerateOutput {
@@ -46,7 +50,9 @@ function collect(events: Iterable<ArtifactEvent>, into: Collected): void {
4650
/**
4751
* Generate one design artifact in response to a user prompt.
4852
* Tier 1: blocking call, returns the parsed artifact list at the end.
49-
* Tier 2 will switch to streaming with intermediate events.
53+
* Wraps the provider call in `completeWithRetry` so transient 5xx/429/network
54+
* failures retry with backoff. Caller passes `signal` to cancel and `onRetry`
55+
* to surface retry attempts in the UI.
5056
*/
5157
export async function generate(input: GenerateInput): Promise<GenerateOutput> {
5258
if (!input.prompt.trim()) {
@@ -59,10 +65,19 @@ export async function generate(input: GenerateInput): Promise<GenerateOutput> {
5965
{ role: 'user', content: input.prompt },
6066
];
6167

62-
const result = await complete(input.model, messages, {
63-
apiKey: input.apiKey,
64-
...(input.baseUrl !== undefined ? { baseUrl: input.baseUrl } : {}),
65-
});
68+
const result = await completeWithRetry(
69+
input.model,
70+
messages,
71+
{
72+
apiKey: input.apiKey,
73+
...(input.baseUrl !== undefined ? { baseUrl: input.baseUrl } : {}),
74+
...(input.signal !== undefined ? { signal: input.signal } : {}),
75+
},
76+
{
77+
...(input.onRetry !== undefined ? { onRetry: input.onRetry } : {}),
78+
},
79+
complete,
80+
);
6681

6782
const parser = createArtifactParser();
6883
const collected: Collected = { text: '', artifacts: [] };

packages/providers/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ export function detectProviderFromKey(key: string): ModelRef['provider'] | null
101101
export { pingProvider } from './validate';
102102
export type { ValidateResult } from './validate';
103103

104+
export { completeWithRetry, classifyError, sleepWithAbort } from './retry';
105+
export type { CompleteWithRetryOptions, RetryReason } from './retry';
106+
104107
// Tier 2 surface (not yet implemented):
105108
// structuredComplete<T>(model, schema, messages, opts): Promise<T>
106109
// streamArtifacts(model, messages, opts): AsyncIterable<ArtifactEvent>
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { type ChatMessage, CodesignError, type ModelRef } from '@open-codesign/shared';
2+
import { describe, expect, it, vi } from 'vitest';
3+
import type { GenerateOptions, GenerateResult } from './index';
4+
import { type RetryReason, classifyError, completeWithRetry } from './retry';
5+
6+
const MODEL: ModelRef = { provider: 'anthropic', modelId: 'claude-sonnet-4-6' };
7+
const MESSAGES: ChatMessage[] = [{ role: 'user', content: 'hi' }];
8+
const OPTS: GenerateOptions = { apiKey: 'test-key' };
9+
10+
const ok: GenerateResult = { content: 'hello', inputTokens: 1, outputTokens: 1, costUsd: 0 };
11+
12+
class HttpError extends Error {
13+
constructor(
14+
message: string,
15+
public readonly status: number,
16+
public readonly headers?: Record<string, string>,
17+
) {
18+
super(message);
19+
this.name = 'HttpError';
20+
}
21+
}
22+
23+
describe('classifyError', () => {
24+
it('marks 5xx as retryable', () => {
25+
expect(classifyError(new HttpError('boom', 503))).toMatchObject({ retry: true });
26+
});
27+
it('marks 4xx (non-429) as non-retryable', () => {
28+
expect(classifyError(new HttpError('bad', 400))).toMatchObject({ retry: false });
29+
});
30+
it('marks 429 as retryable and parses retry-after seconds', () => {
31+
const d = classifyError(new HttpError('slow', 429, { 'retry-after': '7' }));
32+
expect(d.retry).toBe(true);
33+
expect(d.retryAfterMs).toBe(7000);
34+
});
35+
it('marks AbortError as not retryable', () => {
36+
const err = new DOMException('Aborted', 'AbortError');
37+
expect(classifyError(err)).toMatchObject({ retry: false, reason: 'aborted' });
38+
});
39+
it('marks TypeError (fetch failure) as retryable', () => {
40+
expect(classifyError(new TypeError('fetch failed'))).toMatchObject({ retry: true });
41+
});
42+
});
43+
44+
describe('completeWithRetry', () => {
45+
it('returns the result on first-try success', async () => {
46+
const impl = vi.fn().mockResolvedValueOnce(ok);
47+
const out = await completeWithRetry(MODEL, MESSAGES, OPTS, {}, impl);
48+
expect(out).toEqual(ok);
49+
expect(impl).toHaveBeenCalledTimes(1);
50+
});
51+
52+
it('retries on 503 then succeeds, surfacing each attempt via onRetry', async () => {
53+
const impl = vi
54+
.fn()
55+
.mockRejectedValueOnce(new HttpError('boom', 503))
56+
.mockResolvedValueOnce(ok);
57+
const onRetry = vi.fn<(info: RetryReason) => void>();
58+
const out = await completeWithRetry(MODEL, MESSAGES, OPTS, { baseDelayMs: 1, onRetry }, impl);
59+
expect(out).toEqual(ok);
60+
expect(impl).toHaveBeenCalledTimes(2);
61+
expect(onRetry).toHaveBeenCalledTimes(1);
62+
expect(onRetry.mock.calls[0]?.[0].reason).toMatch(/server error/);
63+
});
64+
65+
it('throws after exhausting retries', async () => {
66+
const impl = vi.fn().mockRejectedValue(new HttpError('still down', 500));
67+
await expect(
68+
completeWithRetry(MODEL, MESSAGES, OPTS, { baseDelayMs: 1, maxRetries: 3 }, impl),
69+
).rejects.toThrow(/still down/);
70+
expect(impl).toHaveBeenCalledTimes(3);
71+
});
72+
73+
it('does not retry on a 401 client error', async () => {
74+
const impl = vi.fn().mockRejectedValue(new HttpError('unauthorized', 401));
75+
await expect(
76+
completeWithRetry(MODEL, MESSAGES, OPTS, { baseDelayMs: 1 }, impl),
77+
).rejects.toThrow(/unauthorized/);
78+
expect(impl).toHaveBeenCalledTimes(1);
79+
});
80+
81+
it('honours Retry-After on 429 (delay is at least retryAfterMs)', async () => {
82+
const impl = vi
83+
.fn()
84+
.mockRejectedValueOnce(new HttpError('slow', 429, { 'retry-after': '0.05' }))
85+
.mockResolvedValueOnce(ok);
86+
const onRetry = vi.fn<(info: RetryReason) => void>();
87+
const out = await completeWithRetry(MODEL, MESSAGES, OPTS, { baseDelayMs: 1, onRetry }, impl);
88+
expect(out).toEqual(ok);
89+
const info = onRetry.mock.calls[0]?.[0];
90+
expect(info?.retryAfterMs).toBe(50);
91+
expect(info?.delayMs).toBeGreaterThanOrEqual(50);
92+
});
93+
94+
it('aborts immediately when signal is already aborted', async () => {
95+
const impl = vi.fn().mockResolvedValue(ok);
96+
const ctrl = new AbortController();
97+
ctrl.abort();
98+
await expect(
99+
completeWithRetry(MODEL, MESSAGES, { ...OPTS, signal: ctrl.signal }, {}, impl),
100+
).rejects.toBeInstanceOf(CodesignError);
101+
expect(impl).not.toHaveBeenCalled();
102+
});
103+
104+
it('aborts mid-backoff when signal fires during retry sleep', async () => {
105+
const impl = vi.fn().mockRejectedValue(new HttpError('boom', 503));
106+
const ctrl = new AbortController();
107+
const promise = completeWithRetry(
108+
MODEL,
109+
MESSAGES,
110+
{ ...OPTS, signal: ctrl.signal },
111+
{ baseDelayMs: 5_000, maxRetries: 5 },
112+
impl,
113+
);
114+
setTimeout(() => ctrl.abort(), 10);
115+
await expect(promise).rejects.toThrow();
116+
expect(impl).toHaveBeenCalledTimes(1);
117+
});
118+
});

0 commit comments

Comments
 (0)