From 7a294a021de6dc52323d97c7924ffdcc217c2584 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 6 Jun 2024 14:10:51 +0000 Subject: [PATCH 1/7] feat(utils): Backfill stack trace on fetch errors if it is missing --- packages/utils/src/instrument/fetch.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index 4c502334024a..d4c6891fc36d 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import type { HandlerDataFetch } from '@sentry/types'; +import { isError } from '../is'; import { fill } from '../object'; import { supportsNativeFetch } from '../supports'; import { timestampInSeconds } from '../time'; @@ -65,6 +66,15 @@ function instrumentFetch(): void { }; 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 = new Error(error.message).stack; + } + // 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. From 859d2b1bb400ccb171643f4c758edd86f196dab4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Jun 2024 09:35:36 +0000 Subject: [PATCH 2/7] framesToPop --- packages/utils/src/instrument/fetch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index d4c6891fc36d..ea134dec1105 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -2,7 +2,7 @@ import type { HandlerDataFetch } from '@sentry/types'; import { isError } from '../is'; -import { fill } from '../object'; +import { addNonEnumerableProperty, fill } from '../object'; import { supportsNativeFetch } from '../supports'; import { timestampInSeconds } from '../time'; import { GLOBAL_OBJ } from '../worldwide'; @@ -73,6 +73,7 @@ function instrumentFetch(): void { // have a stack trace, so the SDK backfilled the stack trace so // you can see which fetch call failed. error.stack = new Error(error.message).stack; + addNonEnumerableProperty(error, 'framesToPop', 1); } // NOTE: If you are a Sentry user, and you are seeing this stack frame, From 95189945ba71ba4864481386630a596c1687ef6d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Jun 2024 12:51:29 +0000 Subject: [PATCH 3/7] Odd test fix --- .../suites/tracing/request/fetch/test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts index de6b1521d686..6a98022dcdd2 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts @@ -17,12 +17,8 @@ sentryTest('should create spans for fetch requests', async ({ getLocalTestPath, // We will wait 500ms for all envelopes to be sent. Generally, in all browsers, the last sent // envelope contains tracing data. - - // If we are on FF or webkit: - // 1st envelope contains CORS error - // 2nd envelope contains the tracing data we want to check here - const envelopes = await getMultipleSentryEnvelopeRequests(page, 2, { url, timeout: 10000 }); - const tracingEvent = envelopes[envelopes.length - 1]; // last envelope contains tracing data on all browsers + const envelopes = await getMultipleSentryEnvelopeRequests(page, 4, { url, timeout: 10000 }); + const tracingEvent = envelopes.find(event => event.type === 'transaction')!; // last envelope contains tracing data on all browsers const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client'); From 79b6fd5a5b20d8642e7e6ed3b5db434cd0927d81 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Jun 2024 16:10:08 +0000 Subject: [PATCH 4/7] test --- .../browser-integration-tests/playwright.config.ts | 2 +- .../globalHandlers/fetchStackTrace/subject.js | 1 + .../globalHandlers/fetchStackTrace/test.ts | 13 +++++++++++++ .../suites/integrations/globalHandlers/init.js | 7 +++++++ packages/utils/src/instrument/fetch.ts | 1 - 5 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/globalHandlers/init.js diff --git a/dev-packages/browser-integration-tests/playwright.config.ts b/dev-packages/browser-integration-tests/playwright.config.ts index 77ed6014d230..b03f758e11dc 100644 --- a/dev-packages/browser-integration-tests/playwright.config.ts +++ b/dev-packages/browser-integration-tests/playwright.config.ts @@ -11,7 +11,7 @@ const config: PlaywrightTestConfig = { testMatch: /test.ts/, use: { - trace: process.env.CI ? 'retain-on-failure' : 'off', + trace: 'retain-on-failure', }, projects: [ diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/subject.js b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/subject.js new file mode 100644 index 000000000000..2f2e65131b96 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/subject.js @@ -0,0 +1 @@ +fetch('http://localhost:123/fake/endpoint/that/will/fail'); diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts new file mode 100644 index 000000000000..6fb79eb71c5e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts @@ -0,0 +1,13 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getMultipleSentryEnvelopeRequests } from '../../../../utils/helpers'; + +sentryTest('should create errors with stack traces for failing fetch calls', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const envelopes = await getMultipleSentryEnvelopeRequests(page, 3, { url, timeout: 10000 }); + const errorEvent = envelopes.find(event => !event.type)!; + expect(errorEvent?.exception?.values?.[0].stacktrace).toBeDefined(); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/init.js b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/init.js new file mode 100644 index 000000000000..d8c94f36fdd0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/init.js @@ -0,0 +1,7 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', +}); diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index ea134dec1105..7fedbf81fb4f 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -73,7 +73,6 @@ function instrumentFetch(): void { // have a stack trace, so the SDK backfilled the stack trace so // you can see which fetch call failed. error.stack = new Error(error.message).stack; - addNonEnumerableProperty(error, 'framesToPop', 1); } // NOTE: If you are a Sentry user, and you are seeing this stack frame, From 6ec2efed29f153b86132bbb2c80db12ff6c635f1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 11 Jun 2024 07:24:43 +0000 Subject: [PATCH 5/7] more narrow test --- .../suites/integrations/globalHandlers/fetchStackTrace/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts index 6fb79eb71c5e..8d70241ec592 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/fetchStackTrace/test.ts @@ -9,5 +9,5 @@ sentryTest('should create errors with stack traces for failing fetch calls', asy const envelopes = await getMultipleSentryEnvelopeRequests(page, 3, { url, timeout: 10000 }); const errorEvent = envelopes.find(event => !event.type)!; - expect(errorEvent?.exception?.values?.[0].stacktrace).toBeDefined(); + expect(errorEvent?.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(0); }); From 3cc270dd51ec2cc9ee8f4f7417cc9fe5ed84fb18 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 11 Jun 2024 07:30:10 +0000 Subject: [PATCH 6/7] lint --- packages/utils/src/instrument/fetch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index 7fedbf81fb4f..d4c6891fc36d 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -2,7 +2,7 @@ import type { HandlerDataFetch } from '@sentry/types'; import { isError } from '../is'; -import { addNonEnumerableProperty, fill } from '../object'; +import { fill } from '../object'; import { supportsNativeFetch } from '../supports'; import { timestampInSeconds } from '../time'; import { GLOBAL_OBJ } from '../worldwide'; From dac339158b5a0940c70085719ee37e69ef0a7503 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 11 Jun 2024 08:30:44 +0000 Subject: [PATCH 7/7] fix trace locality --- packages/utils/src/instrument/fetch.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts index d4c6891fc36d..24c435fdf07c 100644 --- a/packages/utils/src/instrument/fetch.ts +++ b/packages/utils/src/instrument/fetch.ts @@ -2,7 +2,7 @@ import type { HandlerDataFetch } from '@sentry/types'; import { isError } from '../is'; -import { fill } from '../object'; +import { addNonEnumerableProperty, fill } from '../object'; import { supportsNativeFetch } from '../supports'; import { timestampInSeconds } from '../time'; import { GLOBAL_OBJ } from '../worldwide'; @@ -46,6 +46,15 @@ function instrumentFetch(): void { ...handlerData, }); + // We capture the stack right here and not in the Promise error callback because Safari (and probably other + // browsers too) will wipe the stack trace up to this point, only leaving us with this file which is useless. + + // 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. + const virtualStackTrace = new Error().stack; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access return originalFetch.apply(GLOBAL_OBJ, args).then( (response: Response) => { @@ -72,7 +81,8 @@ function instrumentFetch(): void { // 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 = new Error(error.message).stack; + error.stack = virtualStackTrace; + addNonEnumerableProperty(error, 'framesToPop', 1); } // NOTE: If you are a Sentry user, and you are seeing this stack frame,