diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/init.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/init.js new file mode 100644 index 000000000000..7c200c542c56 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/subject.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/subject.js new file mode 100644 index 000000000000..78028b473ad7 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/subject.js @@ -0,0 +1,36 @@ +let controller; + +const startFetch = e => { + controller = new AbortController(); + const { signal } = controller; + + Sentry.startSpan( + { + name: 'with-abort-controller', + forceTransaction: true, + }, + async () => { + await fetch('http://localhost:7654/foo', { signal }) + .then(response => response.json()) + .then(data => { + console.log('Fetch succeeded:', data); + }) + .catch(err => { + if (err.name === 'AbortError') { + console.log('Fetch aborted'); + } else { + console.error('Fetch error:', err); + } + }); + }, + ); +}; + +const abortFetch = e => { + if (controller) { + controller.abort(); + } +}; + +document.querySelector('[data-test-id=start-button]').addEventListener('click', startFetch); +document.querySelector('[data-test-id=abort-button]').addEventListener('click', abortFetch); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/template.html b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/template.html new file mode 100644 index 000000000000..18cd917fe30f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/template.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/test.ts b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/test.ts new file mode 100644 index 000000000000..6cc3a0cd32a9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withAbortController/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; +import type { Event as SentryEvent } from '@sentry/types'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../../utils/helpers'; + +sentryTest('should handle aborted fetch calls', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.route('**/foo', async () => { + // never fulfil this route because we abort the request as part of the test + }); + + const transactionEventPromise = getFirstSentryEnvelopeRequest(page); + + const hasAbortedFetchPromise = new Promise(resolve => { + page.on('console', msg => { + if (msg.type() === 'log' && msg.text() === 'Fetch aborted') { + resolve(); + } + }); + }); + + await page.goto(url); + + await page.locator('[data-test-id=start-button]').click(); + await page.locator('[data-test-id=abort-button]').click(); + + const transactionEvent = await transactionEventPromise; + + // assert that fetch calls do not return undefined + const fetchBreadcrumbs = transactionEvent.breadcrumbs?.filter( + ({ category, data }) => category === 'fetch' && data === undefined, + ); + expect(fetchBreadcrumbs).toHaveLength(0); + + await expect(hasAbortedFetchPromise).resolves.toBeUndefined(); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/src/pages/SSE.tsx b/dev-packages/e2e-tests/test-applications/react-router-6/src/pages/SSE.tsx index 49e53b09cfa2..64a9f5717114 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/src/pages/SSE.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-6/src/pages/SSE.tsx @@ -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; @@ -42,6 +49,9 @@ const SSE = () => { + ); }; diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/sse.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/sse.test.ts index 5d4533726e36..92c06543c0b8 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/tests/sse.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6/tests/sse.test.ts @@ -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'); diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index 0ea1a4ec8d9f..afa209c01929 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -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 { if (res && res.body) { const responseReader = res.body.getReader(); @@ -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 { // clone response for awaiting stream - let clonedResponseForResolving: Response | undefined; + let clonedResponseForResolving: Response; try { clonedResponseForResolving = response.clone(); - } catch (e) { - // noop + } catch { + return; } await resolveResponse(clonedResponseForResolving, () => {