Skip to content

Commit f260a36

Browse files
committed
fix(utils): Handle when requests get aborted
1 parent 5f3f531 commit f260a36

File tree

3 files changed

+75
-36
lines changed

3 files changed

+75
-36
lines changed

dev-packages/e2e-tests/test-applications/react-router-6/src/pages/SSE.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,24 @@ import * as Sentry from '@sentry/react';
22
// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
33
import * as React from 'react';
44

5-
const fetchSSE = async ({ timeout }: { timeout: boolean }) => {
5+
const fetchSSE = async ({ timeout, abort = false }: { timeout: boolean, abort?: boolean }) => {
66
Sentry.startSpanManual({ name: 'sse stream using fetch' }, async span => {
7+
const controller = new AbortController();
8+
79
const res = await Sentry.startSpan({ name: 'sse fetch call' }, async () => {
810
const endpoint = `http://localhost:8080/${timeout ? 'sse-timeout' : 'sse'}`;
9-
return await fetch(endpoint);
11+
12+
const signal = controller.signal;
13+
return await fetch(endpoint, { signal });
1014
});
1115

1216
const stream = res.body;
1317
const reader = stream?.getReader();
1418

1519
const readChunk = async () => {
20+
if (abort) {
21+
controller.abort();
22+
}
1623
const readRes = await reader?.read();
1724
if (readRes?.done) {
1825
return;
@@ -27,6 +34,7 @@ const fetchSSE = async ({ timeout }: { timeout: boolean }) => {
2734
await readChunk();
2835
} catch (error) {
2936
console.error('Could not fetch sse', error);
37+
(window as any)._FETCH_ABORT_ERROR = error;
3038
}
3139

3240
span.end();
@@ -42,6 +50,9 @@ const SSE = () => {
4250
<button id="fetch-timeout-button" onClick={() => fetchSSE({ timeout: true })}>
4351
Fetch timeout SSE
4452
</button>
53+
<button id="fetch-sse-abort" onClick={() => fetchSSE({ timeout: false, abort: true })}>
54+
Fetch SSE with error
55+
</button>
4556
</>
4657
);
4758
};

dev-packages/e2e-tests/test-applications/react-router-6/tests/sse.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,43 @@ test('Waits for sse streaming when creating spans', async ({ page }) => {
3434
expect(resolveBodyDuration).toBe(2);
3535
});
3636

37+
test('Waits for sse streaming when sse has been explicitly aborted', async ({ page }) => {
38+
await page.goto('/sse');
39+
40+
const transactionPromise = waitForTransaction('react-router-6', async transactionEvent => {
41+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
42+
});
43+
44+
const fetchButton = page.locator('id=fetch-sse-abort');
45+
await fetchButton.click();
46+
47+
const rootSpan = await transactionPromise;
48+
console.log(JSON.stringify(rootSpan, null, 2));
49+
const sseFetchCall = rootSpan.spans?.filter(span => span.description === 'sse fetch call')[0] as SpanJSON;
50+
const httpGet = rootSpan.spans?.filter(span => span.description === 'GET http://localhost:8080/sse')[0] as SpanJSON;
51+
52+
expect(sseFetchCall).toBeDefined();
53+
expect(httpGet).toBeDefined();
54+
55+
expect(sseFetchCall?.timestamp).toBeDefined();
56+
expect(sseFetchCall?.start_timestamp).toBeDefined();
57+
expect(httpGet?.timestamp).toBeDefined();
58+
expect(httpGet?.start_timestamp).toBeDefined();
59+
60+
// http headers get sent instantly from the server
61+
const resolveDuration = Math.round((sseFetchCall.timestamp as number) - sseFetchCall.start_timestamp);
62+
63+
// body streams after 0s because it has been aborted
64+
const resolveBodyDuration = Math.round((httpGet.timestamp as number) - httpGet.start_timestamp);
65+
66+
expect(resolveDuration).toBe(0);
67+
expect(resolveBodyDuration).toBe(0);
68+
69+
// validate abort eror was thrown by inspecting console
70+
const consoleBreadcrumb = rootSpan.breadcrumbs?.find(breadcrumb => breadcrumb.category === 'console');
71+
expect(consoleBreadcrumb?.message).toBe('Could not fetch sse AbortError: BodyStreamBuffer was aborted');
72+
});
73+
3774
test('Aborts when stream takes longer than 5s', async ({ page }) => {
3875
await page.goto('/sse');
3976

packages/utils/src/instrument/fetch.ts

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -80,47 +80,42 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
8080
if (onFetchResolved) {
8181
onFetchResolved(response);
8282
} else {
83-
const finishedHandlerData: HandlerDataFetch = {
83+
triggerHandlers('fetch', {
8484
...handlerData,
8585
endTimestamp: timestampInSeconds() * 1000,
8686
response,
87-
};
88-
triggerHandlers('fetch', finishedHandlerData);
87+
});
8988
}
9089

9190
return response;
9291
},
9392
(error: Error) => {
94-
if (!onFetchResolved) {
95-
const erroredHandlerData: HandlerDataFetch = {
96-
...handlerData,
97-
endTimestamp: timestampInSeconds() * 1000,
98-
error,
99-
};
100-
101-
triggerHandlers('fetch', erroredHandlerData);
102-
103-
if (isError(error) && error.stack === undefined) {
104-
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
105-
// it means the error, that was caused by your fetch call did not
106-
// have a stack trace, so the SDK backfilled the stack trace so
107-
// you can see which fetch call failed.
108-
error.stack = virtualStackTrace;
109-
addNonEnumerableProperty(error, 'framesToPop', 1);
110-
}
93+
triggerHandlers('fetch', {
94+
...handlerData,
95+
endTimestamp: timestampInSeconds() * 1000,
96+
error,
97+
});
11198

99+
if (isError(error) && error.stack === undefined) {
112100
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
113-
// it means the sentry.javascript SDK caught an error invoking your application code.
114-
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
115-
throw error;
101+
// it means the error, that was caused by your fetch call did not
102+
// have a stack trace, so the SDK backfilled the stack trace so
103+
// you can see which fetch call failed.
104+
error.stack = virtualStackTrace;
105+
addNonEnumerableProperty(error, 'framesToPop', 1);
116106
}
107+
108+
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
109+
// it means the sentry.javascript SDK caught an error invoking your application code.
110+
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
111+
throw error;
117112
},
118113
);
119114
};
120115
});
121116
}
122117

123-
function resolveResponse(res: Response | undefined, onFinishedResolving: () => void): void {
118+
async function resolveResponse(res: Response | undefined, onFinishedResolving: () => void): Promise<void> {
124119
if (res && res.body) {
125120
const responseReader = res.body.getReader();
126121

@@ -146,25 +141,21 @@ function resolveResponse(res: Response | undefined, onFinishedResolving: () => v
146141
}
147142
}
148143

149-
responseReader
144+
return responseReader
150145
.read()
151146
.then(consumeChunks)
152-
.then(() => {
153-
onFinishedResolving();
154-
})
155-
.catch(() => {
156-
// noop
157-
});
147+
.then(onFinishedResolving)
148+
.catch(() => undefined);
158149
}
159150
}
160151

161152
async function streamHandler(response: Response): Promise<void> {
162153
// clone response for awaiting stream
163-
let clonedResponseForResolving: Response | undefined;
154+
let clonedResponseForResolving: Response;
164155
try {
165156
clonedResponseForResolving = response.clone();
166-
} catch (e) {
167-
// noop
157+
} catch {
158+
return;
168159
}
169160

170161
await resolveResponse(clonedResponseForResolving, () => {

0 commit comments

Comments
 (0)