Skip to content

Commit a4e6065

Browse files
committed
fix: sanitize route errors and settle stream aborts
1 parent 02e5fcd commit a4e6065

4 files changed

Lines changed: 68 additions & 13 deletions

File tree

‎src/App.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export class RouteErrorBoundary extends Component<{ children: ReactNode }, { fai
100100

101101
componentDidCatch(error: Error, info: ErrorInfo) {
102102
console.error('Route rendering failed', error, info);
103-
captureException(error, { boundary: "route" });
103+
captureException(new Error("Route rendering failed"), { boundary: "route" });
104104
}
105105

106106
render() {

‎src/lib/ai/streaming.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,23 @@ export function consumeSSEChunk(buffer: string, chunk: string): { events: Parsed
7070
return { events, buffer: rest };
7171
}
7272

73+
function abortError(): DOMException {
74+
return new DOMException('Aborted', 'AbortError');
75+
}
76+
77+
function waitForCaller<T>(operation: PromiseLike<T>, signal?: AbortSignal): Promise<T> {
78+
if (!signal) return Promise.resolve(operation);
79+
if (signal.aborted) return Promise.reject(abortError());
80+
81+
return new Promise<T>((resolve, reject) => {
82+
const onAbort = () => reject(abortError());
83+
signal.addEventListener('abort', onAbort, { once: true });
84+
Promise.resolve(operation)
85+
.then(resolve, reject)
86+
.finally(() => signal.removeEventListener('abort', onAbort));
87+
});
88+
}
89+
7390
function captureStreamFailure(
7491
error: unknown,
7592
provider: string,
@@ -115,14 +132,14 @@ export async function streamAskChat(
115132
});
116133

117134
if (handlers.signal?.aborted) {
118-
const error = new DOMException('Aborted', 'AbortError');
135+
const error = abortError();
119136
captureStreamFailure(error, provider, config.model, startTime, true);
120137
throw error;
121138
}
122139

123140
let sessionData: Awaited<ReturnType<typeof supabase.auth.getSession>>['data'];
124141
try {
125-
({ data: sessionData } = await supabase.auth.getSession());
142+
({ data: sessionData } = await waitForCaller(supabase.auth.getSession(), handlers.signal));
126143
} catch (error) {
127144
captureStreamFailure(error, provider, config.model, startTime, Boolean(handlers.signal?.aborted));
128145
throw error;
@@ -135,9 +152,9 @@ export async function streamAskChat(
135152
throw error;
136153
}
137154
if (handlers.signal?.aborted) {
138-
const error = new DOMException('Aborted', 'AbortError');
155+
const error = abortError();
139156
captureStreamFailure(error, provider, config.model, startTime, true);
140-
throw new DOMException('Aborted', 'AbortError');
157+
throw error;
141158
}
142159

143160
const controller = new AbortController();
@@ -236,6 +253,7 @@ export async function streamAskChat(
236253
throw new DOMException('Aborted', 'AbortError');
237254
}
238255
const { done, value } = await reader.read();
256+
if (handlers.signal?.aborted) throw abortError();
239257
if (done) break;
240258

241259
const parsed = consumeSSEChunk(buffer, decoder.decode(value, { stream: true }));
@@ -261,9 +279,13 @@ export async function streamAskChat(
261279
return finalOutput;
262280
} catch (error) {
263281
const callerAborted = Boolean(handlers.signal?.aborted);
264-
const reportedError = !callerAborted && controller.signal.aborted ? new TimeoutError() : error;
282+
const reportedError = callerAborted
283+
? abortError()
284+
: controller.signal.aborted
285+
? new TimeoutError()
286+
: error;
265287
captureStreamFailure(reportedError, provider, config.model, startTime, callerAborted);
266-
if (handlers.signal?.aborted) throw error;
288+
if (callerAborted) throw reportedError;
267289
if (controller.signal.aborted) throw reportedError;
268290
throw error;
269291
} finally {

‎src/test/ai/streaming.test.ts‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,7 @@ describe('AI streaming helpers', () => {
232232
});
233233

234234
it('records caller aborts without sending them to Sentry', async () => {
235-
let resolveSession!: (result: { data: { session: { access_token: string } } }) => void;
236-
supabaseMock.auth.getSession.mockImplementationOnce(() => new Promise((resolve) => {
237-
resolveSession = resolve;
238-
}));
235+
supabaseMock.auth.getSession.mockImplementationOnce(() => new Promise(() => undefined));
239236
const controller = new AbortController();
240237
const request = streamAskChat(
241238
'private question',
@@ -246,7 +243,6 @@ describe('AI streaming helpers', () => {
246243
{ onDelta: () => undefined, signal: controller.signal },
247244
);
248245
controller.abort();
249-
resolveSession({ data: { session: { access_token: 'test-token' } } });
250246

251247
await expect(request).rejects.toMatchObject({ name: 'AbortError' });
252248
expect(captureProductEventMock).toHaveBeenNthCalledWith(2, 'ai_request_failed', expect.objectContaining({
@@ -258,5 +254,38 @@ describe('AI streaming helpers', () => {
258254
}));
259255
expect(captureExceptionMock).not.toHaveBeenCalled();
260256
});
257+
258+
it('does not report success when cancellation settles a pending stream read', async () => {
259+
const controller = new AbortController();
260+
const encoder = new TextEncoder();
261+
const body = new ReadableStream<Uint8Array>({
262+
start(streamController) {
263+
streamController.enqueue(encoder.encode(
264+
'event: final\ndata: {"output":{"response":"answer","followUpQuestions":[],"uncertainties":[]}}\n\n',
265+
));
266+
setTimeout(() => controller.abort(), 0);
267+
},
268+
});
269+
mockFetchOnce(body);
270+
271+
await expect(streamAskChat(
272+
'private question',
273+
[],
274+
'general',
275+
streamConfig,
276+
streamOptions,
277+
{ onDelta: () => undefined, signal: controller.signal },
278+
)).rejects.toMatchObject({ name: 'AbortError' });
279+
280+
expect(captureProductEventMock).toHaveBeenLastCalledWith(
281+
'ai_request_failed',
282+
expect.objectContaining({ errorName: 'AbortError', streaming: true }),
283+
);
284+
expect(captureProductEventMock).not.toHaveBeenCalledWith(
285+
'ai_request_succeeded',
286+
expect.anything(),
287+
);
288+
expect(captureExceptionMock).not.toHaveBeenCalled();
289+
});
261290
});
262291
});

‎src/test/app-route-analytics-error-boundary.test.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,11 @@ describe("App route observability", () => {
107107
);
108108

109109
expect(screen.getByRole("heading", { name: "Unable to load this page" })).toBeInTheDocument();
110-
expect(observability.captureException).toHaveBeenCalledWith(error, { boundary: "route" });
110+
expect(observability.captureException).toHaveBeenCalledWith(
111+
expect.objectContaining({ name: "Error", message: "Route rendering failed" }),
112+
{ boundary: "route" },
113+
);
114+
expect(JSON.stringify(observability.captureException.mock.calls)).not.toContain(error.message);
111115
expect(observability.captureException.mock.calls[0][1]).not.toHaveProperty("componentStack");
112116
} finally {
113117
window.removeEventListener("error", suppressWindowError);

0 commit comments

Comments
 (0)