Skip to content

Commit

Permalink
fix(utils): Handle when requests get aborted
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Aug 2, 2024
1 parent 5f3f531 commit f9e328c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,24 @@ import * as Sentry from '@sentry/react';
// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
import * as React from 'react';

const fetchSSE = async ({ timeout }: { timeout: boolean }) => {
const fetchSSE = async ({ timeout, abort = false }: { timeout: boolean, abort?: boolean }) => {
Sentry.startSpanManual({ name: 'sse stream using fetch' }, async span => {
const controller = new AbortController();

const res = await Sentry.startSpan({ name: 'sse fetch call' }, async () => {
const endpoint = `http://localhost:8080/${timeout ? 'sse-timeout' : 'sse'}`;
return await fetch(endpoint);

const signal = controller.signal;
return await fetch(endpoint, { signal });
});

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

const readChunk = async () => {
if (abort) {
controller.abort();
}
const readRes = await reader?.read();
if (readRes?.done) {
return;
Expand Down Expand Up @@ -42,6 +49,9 @@ const SSE = () => {
<button id="fetch-timeout-button" onClick={() => fetchSSE({ timeout: true })}>
Fetch timeout SSE
</button>
<button id="fetch-sse-abort" onClick={() => fetchSSE({ timeout: false, abort: true })}>
Fetch SSE with error
</button>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,43 @@ test('Waits for sse streaming when creating spans', async ({ page }) => {
expect(resolveBodyDuration).toBe(2);
});

test('Waits for sse streaming when sse has been explicitly aborted', async ({ page }) => {
await page.goto('/sse');

const transactionPromise = waitForTransaction('react-router-6', async transactionEvent => {
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
});

const fetchButton = page.locator('id=fetch-sse-abort');
await fetchButton.click();

const rootSpan = await transactionPromise;
console.log(JSON.stringify(rootSpan, null, 2));
const sseFetchCall = rootSpan.spans?.filter(span => span.description === 'sse fetch call')[0] as SpanJSON;
const httpGet = rootSpan.spans?.filter(span => span.description === 'GET http://localhost:8080/sse')[0] as SpanJSON;

expect(sseFetchCall).toBeDefined();
expect(httpGet).toBeDefined();

expect(sseFetchCall?.timestamp).toBeDefined();
expect(sseFetchCall?.start_timestamp).toBeDefined();
expect(httpGet?.timestamp).toBeDefined();
expect(httpGet?.start_timestamp).toBeDefined();

// http headers get sent instantly from the server
const resolveDuration = Math.round((sseFetchCall.timestamp as number) - sseFetchCall.start_timestamp);

// body streams after 0s because it has been aborted
const resolveBodyDuration = Math.round((httpGet.timestamp as number) - httpGet.start_timestamp);

expect(resolveDuration).toBe(0);
expect(resolveBodyDuration).toBe(0);

// validate abort eror was thrown by inspecting console
const consoleBreadcrumb = rootSpan.breadcrumbs?.find(breadcrumb => breadcrumb.category === 'console');
expect(consoleBreadcrumb?.message).toBe('Could not fetch sse AbortError: BodyStreamBuffer was aborted');
});

test('Aborts when stream takes longer than 5s', async ({ page }) => {
await page.goto('/sse');

Expand Down
59 changes: 25 additions & 34 deletions packages/utils/src/instrument/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,47 +80,42 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
if (onFetchResolved) {
onFetchResolved(response);
} else {
const finishedHandlerData: HandlerDataFetch = {
triggerHandlers('fetch', {
...handlerData,
endTimestamp: timestampInSeconds() * 1000,
response,
};
triggerHandlers('fetch', finishedHandlerData);
});
}

return response;
},
(error: Error) => {
if (!onFetchResolved) {
const erroredHandlerData: HandlerDataFetch = {
...handlerData,
endTimestamp: timestampInSeconds() * 1000,
error,
};

triggerHandlers('fetch', erroredHandlerData);

if (isError(error) && error.stack === undefined) {
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the error, that was caused by your fetch call did not
// have a stack trace, so the SDK backfilled the stack trace so
// you can see which fetch call failed.
error.stack = virtualStackTrace;
addNonEnumerableProperty(error, 'framesToPop', 1);
}
triggerHandlers('fetch', {
...handlerData,
endTimestamp: timestampInSeconds() * 1000,
error,
});

if (isError(error) && error.stack === undefined) {
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the sentry.javascript SDK caught an error invoking your application code.
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
throw error;
// it means the error, that was caused by your fetch call did not
// have a stack trace, so the SDK backfilled the stack trace so
// you can see which fetch call failed.
error.stack = virtualStackTrace;
addNonEnumerableProperty(error, 'framesToPop', 1);
}

// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the sentry.javascript SDK caught an error invoking your application code.
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
throw error;
},
);
};
});
}

function resolveResponse(res: Response | undefined, onFinishedResolving: () => void): void {
async function resolveResponse(res: Response | undefined, onFinishedResolving: () => void): Promise<void> {
if (res && res.body) {
const responseReader = res.body.getReader();

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

responseReader
return responseReader
.read()
.then(consumeChunks)
.then(() => {
onFinishedResolving();
})
.catch(() => {
// noop
});
.then(onFinishedResolving)
.catch(() => undefined);
}
}

async function streamHandler(response: Response): Promise<void> {
// clone response for awaiting stream
let clonedResponseForResolving: Response | undefined;
let clonedResponseForResolving: Response;
try {
clonedResponseForResolving = response.clone();
} catch (e) {
// noop
} catch {
return;
}

await resolveResponse(clonedResponseForResolving, () => {
Expand Down

0 comments on commit f9e328c

Please sign in to comment.