From 267ebe05c5cf09369eb84d883f1de7675b879bb0 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 21 Mar 2025 10:30:29 +0100 Subject: [PATCH 01/15] ref: Avoid some usage of `dropUndefinedKeys()` (#15757) This gets rid of some unnecessary invocations of `dropUndefinedKeys()` that we have. --- packages/browser-utils/src/metrics/cls.ts | 6 +-- packages/core/src/currentScopes.ts | 10 ++-- packages/core/src/tracing/sentrySpan.ts | 4 +- packages/core/src/utils-hoist/envelope.ts | 2 +- .../core/src/utils/applyScopeDataToEvent.ts | 21 ++++---- .../lib/utils/applyScopeDataToEvent.test.ts | 48 +++++++++++++++++++ packages/opentelemetry/src/spanExporter.ts | 22 ++++----- 7 files changed, 77 insertions(+), 36 deletions(-) diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index f9a6c662d79d..8e362bbfae1c 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -10,7 +10,7 @@ import { getRootSpan, spanToJSON, } from '@sentry/core'; -import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core'; +import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core'; import type { SpanAttributes } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; import { addClsInstrumentationHandler } from './instrument'; @@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift'; - const attributes: SpanAttributes = dropUndefinedKeys({ + const attributes: SpanAttributes = { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0, // attach the pageload span id to the CLS span so that we can link them in the UI 'sentry.pageload.span_id': pageloadSpanId, - }); + }; const span = startStandaloneWebVitalSpan({ name, diff --git a/packages/core/src/currentScopes.ts b/packages/core/src/currentScopes.ts index 6fab81298530..6bcdca2ae17b 100644 --- a/packages/core/src/currentScopes.ts +++ b/packages/core/src/currentScopes.ts @@ -4,7 +4,6 @@ import type { Client } from './client'; import { Scope } from './scope'; import type { TraceContext } from './types-hoist'; import { generateSpanId } from './utils-hoist'; -import { dropUndefinedKeys } from './utils-hoist/object'; /** * Get the currently active scope. @@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext { const { traceId, parentSpanId, propagationSpanId } = propagationContext; - const traceContext: TraceContext = dropUndefinedKeys({ + const traceContext: TraceContext = { trace_id: traceId, span_id: propagationSpanId || generateSpanId(), - parent_span_id: parentSpanId, - }); + }; + + if (parentSpanId) { + traceContext.parent_span_id = parentSpanId; + } return traceContext; } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 53f103c5ed52..6641e1e87296 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -373,9 +373,7 @@ export class SentrySpan implements Span { sdkProcessingMetadata: { capturedSpanScope, capturedSpanIsolationScope, - ...dropUndefinedKeys({ - dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), - }), + dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), }, ...(source && { transaction_info: { diff --git a/packages/core/src/utils-hoist/envelope.ts b/packages/core/src/utils-hoist/envelope.ts index 9655f9312579..2593079d5db6 100644 --- a/packages/core/src/utils-hoist/envelope.ts +++ b/packages/core/src/utils-hoist/envelope.ts @@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders( ...(sdkInfo && { sdk: sdkInfo }), ...(!!tunnel && dsn && { dsn: dsnToString(dsn) }), ...(dynamicSamplingContext && { - trace: dropUndefinedKeys({ ...dynamicSamplingContext }), + trace: dynamicSamplingContext, }), }; } diff --git a/packages/core/src/utils/applyScopeDataToEvent.ts b/packages/core/src/utils/applyScopeDataToEvent.ts index 93043b632c2d..1366fceef540 100644 --- a/packages/core/src/utils/applyScopeDataToEvent.ts +++ b/packages/core/src/utils/applyScopeDataToEvent.ts @@ -1,7 +1,6 @@ import type { ScopeData } from '../scope'; import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext'; import type { Breadcrumb, Event, Span } from '../types-hoist'; -import { dropUndefinedKeys } from '../utils-hoist/object'; import { merge } from './merge'; import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils'; @@ -113,24 +112,20 @@ export function mergeArray( function applyDataToEvent(event: Event, data: ScopeData): void { const { extra, tags, user, contexts, level, transactionName } = data; - const cleanedExtra = dropUndefinedKeys(extra); - if (Object.keys(cleanedExtra).length) { - event.extra = { ...cleanedExtra, ...event.extra }; + if (Object.keys(extra).length) { + event.extra = { ...extra, ...event.extra }; } - const cleanedTags = dropUndefinedKeys(tags); - if (Object.keys(cleanedTags).length) { - event.tags = { ...cleanedTags, ...event.tags }; + if (Object.keys(tags).length) { + event.tags = { ...tags, ...event.tags }; } - const cleanedUser = dropUndefinedKeys(user); - if (Object.keys(cleanedUser).length) { - event.user = { ...cleanedUser, ...event.user }; + if (Object.keys(user).length) { + event.user = { ...user, ...event.user }; } - const cleanedContexts = dropUndefinedKeys(contexts); - if (Object.keys(cleanedContexts).length) { - event.contexts = { ...cleanedContexts, ...event.contexts }; + if (Object.keys(contexts).length) { + event.contexts = { ...contexts, ...event.contexts }; } if (level) { diff --git a/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts index 2fb8b2f93d16..5803ab1d3468 100644 --- a/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts +++ b/packages/core/test/lib/utils/applyScopeDataToEvent.test.ts @@ -194,6 +194,54 @@ describe('mergeScopeData', () => { }); describe('applyScopeDataToEvent', () => { + it('should correctly merge nested event and scope data with undefined values', () => { + const eventData: Event = { + user: { + name: 'John', + age: undefined, + location: 'New York', + newThing: undefined, + }, + extra: {}, + }; + + const scopeData: ScopeData = { + eventProcessors: [], + breadcrumbs: [], + user: { + name: 'John', + age: 30, + location: 'Vienna', + role: 'developer', + thing: undefined, + }, + tags: {}, + extra: {}, + contexts: {}, + attachments: [], + propagationContext: { traceId: '1', sampleRand: 0.42 }, + sdkProcessingMetadata: {}, + fingerprint: [], + }; + + applyScopeDataToEvent(eventData, scopeData); + + // Verify merged data structure + expect(eventData).toEqual({ + user: { + name: 'John', + age: undefined, + location: 'New York', + role: 'developer', + thing: undefined, + newThing: undefined, + }, + extra: {}, + breadcrumbs: undefined, + sdkProcessingMetadata: {}, + }); + }); + it("doesn't apply scope.transactionName to transaction events", () => { const data: ScopeData = { eventProcessors: [], diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 1c88afea0f51..e095ab60e805 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined; - const attributes: SpanAttributes = dropUndefinedKeys({ + const attributes: SpanAttributes = { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin, ...data, ...removeSentryAttributes(span.attributes), - }); + }; const { links } = span; const { traceId: trace_id, spanId: span_id } = span.spanContext(); @@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve const status = mapStatus(span); - const traceContext: TraceContext = dropUndefinedKeys({ + const traceContext: TraceContext = { parent_span_id, span_id, trace_id, @@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve op, status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined links: convertSpanLinksForEnvelope(links), - }); + }; const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]; const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined; @@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve transaction: description, type: 'transaction', sdkProcessingMetadata: { - ...dropUndefinedKeys({ - capturedSpanScope: capturedSpanScopes.scope, - capturedSpanIsolationScope: capturedSpanScopes.isolationScope, - sampleRate, - dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span), - }), + capturedSpanScope: capturedSpanScopes.scope, + capturedSpanIsolationScope: capturedSpanScopes.isolationScope, + sampleRate, + dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span), }, ...(source && { transaction_info: { @@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS const { attributes, startTime, endTime, parentSpanId, links } = span; const { op, description, data, origin = 'manual' } = getSpanData(span); - const allData = dropUndefinedKeys({ + const allData = { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, ...removeSentryAttributes(attributes), ...data, - }); + }; const status = mapStatus(span); From 1a4fa0cf5752ca6e07681df43f2e12404aa28f9e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 11:21:16 +0100 Subject: [PATCH 02/15] feat(browser): Add `previous_trace` span links (#15569) This PR adds logic to set the `previous_trace ` span link on root spans (via `browserTracingIntegration`). - added `linkPreviousTrace` integration option to control the trace linking behaviour: - everything is implemented within `browserTracingIntegration`, meaning there's no bundle size hit for error-only users or users who only send manual spans (the latter is a tradeoff but I think it's a fair one) - added unit and integration tests for a bunch of scenarios closes https://github.com/getsentry/sentry-javascript/issues/14992 UPDATE: I rewrote the public API options from having two options (`enablePreviousTrace` and `persistPreviousTrace`) to only one which controls both aspects. --- .size-limit.js | 2 +- .../custom-trace/subject.js | 15 ++ .../custom-trace/template.html | 9 + .../previous-trace-links/custom-trace/test.ts | 63 +++++ .../previous-trace-links/default/test.ts | 92 +++++++ .../previous-trace-links/init.js | 10 + .../interaction-spans/init.js | 9 + .../interaction-spans/template.html | 8 + .../interaction-spans/test.ts | 90 +++++++ .../previous-trace-links/meta/template.html | 9 + .../previous-trace-links/meta/test.ts | 55 +++++ .../negatively-sampled/init.js | 14 ++ .../negatively-sampled/test.ts | 39 +++ .../session-storage/init.js | 9 + .../session-storage/test.ts | 40 ++++ .../tests/client-transactions.test.ts | 14 +- .../tests/transactions.test.ts | 20 ++ .../tests/transactions.test.ts | 10 + .../tests/transactions.test.ts | 10 + .../src/tracing/browserTracingIntegration.ts | 49 ++++ packages/browser/src/tracing/previousTrace.ts | 103 ++++++++ .../tracing/browserTracingIntegration.test.ts | 107 ++++++++- .../test/tracing/previousTrace.test.ts | 225 ++++++++++++++++++ packages/core/src/semanticAttributes.ts | 12 + 24 files changed, 1010 insertions(+), 4 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts create mode 100644 packages/browser/src/tracing/previousTrace.ts create mode 100644 packages/browser/test/tracing/previousTrace.test.ts diff --git a/.size-limit.js b/.size-limit.js index 203e8cecb6ce..7ecd54ab92f4 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -47,7 +47,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '75.5 KB', + limit: '76 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js new file mode 100644 index 000000000000..c6fbf085140a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/subject.js @@ -0,0 +1,15 @@ +const btn1 = document.getElementById('btn1'); +const btn2 = document.getElementById('btn2'); + +btn1.addEventListener('click', () => { + Sentry.startNewTrace(() => { + Sentry.startSpan({name: 'custom root span 1', op: 'custom'}, () => {}); + }); +}); + + +btn2.addEventListener('click', () => { + Sentry.startNewTrace(() => { + Sentry.startSpan({name: 'custom root span 2', op: 'custom'}, () => {}); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html new file mode 100644 index 000000000000..d5b66b29965d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/template.html @@ -0,0 +1,9 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts new file mode 100644 index 000000000000..ab2d8ae2c8af --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/custom-trace/test.ts @@ -0,0 +1,63 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest('manually started custom traces are linked correctly in the chain', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + return pageloadRequest.contexts?.trace; + }); + + const customTrace1Context = await sentryTest.step('Custom trace', async () => { + const customTrace1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'custom'); + await page.locator('#btn1').click(); + const customTrace1Event = envelopeRequestParser(await customTrace1RequestPromise); + + const customTraceCtx = customTrace1Event.contexts?.trace; + + expect(customTraceCtx?.trace_id).not.toEqual(pageloadTraceContext?.trace_id); + expect(customTraceCtx?.links).toEqual([ + { + trace_id: pageloadTraceContext?.trace_id, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + return customTraceCtx; + }); + + await sentryTest.step('Navigation', async () => { + const navigation1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigation1RequestPromise); + const navTraceContext = navigationEvent.contexts?.trace; + + expect(navTraceContext?.trace_id).not.toEqual(customTrace1Context?.trace_id); + expect(navTraceContext?.trace_id).not.toEqual(pageloadTraceContext?.trace_id); + + expect(navTraceContext?.links).toEqual([ + { + trace_id: customTrace1Context?.trace_id, + span_id: customTrace1Context?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts new file mode 100644 index 000000000000..bf1d9f78e308 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts @@ -0,0 +1,92 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest("navigation spans link back to previous trace's root span", async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + return pageloadRequest.contexts?.trace; + }); + + const navigation1TraceContext = await sentryTest.step('First navigation', async () => { + const navigation1RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigation1Request = envelopeRequestParser(await navigation1RequestPromise); + return navigation1Request.contexts?.trace; + }); + + const navigation2TraceContext = await sentryTest.step('Second navigation', async () => { + const navigation2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#bar`); + const navigation2Request = envelopeRequestParser(await navigation2RequestPromise); + return navigation2Request.contexts?.trace; + }); + + const pageloadTraceId = pageloadTraceContext?.trace_id; + const navigation1TraceId = navigation1TraceContext?.trace_id; + const navigation2TraceId = navigation2TraceContext?.trace_id; + + expect(pageloadTraceContext?.links).toBeUndefined(); + + expect(navigation1TraceContext?.links).toEqual([ + { + trace_id: pageloadTraceId, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigation2TraceContext?.links).toEqual([ + { + trace_id: navigation1TraceId, + span_id: navigation1TraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(pageloadTraceId).not.toEqual(navigation1TraceId); + expect(navigation1TraceId).not.toEqual(navigation2TraceId); + expect(pageloadTraceId).not.toEqual(navigation2TraceId); +}); + +sentryTest("doesn't link between hard page reloads by default", async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await sentryTest.step('First pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageload1Event = envelopeRequestParser(await pageloadRequestPromise); + + expect(pageload1Event.contexts?.trace).toBeDefined(); + expect(pageload1Event.contexts?.trace?.links).toBeUndefined(); + }); + + await sentryTest.step('Second pageload', async () => { + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + const pageload2Event = envelopeRequestParser(await pageload2RequestPromise); + + expect(pageload2Event.contexts?.trace).toBeDefined(); + expect(pageload2Event.contexts?.trace?.links).toBeUndefined(); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js new file mode 100644 index 000000000000..1ce4125ee422 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, + debug: true, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js new file mode 100644 index 000000000000..5d32c7af1e43 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1, + integrations: [Sentry.browserTracingIntegration({_experiments: {enableInteractions: true}})], +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html new file mode 100644 index 000000000000..05c7fc4b2417 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/template.html @@ -0,0 +1,8 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts new file mode 100644 index 000000000000..ca2a184f6680 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/interaction-spans/test.ts @@ -0,0 +1,90 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +/* + This is quite peculiar behavior but it's a result of the route-based trace lifetime. + Once we shortened trace lifetime, this whole scenario will change as the interaction + spans will be their own trace. So most likely, we can replace this test with a new one + that covers the new default behavior. +*/ +sentryTest( + 'only the first root spans in the trace link back to the previous trace', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + + const pageloadEvent = envelopeRequestParser(await pageloadRequestPromise); + const traceContext = pageloadEvent.contexts?.trace; + + expect(traceContext).toBeDefined(); + expect(traceContext?.links).toBeUndefined(); + + return traceContext; + }); + + await sentryTest.step('Click Before navigation', async () => { + const interactionRequestPromise = waitForTransactionRequest(page, evt => { + return evt.contexts?.trace?.op === 'ui.action.click'; + }); + await page.click('#btn'); + + const interactionEvent = envelopeRequestParser(await interactionRequestPromise); + const interactionTraceContext = interactionEvent.contexts?.trace; + + // sanity check: route-based trace lifetime means the trace_id should be the same + expect(interactionTraceContext?.trace_id).toBe(pageloadTraceContext?.trace_id); + + // no links yet as previous root span belonged to same trace + expect(interactionTraceContext?.links).toBeUndefined(); + }); + + const navigationTraceContext = await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigationRequestPromise); + + const traceContext = navigationEvent.contexts?.trace; + + expect(traceContext?.op).toBe('navigation'); + expect(traceContext?.links).toEqual([ + { + trace_id: pageloadTraceContext?.trace_id, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(traceContext?.trace_id).not.toEqual(traceContext?.links![0].trace_id); + return traceContext; + }); + + await sentryTest.step('Click After navigation', async () => { + const interactionRequestPromise = waitForTransactionRequest(page, evt => { + return evt.contexts?.trace?.op === 'ui.action.click'; + }); + await page.click('#btn'); + const interactionEvent = envelopeRequestParser(await interactionRequestPromise); + + const interactionTraceContext = interactionEvent.contexts?.trace; + + // sanity check: route-based trace lifetime means the trace_id should be the same + expect(interactionTraceContext?.trace_id).toBe(navigationTraceContext?.trace_id); + + // since this is the second root span in the trace, it doesn't link back to the previous trace + expect(interactionTraceContext?.links).toBeUndefined(); + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html new file mode 100644 index 000000000000..f8024594da10 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/template.html @@ -0,0 +1,9 @@ + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts new file mode 100644 index 000000000000..f5e2c7d613e0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/meta/test.ts @@ -0,0 +1,55 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + "links back to previous trace's local root span if continued from meta tags", + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const metaTagTraceId = '12345678901234567890123456789012'; + + const pageloadTraceContext = await sentryTest.step('Initial pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + + const traceContext = pageloadRequest.contexts?.trace; + + // sanity check + expect(traceContext?.trace_id).toBe(metaTagTraceId); + + expect(traceContext?.links).toBeUndefined(); + + return traceContext; + }); + + const navigationTraceContext = await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + return navigationRequest.contexts?.trace; + }); + + const navigationTraceId = navigationTraceContext?.trace_id; + + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: metaTagTraceId, + span_id: pageloadTraceContext?.span_id, + sampled: true, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigationTraceId).not.toEqual(metaTagTraceId); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js new file mode 100644 index 000000000000..6c884b0632c8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/init.js @@ -0,0 +1,14 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampler: (ctx) => { + if (ctx.attributes['sentry.origin'] === 'auto.pageload.browser') { + return 0; + } + return 1; + } +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts new file mode 100644 index 000000000000..2563b22ad701 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts @@ -0,0 +1,39 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest('includes a span link to a previously negatively sampled span', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await sentryTest.step('Initial pageload', async () => { + // No event to check for an event here because this pageload span is sampled negatively! + await page.goto(url); + }); + + await sentryTest.step('Navigation', async () => { + const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation'); + await page.goto(`${url}#foo`); + const navigationEvent = envelopeRequestParser(await navigationRequestPromise); + const navigationTraceContext = navigationEvent.contexts?.trace; + + expect(navigationTraceContext?.op).toBe('navigation'); + expect(navigationTraceContext?.links).toEqual([ + { + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + sampled: false, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }, + ]); + + expect(navigationTraceContext?.trace_id).not.toEqual(navigationTraceContext?.links![0].trace_id); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js new file mode 100644 index 000000000000..c346a6e054b3 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({linkPreviousTrace: 'session-storage'})], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts new file mode 100644 index 000000000000..7489f528a0e8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/session-storage/test.ts @@ -0,0 +1,40 @@ +import { expect } from '@playwright/test'; +import { SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE } from '@sentry/core'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest('adds link between hard page reloads when opting into sessionStorage', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageload1TraceContext = await sentryTest.step('First pageload', async () => { + const pageloadRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.goto(url); + const pageload1Event = envelopeRequestParser(await pageloadRequestPromise); + const pageload1TraceContext = pageload1Event.contexts?.trace; + expect(pageload1TraceContext).toBeDefined(); + expect(pageload1TraceContext?.links).toBeUndefined(); + return pageload1TraceContext; + }); + + const pageload2Event = await sentryTest.step('Hard page reload', async () => { + const pageload2RequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'pageload'); + await page.reload(); + return envelopeRequestParser(await pageload2RequestPromise); + }); + + expect(pageload2Event.contexts?.trace?.links).toEqual([ + { + trace_id: pageload1TraceContext?.trace_id, + span_id: pageload1TraceContext?.span_id, + sampled: true, + attributes: { [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace' }, + }, + ]); + + expect(pageload1TraceContext?.trace_id).not.toEqual(pageload2Event.contexts?.trace?.trace_id); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts index 8721a4d086bb..cbb2cae29265 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts @@ -43,7 +43,7 @@ test('Sends a pageload transaction to Sentry', async ({ page }) => { ); }); -test('captures a navigation transcation to Sentry', async ({ page }) => { +test('captures a navigation transaction to Sentry', async ({ page }) => { const clientNavigationTxnEventPromise = waitForTransaction('create-next-app', txnEvent => { return txnEvent?.transaction === '/user/[id]'; }); @@ -53,7 +53,7 @@ test('captures a navigation transcation to Sentry', async ({ page }) => { // navigation to page const clickPromise = page.getByText('navigate').click(); - const [clientTxnEvent, serverTxnEvent, _1] = await Promise.all([clientNavigationTxnEventPromise, clickPromise]); + const [clientTxnEvent, serverTxnEvent] = await Promise.all([clientNavigationTxnEventPromise, clickPromise]); expect(clientTxnEvent).toEqual( expect.objectContaining({ @@ -76,6 +76,16 @@ test('captures a navigation transcation to Sentry', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], }, }, request: { diff --git a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts index c35d731915d6..ee0c507076fa 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-browser-router/tests/transactions.test.ts @@ -58,6 +58,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -145,6 +155,16 @@ test('Captures a lazy navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts index 7abb269d15b0..920506838080 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts @@ -122,6 +122,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts index 7c75c395c3af..61a583a7bf55 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-memory-router/tests/transactions.test.ts @@ -55,6 +55,16 @@ test('Captures a navigation transaction', async ({ page }) => { 'sentry.sample_rate': 1, 'sentry.source': 'route', }), + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], op: 'navigation', span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index f34a37542d29..062b308527d6 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -24,6 +24,7 @@ import { getDynamicSamplingContextFromSpan, getIsolationScope, getLocationHref, + getRootSpan, logger, propagationContextFromHeaders, registerSpanErrorInstrumentation, @@ -35,6 +36,12 @@ import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; import { registerBackgroundTabDetection } from './backgroundtab'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; +import type { PreviousTraceInfo } from './previousTrace'; +import { + addPreviousTraceSpanLink, + getPreviousTraceFromSessionStorage, + storePreviousTraceInSessionStorage, +} from './previousTrace'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; @@ -142,6 +149,29 @@ export interface BrowserTracingOptions { */ enableHTTPTimings: boolean; + /** + * Link the currently started trace to a previous trace (e.g. a prior pageload, navigation or + * manually started span). When enabled, this option will allow you to navigate between traces + * in the Sentry UI. + * + * You can set this option to the following values: + * + * - `'in-memory'`: The previous trace data will be stored in memory. + * This is useful for single-page applications and enabled by default. + * + * - `'session-storage'`: The previous trace data will be stored in the `sessionStorage`. + * This is useful for multi-page applications or static sites but it means that the + * Sentry SDK writes to the browser's `sessionStorage`. + * + * - `'off'`: The previous trace data will not be stored or linked. + * + * Note that your `tracesSampleRate` or `tracesSampler` config significantly influences + * how often traces will be linked. + * + * @default 'in-memory' - see explanation above + */ + linkPreviousTrace: 'in-memory' | 'session-storage' | 'off'; + /** * _experiments allows the user to send options to define how this integration works. * @@ -175,6 +205,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { enableLongTask: true, enableLongAnimationFrame: true, enableInp: true, + linkPreviousTrace: 'in-memory', _experiments: {}, ...defaultRequestInstrumentationOptions, }; @@ -214,6 +245,7 @@ export const browserTracingIntegration = ((_options: Partial { + if (getRootSpan(span) !== span) { + return; + } + + if (linkPreviousTrace === 'session-storage') { + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(getPreviousTraceFromSessionStorage(), span); + storePreviousTraceInSessionStorage(updatedPreviousTraceInfo); + } else { + inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span); + } + }); + } + if (WINDOW.location) { if (instrumentPageLoad) { const origin = browserPerformanceTimeOrigin(); diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts new file mode 100644 index 000000000000..36b6936abf3d --- /dev/null +++ b/packages/browser/src/tracing/previousTrace.ts @@ -0,0 +1,103 @@ +import type { Span } from '@sentry/core'; +import { logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, spanToJSON, type SpanContextData } from '@sentry/core'; +import { WINDOW } from '../exports'; +import { DEBUG_BUILD } from '../debug-build'; + +export interface PreviousTraceInfo { + /** + * Span context of the previous trace's local root span + */ + spanContext: SpanContextData; + + /** + * Timestamp in seconds when the previous trace was started + */ + startTimestamp: number; +} + +// 1h in seconds +export const PREVIOUS_TRACE_MAX_DURATION = 216_000; + +// session storage key +export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; + +/** + * Adds a previous_trace span link to the passed span if the passed + * previousTraceInfo is still valid. + * + * @returns the updated previous trace info (based on the current span/trace) to + * be used on the next call + */ +export function addPreviousTraceSpanLink( + previousTraceInfo: PreviousTraceInfo | undefined, + span: Span, +): PreviousTraceInfo { + const spanJson = spanToJSON(span); + + if (!previousTraceInfo) { + return { + spanContext: span.spanContext(), + startTimestamp: spanJson.start_timestamp, + }; + } + + if (previousTraceInfo.spanContext.traceId === spanJson.trace_id) { + // This means, we're still in the same trace so let's not update the previous trace info + // or add a link to the current span. + // Once we move away from the long-lived, route-based trace model, we can remove this cases + return previousTraceInfo; + } + + // Only add the link if the startTimeStamp of the previous trace's root span is within + // PREVIOUS_TRACE_MAX_DURATION (1h) of the current root span's startTimestamp + // This is done to + // - avoid adding links to "stale" traces + // - enable more efficient querying for previous/next traces in Sentry + if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) { + if (DEBUG_BUILD) { + logger.info( + `Adding previous_trace ${previousTraceInfo.spanContext} link to span ${{ + op: spanJson.op, + ...span.spanContext(), + }}`, + ); + } + + span.addLink({ + context: previousTraceInfo.spanContext, + attributes: { + [SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace', + }, + }); + } + + return { + spanContext: span.spanContext(), + startTimestamp: spanToJSON(span).start_timestamp, + }; +} + +/** + * Stores @param previousTraceInfo in sessionStorage. + */ +export function storePreviousTraceInSessionStorage(previousTraceInfo: PreviousTraceInfo): void { + try { + WINDOW.sessionStorage.setItem(PREVIOUS_TRACE_KEY, JSON.stringify(previousTraceInfo)); + } catch (e) { + // Ignore potential errors (e.g. if sessionStorage is not available) + DEBUG_BUILD && logger.warn('Could not store previous trace in sessionStorage', e); + } +} + +/** + * Retrieves the previous trace from sessionStorage if available. + */ +export function getPreviousTraceFromSessionStorage(): PreviousTraceInfo | undefined { + try { + const previousTraceInfo = WINDOW.sessionStorage?.getItem(PREVIOUS_TRACE_KEY); + // @ts-expect-error - intentionally risking JSON.parse throwing when previousTraceInfo is null to save bundle size + return JSON.parse(previousTraceInfo); + } catch (e) { + return undefined; + } +} diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 0b659332df99..ee43935cd531 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -203,6 +203,16 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span?.spanContext().spanId, + trace_id: span?.spanContext().traceId, + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -230,6 +240,16 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span2?.spanContext().spanId, + trace_id: span2?.spanContext().traceId, + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -483,6 +503,16 @@ describe('browserTracingIntegration', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom', }, + links: [ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -494,7 +524,13 @@ describe('browserTracingIntegration', () => { const client = new BrowserClient( getDefaultBrowserClientOptions({ tracesSampleRate: 1, - integrations: [browserTracingIntegration({ instrumentNavigation: false })], + integrations: [ + browserTracingIntegration({ + instrumentNavigation: false, + // disabling previous trace b/c not relevant for this test + linkPreviousTrace: 'off', + }), + ], }), ); setCurrentClient(client); @@ -992,6 +1028,75 @@ describe('browserTracingIntegration', () => { }); }); + describe('linkPreviousTrace', () => { + it('registers the previous trace listener on span start by default', () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })], + }), + ); + setCurrentClient(client); + client.init(); + + const span1 = startInactiveSpan({ name: 'test span 1', forceTransaction: true }); + span1.end(); + const span1Json = spanToJSON(span1); + + expect(span1Json.links).toBeUndefined(); + + // ensure we start a new trace + getCurrentScope().setPropagationContext({ traceId: '123', sampleRand: 0.2 }); + + const span2 = startInactiveSpan({ name: 'test span 2', forceTransaction: true }); + span2.end(); + const spanJson2 = spanToJSON(span2); + + expect(spanJson2.links).toEqual([ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + sampled: true, + span_id: span1Json.span_id, + trace_id: span1Json.trace_id, + }, + ]); + }); + + it("doesn't register the previous trace listener on span start if disabled", () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [ + browserTracingIntegration({ + instrumentPageLoad: false, + instrumentNavigation: false, + linkPreviousTrace: 'off', + }), + ], + }), + ); + setCurrentClient(client); + client.init(); + + const span1 = startInactiveSpan({ name: 'test span 1', forceTransaction: true }); + span1.end(); + const span1Json = spanToJSON(span1); + + expect(span1Json.links).toBeUndefined(); + + // ensure we start a new trace + getCurrentScope().setPropagationContext({ traceId: '123', sampleRand: 0.2 }); + + const span2 = startInactiveSpan({ name: 'test span 2', forceTransaction: true }); + span2.end(); + const spanJson2 = spanToJSON(span2); + + expect(spanJson2.links).toBeUndefined(); + }); + }); + // TODO(lforst): I cannot manage to get this test to pass. /* it('heartbeatInterval can be a custom value', () => { diff --git a/packages/browser/test/tracing/previousTrace.test.ts b/packages/browser/test/tracing/previousTrace.test.ts new file mode 100644 index 000000000000..f5815cbedc68 --- /dev/null +++ b/packages/browser/test/tracing/previousTrace.test.ts @@ -0,0 +1,225 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type { PreviousTraceInfo } from '../../src/tracing/previousTrace'; +import { + addPreviousTraceSpanLink, + getPreviousTraceFromSessionStorage, + PREVIOUS_TRACE_KEY, + PREVIOUS_TRACE_MAX_DURATION, +} from '../../src/tracing/previousTrace'; +import { SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core'; +import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace'; + +describe('addPreviousTraceSpanLink', () => { + it(`adds a previous_trace span link to startSpanOptions if the previous trace was created within ${PREVIOUS_TRACE_MAX_DURATION}s`, () => { + const currentSpanStart = timestampInSeconds(); + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + // max time reached almost exactly + startTimestamp: currentSpanStart - PREVIOUS_TRACE_MAX_DURATION + 1, + }; + + const currentSpan = new SentrySpan({ + name: 'test', + startTimestamp: currentSpanStart, + parentSpanId: '789', + spanId: 'abc', + traceId: 'def', + sampled: true, + }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); + + expect(spanToJSON(currentSpan).links).toEqual([ + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + trace_id: '123', + span_id: '456', + sampled: true, + }, + ]); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it(`doesn't add a previous_trace span link if the previous trace was created more than ${PREVIOUS_TRACE_MAX_DURATION}s ago`, () => { + const currentSpanStart = timestampInSeconds(); + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 0, + }, + startTimestamp: Date.now() / 1000 - PREVIOUS_TRACE_MAX_DURATION - 1, + }; + + const currentSpan = new SentrySpan({ + name: '/dashboard', + startTimestamp: currentSpanStart, + }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); + + expect(spanToJSON(currentSpan).links).toBeUndefined(); + + // but still updates the previousTraceInfo to the current span + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it("doesn't overwrite previously existing span links", () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + const currentSpanStart = timestampInSeconds(); + + const currentSpan = new SentrySpan({ + name: '/dashboard', + links: [ + { + context: { + traceId: '789', + spanId: '101', + traceFlags: 1, + }, + attributes: { + someKey: 'someValue', + }, + }, + ], + startTimestamp: currentSpanStart, + }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); + + expect(spanToJSON(currentSpan).links).toEqual([ + { + trace_id: '789', + span_id: '101', + sampled: true, + attributes: { + someKey: 'someValue', + }, + }, + { + attributes: { + 'sentry.link.type': 'previous_trace', + }, + trace_id: '123', + span_id: '456', + sampled: true, + }, + ]); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it("doesn't add a link and returns the current span's data as previous trace info, if previous trace info was undefined", () => { + const currentSpanStart = timestampInSeconds(); + const currentSpan = new SentrySpan({ name: 'test', startTimestamp: currentSpanStart }); + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(undefined, currentSpan); + + expect(spanToJSON(currentSpan).links).toBeUndefined(); + + expect(updatedPreviousTraceInfo).toEqual({ + spanContext: currentSpan.spanContext(), + startTimestamp: currentSpanStart, + }); + }); + + it("doesn't add a link and returns the unchanged previous trace info if the current span is part of the same trace", () => { + const currentSpanStart = timestampInSeconds(); + const currentSpan = new SentrySpan({ + name: 'test', + startTimestamp: currentSpanStart, + traceId: '123', + spanId: '456', + }); + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: currentSpanStart - 1, + }; + + const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan); + + expect(spanToJSON(currentSpan).links).toBeUndefined(); + + expect(updatedPreviousTraceInfo).toBe(previousTraceInfo); + }); +}); + +describe('store and retrieve previous trace data via sessionStorage ', () => { + const storage: Record = {}; + const sessionStorageMock = { + setItem: vi.fn((key, value) => { + storage[key] = value; + }), + getItem: vi.fn(key => storage[key]), + }; + + beforeEach(() => { + vi.clearAllMocks(); + // @ts-expect-error - mock contains only necessary API + globalThis.sessionStorage = sessionStorageMock; + }); + + it('stores the previous trace info in sessionStorage', () => { + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + storePreviousTraceInSessionStorage(previousTraceInfo); + expect(sessionStorageMock.setItem).toHaveBeenCalledWith(PREVIOUS_TRACE_KEY, JSON.stringify(previousTraceInfo)); + expect(getPreviousTraceFromSessionStorage()).toEqual(previousTraceInfo); + }); + + it("doesn't throw if accessing sessionStorage fails and returns undefined", () => { + // @ts-expect-error - this is fine + globalThis.sessionStorage = undefined; + + const previousTraceInfo: PreviousTraceInfo = { + spanContext: { + traceId: '123', + spanId: '456', + traceFlags: 1, + }, + startTimestamp: Date.now() / 1000, + }; + + expect(() => storePreviousTraceInSessionStorage(previousTraceInfo)).not.toThrow(); + expect(getPreviousTraceFromSessionStorage).not.toThrow(); + expect(getPreviousTraceFromSessionStorage()).toBeUndefined(); + }); +}); diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index dea57836d3bc..aa25b70f7304 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -57,3 +57,15 @@ export const SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE = 'cache.item_size'; /** TODO: Remove these once we update to latest semantic conventions */ export const SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD = 'http.request.method'; export const SEMANTIC_ATTRIBUTE_URL_FULL = 'url.full'; + +/** + * A span link attribute to mark the link as a special span link. + * + * Known values: + * - `previous_trace`: The span links to the frontend root span of the previous trace. + * - `next_trace`: The span links to the frontend root span of the next trace. (Not set by the SDK) + * + * Other values may be set as appropriate. + * @see https://develop.sentry.dev/sdk/telemetry/traces/span-links/#link-types + */ +export const SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE = 'sentry.link.type'; From 0c21efaafc00cc712a1fcc479a2243ca3029d563 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 14:23:12 +0100 Subject: [PATCH 03/15] test(e2e): Fix failing nextjs-t3 e2e test app (#15785) https://github.com/trpc/trpc/pull/6617 removed an unstable API we used in the app. I just switched it over to use the stable replacement --- .../e2e-tests/test-applications/nextjs-t3/src/trpc/react.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/react.tsx b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/react.tsx index 12459d66eee6..7e6f2fa09c49 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/react.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/react.tsx @@ -1,7 +1,7 @@ 'use client'; import { type QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { loggerLink, unstable_httpBatchStreamLink } from '@trpc/client'; +import { loggerLink, httpBatchStreamLink } from '@trpc/client'; import { createTRPCReact } from '@trpc/react-query'; import { type inferRouterInputs, type inferRouterOutputs } from '@trpc/server'; import { useState } from 'react'; @@ -46,7 +46,7 @@ export function TRPCReactProvider(props: { children: React.ReactNode }) { enabled: op => process.env.NODE_ENV === 'development' || (op.direction === 'down' && op.result instanceof Error), }), - unstable_httpBatchStreamLink({ + httpBatchStreamLink({ transformer: SuperJSON, url: getBaseUrl() + '/api/trpc', headers: () => { From e242c17148bcfb762c2dda6ada8ef3b681378cef Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 21 Mar 2025 14:45:23 +0100 Subject: [PATCH 04/15] fix(browser): Fix incorrect previous trace max duration (#15782) --- packages/browser/src/tracing/previousTrace.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/tracing/previousTrace.ts b/packages/browser/src/tracing/previousTrace.ts index 36b6936abf3d..fd968c5a5cc3 100644 --- a/packages/browser/src/tracing/previousTrace.ts +++ b/packages/browser/src/tracing/previousTrace.ts @@ -16,7 +16,7 @@ export interface PreviousTraceInfo { } // 1h in seconds -export const PREVIOUS_TRACE_MAX_DURATION = 216_000; +export const PREVIOUS_TRACE_MAX_DURATION = 3600; // session storage key export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace'; From 67db14b8993db8f08d313c4e77282512e4c05053 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 21 Mar 2025 14:57:16 +0100 Subject: [PATCH 05/15] feat(nextjs): Support `instrumentation-client.ts` (#15705) --- .../nextjs-turbo/app/layout.tsx | 7 +-- .../app/pageload-transaction/page.tsx | 3 + .../nextjs-turbo/instrumentation-client.ts | 9 +++ .../nextjs-turbo/package.json | 2 +- .../nextjs-turbo/pages/_app.tsx | 1 - .../nextjs-turbo/sentry.client.config.ts | 17 ------ .../app-router/pageload-transaction.test.ts | 27 +++++++++ .../client-trace-propagation.test.ts | 11 ++++ packages/nextjs/src/config/types.ts | 1 + packages/nextjs/src/config/webpack.ts | 42 +++++++++++++- .../nextjs/src/config/withSentryConfig.ts | 55 +++++++++++++++++-- 11 files changed, 141 insertions(+), 34 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-turbo/app/pageload-transaction/page.tsx create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts delete mode 100644 dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/app-router/pageload-transaction.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx b/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx index 999836e58b3b..c8f9cee0b787 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx @@ -1,12 +1,7 @@ -import { HackComponentToRunSideEffectsInSentryClientConfig } from '../sentry.client.config'; - export default function Layout({ children }: { children: React.ReactNode }) { return ( - - - {children} - + {children} ); } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/pageload-transaction/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/pageload-transaction/page.tsx new file mode 100644 index 000000000000..4d692cbabd9b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/pageload-transaction/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return

Hello World!

; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts new file mode 100644 index 000000000000..85bd765c9c44 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, + sendDefaultPii: true, +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/package.json b/dev-packages/e2e-tests/test-applications/nextjs-turbo/package.json index d2cdcfb89561..243c719da4c2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/package.json @@ -17,7 +17,7 @@ "@types/node": "^18.19.1", "@types/react": "18.0.26", "@types/react-dom": "18.0.9", - "next": "15.0.0", + "next": "15.3.0-canary.8", "react": "rc", "react-dom": "rc", "typescript": "~5.0.0" diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx b/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx index 6b90ee6bc586..7f0b03d2959a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx @@ -1,5 +1,4 @@ import type { AppProps } from 'next/app'; -import '../sentry.client.config'; export default function CustomApp({ Component, pageProps }: AppProps) { return ; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts deleted file mode 100644 index 7a49f1b55e11..000000000000 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts +++ /dev/null @@ -1,17 +0,0 @@ -'use client'; - -import * as Sentry from '@sentry/nextjs'; - -if (typeof window !== 'undefined') { - Sentry.init({ - environment: 'qa', // dynamic sampling bias to keep transactions - dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, - tunnel: `http://localhost:3031/`, // proxy server - tracesSampleRate: 1.0, - sendDefaultPii: true, - }); -} - -export function HackComponentToRunSideEffectsInSentryClientConfig() { - return null; -} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/app-router/pageload-transaction.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/app-router/pageload-transaction.test.ts new file mode 100644 index 000000000000..62a072b4ae7f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/app-router/pageload-transaction.test.ts @@ -0,0 +1,27 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { parseSemver } from '@sentry/core'; + +const packageJson = require('../../package.json'); +const nextjsVersion = packageJson.dependencies.next; +const { major, minor } = parseSemver(nextjsVersion); + +test('Should record pageload transactions (this test verifies that the client SDK is initialized)', async ({ + page, +}) => { + // TODO: Remove this skippage when Next.js 15.3.0 is released and bump version in package json to 15.3.0 + test.skip( + major === 15 && minor !== undefined && minor < 3, + 'Next.js version does not support clientside instrumentation', + ); + + const pageloadTransactionPromise = waitForTransaction('nextjs-turbo', async transactionEvent => { + return transactionEvent?.transaction === '/pageload-transaction'; + }); + + await page.goto(`/pageload-transaction`); + + const pageloadTransaction = await pageloadTransactionPromise; + + expect(pageloadTransaction).toBeDefined(); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts index 20a9181d7f8e..52e492b3f234 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts @@ -1,7 +1,18 @@ import { expect, test } from '@playwright/test'; import { waitForTransaction } from '@sentry-internal/test-utils'; +import { parseSemver } from '@sentry/core'; + +const packageJson = require('../../package.json'); +const nextjsVersion = packageJson.dependencies.next; +const { major, minor } = parseSemver(nextjsVersion); test('Should propagate traces from server to client in pages router', async ({ page }) => { + // TODO: Remove this skippage when Next.js 15.3.0 is released and bump version in package json to 15.3.0 + test.skip( + major === 15 && minor !== undefined && minor < 3, + 'Next.js version does not support clientside instrumentation', + ); + const serverTransactionPromise = waitForTransaction('nextjs-turbo', async transactionEvent => { return transactionEvent?.transaction === 'GET /[param]/pages-router-client-trace-propagation'; }); diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts index 965233d08b76..95c15b887573 100644 --- a/packages/nextjs/src/config/types.ts +++ b/packages/nextjs/src/config/types.ts @@ -44,6 +44,7 @@ export type NextConfigObject = { // Next.js experimental options experimental?: { instrumentationHook?: boolean; + clientInstrumentationHook?: boolean; clientTraceMetadata?: string[]; }; productionBrowserSourceMaps?: boolean; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index de047c0b8cf4..cababfa63002 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -339,6 +339,14 @@ export function constructWebpackConfigFunction( // be fixed by using `bind`, but this is way simpler.) const origEntryProperty = newConfig.entry; newConfig.entry = async () => addSentryToClientEntryProperty(origEntryProperty, buildContext); + + const clientSentryConfigFileName = getClientSentryConfigFile(projectDir); + if (clientSentryConfigFileName) { + // eslint-disable-next-line no-console + console.warn( + `[@sentry/nextjs] DEPRECATION WARNING: It is recommended renaming your \`${clientSentryConfigFileName}\` file, or moving its content to \`instrumentation-client.ts\`. When using Turbopack \`${clientSentryConfigFileName}\` will no longer work. Read more about the \`instrumentation-client.ts\` file: https://nextjs.org/docs/app/api-reference/config/next-config-js/clientInstrumentationHook`, + ); + } } // We don't want to do any webpack plugin stuff OR any source maps stuff in dev mode. @@ -430,9 +438,17 @@ async function addSentryToClientEntryProperty( typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty }; const clientSentryConfigFileName = getClientSentryConfigFile(projectDir); + const instrumentationClientFileName = getInstrumentationClientFile(projectDir); - // we need to turn the filename into a path so webpack can find it - const filesToInject = clientSentryConfigFileName ? [`./${clientSentryConfigFileName}`] : []; + const filesToInject = []; + if (clientSentryConfigFileName) { + // we need to turn the filename into a path so webpack can find it + filesToInject.push(`./${clientSentryConfigFileName}`); + } + if (instrumentationClientFileName) { + // we need to turn the filename into a path so webpack can find it + filesToInject.push(`./${instrumentationClientFileName}`); + } // inject into all entry points which might contain user's code for (const entryPointName in newEntryProperty) { @@ -530,7 +546,7 @@ function warnAboutDeprecatedConfigFiles( * * @param projectDir The root directory of the project, where config files would be located */ -export function getClientSentryConfigFile(projectDir: string): string | void { +function getClientSentryConfigFile(projectDir: string): string | void { const possibilities = ['sentry.client.config.ts', 'sentry.client.config.js']; for (const filename of possibilities) { @@ -540,6 +556,26 @@ export function getClientSentryConfigFile(projectDir: string): string | void { } } +/** + * Searches for a `instrumentation-client.ts|js` file and returns its file name if it finds one. (ts being prioritized) + * + * @param projectDir The root directory of the project, where config files would be located + */ +function getInstrumentationClientFile(projectDir: string): string | void { + const possibilities = [ + ['src', 'instrumentation-client.js'], + ['src', 'instrumentation-client.ts'], + ['instrumentation-client.js'], + ['instrumentation-client.ts'], + ]; + + for (const pathParts of possibilities) { + if (fs.existsSync(path.resolve(projectDir, ...pathParts))) { + return path.join(...pathParts); + } + } +} + /** * Add files to a specific element of the given `entry` webpack config property. * diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index e811697cbe86..7250af15ff8e 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -153,14 +153,57 @@ function getFinalConfigObject( } } - if (process.env.TURBOPACK && !process.env.SENTRY_SUPPRESS_TURBOPACK_WARNING) { + if (nextJsVersion) { + const { major, minor, patch, prerelease } = parseSemver(nextJsVersion); + const isSupportedVersion = + major !== undefined && + minor !== undefined && + patch !== undefined && + (major > 15 || + (major === 15 && minor > 3) || + (major === 15 && minor === 3 && patch > 0 && prerelease === undefined)); + const isSupportedCanary = + major !== undefined && + minor !== undefined && + patch !== undefined && + prerelease !== undefined && + major === 15 && + minor === 3 && + patch === 0 && + prerelease.startsWith('canary.') && + parseInt(prerelease.split('.')[1] || '', 10) >= 8; + const supportsClientInstrumentation = isSupportedCanary || isSupportedVersion; + + if (supportsClientInstrumentation) { + incomingUserNextConfigObject.experimental = { + clientInstrumentationHook: true, + ...incomingUserNextConfigObject.experimental, + }; + } else if (process.env.TURBOPACK) { + if (process.env.NODE_ENV === 'development') { + // eslint-disable-next-line no-console + console.warn( + `[@sentry/nextjs] WARNING: You are using the Sentry SDK with Turbopack (\`next dev --turbo\`). The Sentry SDK is compatible with Turbopack on Next.js version 15.3.0 or later. You are currently on ${nextJsVersion}. Please upgrade to a newer Next.js version to use the Sentry SDK with Turbopack. Note that the SDK will continue to work for non-Turbopack production builds. This warning is only about dev-mode.`, + ); + } else if (process.env.NODE_ENV === 'production') { + // eslint-disable-next-line no-console + console.warn( + `[@sentry/nextjs] WARNING: You are using the Sentry SDK with Turbopack (\`next build --turbo\`). The Sentry SDK is compatible with Turbopack on Next.js version 15.3.0 or later. You are currently on ${nextJsVersion}. Please upgrade to a newer Next.js version to use the Sentry SDK with Turbopack. Note that as Turbopack is still experimental for production builds, some of the Sentry SDK features like source maps will not work. Follow this issue for progress on Sentry + Turbopack: https://github.com/getsentry/sentry-javascript/issues/8105.`, + ); + } + } + } else { + // If we cannot detect a Next.js version for whatever reason, the sensible default is still to set the `experimental.instrumentationHook`. + incomingUserNextConfigObject.experimental = { + clientInstrumentationHook: true, + ...incomingUserNextConfigObject.experimental, + }; + } + + if (incomingUserNextConfigObject.experimental?.clientInstrumentationHook === false) { // eslint-disable-next-line no-console console.warn( - `[@sentry/nextjs] WARNING: You are using the Sentry SDK with \`next ${ - process.env.NODE_ENV === 'development' ? 'dev' : 'build' - } --turbo\`. The Sentry SDK doesn't yet fully support Turbopack. The SDK will not be loaded in the browser, and serverside instrumentation will be inaccurate or incomplete. ${ - process.env.NODE_ENV === 'development' ? 'Production builds without `--turbo` will still fully work. ' : '' - }If you are just trying out Sentry or attempting to configure the SDK, we recommend temporarily removing the \`--turbo\` flag while you are developing locally. Follow this issue for progress on Sentry + Turbopack: https://github.com/getsentry/sentry-javascript/issues/8105. (You can suppress this warning by setting SENTRY_SUPPRESS_TURBOPACK_WARNING=1 as environment variable)`, + '[@sentry/nextjs] WARNING: You set the `experimental.clientInstrumentationHook` option to `false`. Note that Sentry will not be initialized if you did not set it up inside `instrumentation-client.(js|ts)`.', ); } From ab161235d27ae0dca2da42031ff085cc33a7d0e2 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 21 Mar 2025 15:00:01 +0100 Subject: [PATCH 06/15] feat(core): Optimize `dropUndefinedKeys` (#15760) Updates the implementation of `dropUndefinedKeys`: - added early returns - used for loops in case of larger data structures - handle array case before object case to avoid calls to `isPojo` - simplified `isPojo` by checking [Object.prototype.constructor](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/constructor#description) ref https://github.com/getsentry/sentry-javascript/issues/15725 --- packages/core/src/utils-hoist/object.ts | 71 ++++++++++++------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/packages/core/src/utils-hoist/object.ts b/packages/core/src/utils-hoist/object.ts index 2c9080682a9d..7826e960d982 100644 --- a/packages/core/src/utils-hoist/object.ts +++ b/packages/core/src/utils-hoist/object.ts @@ -3,7 +3,7 @@ import type { WrappedFunction } from '../types-hoist'; import { htmlTreeAsString } from './browser'; import { DEBUG_BUILD } from './debug-build'; -import { isElement, isError, isEvent, isInstanceOf, isPlainObject, isPrimitive } from './is'; +import { isElement, isError, isEvent, isInstanceOf, isPrimitive } from './is'; import { logger } from './logger'; import { truncate } from './string'; @@ -222,58 +222,55 @@ export function dropUndefinedKeys(inputValue: T): T { } function _dropUndefinedKeys(inputValue: T, memoizationMap: Map): T { - if (isPojo(inputValue)) { - // If this node has already been visited due to a circular reference, return the object it was mapped to in the new object - const memoVal = memoizationMap.get(inputValue); - if (memoVal !== undefined) { - return memoVal as T; - } - - const returnValue: { [key: string]: unknown } = {}; - // Store the mapping of this value in case we visit it again, in case of circular data - memoizationMap.set(inputValue, returnValue); - - for (const key of Object.getOwnPropertyNames(inputValue)) { - if (typeof inputValue[key] !== 'undefined') { - returnValue[key] = _dropUndefinedKeys(inputValue[key], memoizationMap); - } - } + // Early return for primitive values + if (inputValue === null || typeof inputValue !== 'object') { + return inputValue; + } - return returnValue as T; + // Check memo map first for all object types + const memoVal = memoizationMap.get(inputValue); + if (memoVal !== undefined) { + return memoVal as T; } + // handle arrays if (Array.isArray(inputValue)) { - // If this node has already been visited due to a circular reference, return the array it was mapped to in the new object - const memoVal = memoizationMap.get(inputValue); - if (memoVal !== undefined) { - return memoVal as T; - } - const returnValue: unknown[] = []; - // Store the mapping of this value in case we visit it again, in case of circular data + // Store mapping to handle circular references memoizationMap.set(inputValue, returnValue); - inputValue.forEach((item: unknown) => { - returnValue.push(_dropUndefinedKeys(item, memoizationMap)); + inputValue.forEach(value => { + returnValue.push(_dropUndefinedKeys(value, memoizationMap)); }); return returnValue as unknown as T; } + if (isPojo(inputValue)) { + const returnValue: { [key: string]: unknown } = {}; + // Store mapping to handle circular references + memoizationMap.set(inputValue, returnValue); + + const keys = Object.keys(inputValue); + + keys.forEach(key => { + const val = inputValue[key]; + if (val !== undefined) { + returnValue[key] = _dropUndefinedKeys(val, memoizationMap); + } + }); + + return returnValue as T; + } + + // For other object types, return as is return inputValue; } function isPojo(input: unknown): input is Record { - if (!isPlainObject(input)) { - return false; - } - - try { - const name = (Object.getPrototypeOf(input) as { constructor: { name: string } }).constructor.name; - return !name || name === 'Object'; - } catch { - return true; - } + // Plain objects have Object as constructor or no constructor + const constructor = (input as object).constructor; + return constructor === Object || constructor === undefined; } /** From 75f7b93f9487245ed0d4cf52c68b23aded0fd118 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 21 Mar 2025 10:45:31 -0400 Subject: [PATCH 07/15] perf(nestjs): Remove usage of addNonEnumerableProperty (#15766) ref https://github.com/getsentry/sentry-javascript/issues/15725#issuecomment-2741985510 Similar to the work done in https://github.com/getsentry/sentry-javascript/pull/15765 we can avoid usage of `addNonEnumerableProperty` with usage of a `WeakSet`. --- .../src/integrations/sentry-nest-instrumentation.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/nestjs/src/integrations/sentry-nest-instrumentation.ts b/packages/nestjs/src/integrations/sentry-nest-instrumentation.ts index c4f36728c906..58060f844888 100644 --- a/packages/nestjs/src/integrations/sentry-nest-instrumentation.ts +++ b/packages/nestjs/src/integrations/sentry-nest-instrumentation.ts @@ -8,7 +8,6 @@ import { import type { Span } from '@sentry/core'; import { SDK_VERSION, - addNonEnumerableProperty, getActiveSpan, isThenable, startInactiveSpan, @@ -90,7 +89,9 @@ export class SentryNestInstrumentation extends InstrumentationBase { /** * Creates a wrapper function for the @Injectable decorator. */ - private _createWrapInjectable() { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _createWrapInjectable(): (original: any) => (options?: unknown) => (target: InjectableTarget) => any { + const SeenNestjsContextSet = new WeakSet(); // eslint-disable-next-line @typescript-eslint/no-explicit-any return function wrapInjectable(original: any) { return function wrappedInjectable(options?: unknown) { @@ -197,8 +198,8 @@ export class SentryNestInstrumentation extends InstrumentationBase { return withActiveSpan(parentSpan, () => { const handleReturnObservable = Reflect.apply(originalHandle, thisArgHandle, argsHandle); - if (!context._sentryInterceptorInstrumented) { - addNonEnumerableProperty(context, '_sentryInterceptorInstrumented', true); + if (!SeenNestjsContextSet.has(context)) { + SeenNestjsContextSet.add(context); afterSpan = startInactiveSpan( getMiddlewareSpanOptions(target, 'Interceptors - After Route'), ); @@ -209,8 +210,8 @@ export class SentryNestInstrumentation extends InstrumentationBase { } else { const handleReturnObservable = Reflect.apply(originalHandle, thisArgHandle, argsHandle); - if (!context._sentryInterceptorInstrumented) { - addNonEnumerableProperty(context, '_sentryInterceptorInstrumented', true); + if (!SeenNestjsContextSet.has(context)) { + SeenNestjsContextSet.add(context); afterSpan = startInactiveSpan(getMiddlewareSpanOptions(target, 'Interceptors - After Route')); } From e55b8ee049923c4e17ed79df492495cfc2f47726 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 21 Mar 2025 10:46:12 -0400 Subject: [PATCH 08/15] feat(browser): Add logger.X methods to browser SDK (#15763) ref https://github.com/getsentry/sentry-javascript/issues/15526 Continuing off the work from https://github.com/getsentry/sentry-javascript/pull/15717, this PR adds the logging public API to the Browser SDK. It also adds a basic flushing strategy to the SDK that is timeout based. This is done to help save bundle size. The main file added was `log.ts`. This has three areas to look at: 1. The logger methods for `trace`, `debug`, `info`, `warn`, `error`, `fatal` (the log severity levels) as well as an internal capture log helper all these methods call. 2. `addFlushingListeners` which adds listeners to flush the logs buffer on client flush and document visibility hidden. 3. a flush timeout that flushes logs after X seconds, which gets restarted when new logs are captured. I also removed any logs logic from the `BrowserClient`, which should ensure this stays as bundle size efficient as possible. Usage: ```js import * as Sentry from "@sentry/browser"; Sentry.init({ dsn: "your-dsn-here", _experiments: { enableLogs: true // This is required to use the logging features } }); // Trace level (lowest severity) Sentry.logger.trace("This is a trace message", { userId: 123 }); // Debug level Sentry.logger.debug("This is a debug message", { component: "UserProfile" }); // Info level Sentry.logger.info("User logged in successfully", { userId: 123 }); // Warning level Sentry.logger.warn("API response was slow", { responseTime: 2500 }); // Error level Sentry.logger.error("Failed to load user data", { userId: 123, errorCode: 404 }); // Critical level Sentry.logger.critical("Database connection failed", { dbHost: "primary-db" }); // Fatal level (highest severity) Sentry.logger.fatal("Application is shutting down unexpectedly", { memory: "exhausted" }); ``` --- packages/browser/src/client.ts | 4 - packages/browser/src/index.ts | 4 + packages/browser/src/log.ts | 186 +++++++++++++++++++++ packages/browser/test/index.test.ts | 43 +++-- packages/browser/test/log.test.ts | 200 +++++++++++++++++++++++ packages/core/src/index.ts | 2 +- packages/core/src/logs/index.ts | 2 +- packages/core/test/lib/log/index.test.ts | 20 +-- 8 files changed, 431 insertions(+), 30 deletions(-) create mode 100644 packages/browser/src/log.ts create mode 100644 packages/browser/test/log.test.ts diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index bacf269871a4..0e5b3fb6214c 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -15,7 +15,6 @@ import { addAutoIpAddressToUser, applySdkMetadata, getSDKSource, - _INTERNAL_flushLogsBuffer, } from '@sentry/core'; import { eventFromException, eventFromMessage } from './eventbuilder'; import { WINDOW } from './helpers'; @@ -86,9 +85,6 @@ export class BrowserClient extends Client { WINDOW.document.addEventListener('visibilitychange', () => { if (WINDOW.document.visibilityState === 'hidden') { this._flushOutcomes(); - if (this._options._experiments?.enableLogs) { - _INTERNAL_flushLogsBuffer(this); - } } }); } diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index d034330b6283..47bef0cd6dae 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -1,5 +1,9 @@ export * from './exports'; +import * as logger from './log'; + +export { logger }; + export { reportingObserverIntegration } from './integrations/reportingobserver'; export { httpClientIntegration } from './integrations/httpclient'; export { contextLinesIntegration } from './integrations/contextlines'; diff --git a/packages/browser/src/log.ts b/packages/browser/src/log.ts new file mode 100644 index 000000000000..4ae97fd27a94 --- /dev/null +++ b/packages/browser/src/log.ts @@ -0,0 +1,186 @@ +import type { LogSeverityLevel, Log, Client } from '@sentry/core'; +import { getClient, _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer } from '@sentry/core'; + +import { WINDOW } from './helpers'; + +/** + * TODO: Make this configurable + */ +const DEFAULT_FLUSH_INTERVAL = 5000; + +let timeout: ReturnType | undefined; + +/** + * This is a global timeout that is used to flush the logs buffer. + * It is used to ensure that logs are flushed even if the client is not flushed. + */ +function startFlushTimeout(client: Client): void { + if (timeout) { + clearTimeout(timeout); + } + + timeout = setTimeout(() => { + _INTERNAL_flushLogsBuffer(client); + }, DEFAULT_FLUSH_INTERVAL); +} + +let isClientListenerAdded = false; +/** + * This is a function that is used to add a flush listener to the client. + * It is used to ensure that the logger buffer is flushed when the client is flushed. + */ +function addFlushingListeners(client: Client): void { + if (isClientListenerAdded || !client.getOptions()._experiments?.enableLogs) { + return; + } + + isClientListenerAdded = true; + + if (WINDOW.document) { + WINDOW.document.addEventListener('visibilitychange', () => { + if (WINDOW.document.visibilityState === 'hidden') { + _INTERNAL_flushLogsBuffer(client); + } + }); + } + + client.on('flush', () => { + _INTERNAL_flushLogsBuffer(client); + }); +} + +/** + * Capture a log with the given level. + * + * @param level - The level of the log. + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * @param severityNumber - The severity number of the log. + */ +function captureLog( + level: LogSeverityLevel, + message: string, + attributes?: Log['attributes'], + severityNumber?: Log['severityNumber'], +): void { + const client = getClient(); + if (client) { + addFlushingListeners(client); + + startFlushTimeout(client); + } + + _INTERNAL_captureLog({ level, message, attributes, severityNumber }, client, undefined); +} + +/** + * @summary Capture a log with the `trace` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.trace('Hello world', { userId: 100 }); + * ``` + */ +export function trace(message: string, attributes?: Log['attributes']): void { + captureLog('trace', message, attributes); +} + +/** + * @summary Capture a log with the `debug` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.debug('Hello world', { userId: 100 }); + * ``` + */ +export function debug(message: string, attributes?: Log['attributes']): void { + captureLog('debug', message, attributes); +} + +/** + * @summary Capture a log with the `info` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.info('Hello world', { userId: 100 }); + * ``` + */ +export function info(message: string, attributes?: Log['attributes']): void { + captureLog('info', message, attributes); +} + +/** + * @summary Capture a log with the `warn` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.warn('Hello world', { userId: 100 }); + * ``` + */ +export function warn(message: string, attributes?: Log['attributes']): void { + captureLog('warn', message, attributes); +} + +/** + * @summary Capture a log with the `error` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.error('Hello world', { userId: 100 }); + * ``` + */ +export function error(message: string, attributes?: Log['attributes']): void { + captureLog('error', message, attributes); +} + +/** + * @summary Capture a log with the `fatal` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.fatal('Hello world', { userId: 100 }); + * ``` + */ +export function fatal(message: string, attributes?: Log['attributes']): void { + captureLog('fatal', message, attributes); +} + +/** + * @summary Capture a log with the `critical` level. Requires `_experiments.enableLogs` to be enabled. + * + * @param message - The message to log. + * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100. + * + * @example + * + * ``` + * Sentry.logger.critical('Hello world', { userId: 100 }); + * ``` + */ +export function critical(message: string, attributes?: Log['attributes']): void { + captureLog('critical', message, attributes); +} diff --git a/packages/browser/test/index.test.ts b/packages/browser/test/index.test.ts index 815dd39707f9..f6f66184509f 100644 --- a/packages/browser/test/index.test.ts +++ b/packages/browser/test/index.test.ts @@ -29,6 +29,7 @@ import { getCurrentScope, init, showReportDialog, + logger, } from '../src'; import { getDefaultBrowserClientOptions } from './helper/browser-client-options'; import { makeSimpleTransport } from './mocks/simpletransport'; @@ -243,20 +244,21 @@ describe('SentryBrowser', () => { expect(event.exception.values[0]?.stacktrace.frames).not.toHaveLength(0); }); - it('should capture a message', done => { - const options = getDefaultBrowserClientOptions({ - beforeSend: (event: Event): Event | null => { - expect(event.level).toBe('info'); - expect(event.message).toBe('test'); - expect(event.exception).toBeUndefined(); - done(); - return event; - }, - dsn, - }); - setCurrentClient(new BrowserClient(options)); - captureMessage('test'); - }); + it('should capture an message', () => + new Promise(resolve => { + const options = getDefaultBrowserClientOptions({ + beforeSend: event => { + expect(event.level).toBe('info'); + expect(event.message).toBe('test'); + expect(event.exception).toBeUndefined(); + resolve(); + return event; + }, + dsn, + }); + setCurrentClient(new BrowserClient(options)); + captureMessage('test'); + })); it('should capture an event', () => new Promise(resolve => { @@ -322,6 +324,19 @@ describe('SentryBrowser', () => { expect(localBeforeSend).not.toHaveBeenCalled(); }); }); + + describe('logger', () => { + it('exports all log methods', () => { + expect(logger).toBeDefined(); + expect(logger.trace).toBeDefined(); + expect(logger.debug).toBeDefined(); + expect(logger.info).toBeDefined(); + expect(logger.warn).toBeDefined(); + expect(logger.error).toBeDefined(); + expect(logger.fatal).toBeDefined(); + expect(logger.critical).toBeDefined(); + }); + }); }); describe('SentryBrowser initialization', () => { diff --git a/packages/browser/test/log.test.ts b/packages/browser/test/log.test.ts new file mode 100644 index 000000000000..582cc3b45d20 --- /dev/null +++ b/packages/browser/test/log.test.ts @@ -0,0 +1,200 @@ +/** + * @vitest-environment jsdom + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import * as sentryCore from '@sentry/core'; +import { getGlobalScope, getCurrentScope, getIsolationScope } from '@sentry/core'; + +import { init, logger } from '../src'; +import { makeSimpleTransport } from './mocks/simpletransport'; + +const dsn = 'https://53039209a22b4ec1bcc296a3c9fdecd6@sentry.io/4291'; + +// Mock the core functions +vi.mock('@sentry/core', async requireActual => { + return { + ...((await requireActual()) as any), + _INTERNAL_captureLog: vi.fn(), + _INTERNAL_flushLogsBuffer: vi.fn(), + }; +}); + +describe('Logger', () => { + // Use the mocked functions + const mockCaptureLog = vi.mocked(sentryCore._INTERNAL_captureLog); + const mockFlushLogsBuffer = vi.mocked(sentryCore._INTERNAL_flushLogsBuffer); + + beforeEach(() => { + // Reset mocks + mockCaptureLog.mockClear(); + mockFlushLogsBuffer.mockClear(); + + // Reset the global scope, isolation scope, and current scope + getGlobalScope().clear(); + getIsolationScope().clear(); + getCurrentScope().clear(); + getCurrentScope().setClient(undefined); + + // Mock setTimeout and clearTimeout + vi.useFakeTimers(); + + // Initialize with logs enabled + init({ + dsn, + transport: makeSimpleTransport, + _experiments: { + enableLogs: true, + }, + }); + }); + + afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + describe('Logger methods', () => { + it('should export all log methods', () => { + expect(logger).toBeDefined(); + expect(logger.trace).toBeTypeOf('function'); + expect(logger.debug).toBeTypeOf('function'); + expect(logger.info).toBeTypeOf('function'); + expect(logger.warn).toBeTypeOf('function'); + expect(logger.error).toBeTypeOf('function'); + expect(logger.fatal).toBeTypeOf('function'); + expect(logger.critical).toBeTypeOf('function'); + }); + + it('should call _INTERNAL_captureLog with trace level', () => { + logger.trace('Test trace message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'trace', + message: 'Test trace message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with debug level', () => { + logger.debug('Test debug message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'debug', + message: 'Test debug message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with info level', () => { + logger.info('Test info message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'info', + message: 'Test info message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with warn level', () => { + logger.warn('Test warn message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'warn', + message: 'Test warn message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with error level', () => { + logger.error('Test error message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'error', + message: 'Test error message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with fatal level', () => { + logger.fatal('Test fatal message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'fatal', + message: 'Test fatal message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + + it('should call _INTERNAL_captureLog with critical level', () => { + logger.critical('Test critical message', { key: 'value' }); + expect(mockCaptureLog).toHaveBeenCalledWith( + { + level: 'critical', + message: 'Test critical message', + attributes: { key: 'value' }, + severityNumber: undefined, + }, + expect.any(Object), + undefined, + ); + }); + }); + + describe('Automatic flushing', () => { + it('should flush logs after timeout', () => { + logger.info('Test message'); + expect(mockFlushLogsBuffer).not.toHaveBeenCalled(); + + // Fast-forward time by 5000ms (the default flush interval) + vi.advanceTimersByTime(5000); + + expect(mockFlushLogsBuffer).toHaveBeenCalledTimes(1); + expect(mockFlushLogsBuffer).toHaveBeenCalledWith(expect.any(Object)); + }); + + it('should restart the flush timeout when a new log is captured', () => { + logger.info('First message'); + + // Advance time by 3000ms (not enough to trigger flush) + vi.advanceTimersByTime(3000); + expect(mockFlushLogsBuffer).not.toHaveBeenCalled(); + + // Log another message, which should reset the timer + logger.info('Second message'); + + // Advance time by 3000ms again (should be 6000ms total, but timer was reset) + vi.advanceTimersByTime(3000); + expect(mockFlushLogsBuffer).not.toHaveBeenCalled(); + + // Advance time to complete the 5000ms after the second message + vi.advanceTimersByTime(2000); + expect(mockFlushLogsBuffer).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 53b612fccc29..292a9d5d9f6d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -113,7 +113,7 @@ export { instrumentFetchRequest } from './fetch'; export { trpcMiddleware } from './trpc'; export { captureFeedback } from './feedback'; export type { ReportDialogOptions } from './report-dialog'; -export { _INTERNAL_flushLogsBuffer } from './logs'; +export { _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer } from './logs'; // TODO: Make this structure pretty again and don't do "export *" export * from './utils-hoist/index'; diff --git a/packages/core/src/logs/index.ts b/packages/core/src/logs/index.ts index dcd9cd0abaf6..6c6c9b580ee9 100644 --- a/packages/core/src/logs/index.ts +++ b/packages/core/src/logs/index.ts @@ -62,7 +62,7 @@ export function logAttributeToSerializedLogAttribute(key: string, value: unknown * @experimental This method will experience breaking changes. This is not yet part of * the stable Sentry SDK API and can be changed or removed without warning. */ -export function captureLog(log: Log, scope = getCurrentScope(), client = getClient()): void { +export function _INTERNAL_captureLog(log: Log, client = getClient(), scope = getCurrentScope()): void { if (!client) { DEBUG_BUILD && logger.warn('No client available to capture log.'); return; diff --git a/packages/core/test/lib/log/index.test.ts b/packages/core/test/lib/log/index.test.ts index 2d975abda4e1..ab7cfe9bb4b4 100644 --- a/packages/core/test/lib/log/index.test.ts +++ b/packages/core/test/lib/log/index.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { _INTERNAL_flushLogsBuffer, _INTERNAL_getLogBuffer, - captureLog, + _INTERNAL_captureLog, logAttributeToSerializedLogAttribute, } from '../../../src/logs'; import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; @@ -71,12 +71,12 @@ describe('logAttributeToSerializedLogAttribute', () => { }); }); -describe('captureLog', () => { +describe('_INTERNAL_captureLog', () => { it('captures and sends logs', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } }); const client = new TestClient(options); - captureLog({ level: 'info', message: 'test log message' }, undefined, client); + _INTERNAL_captureLog({ level: 'info', message: 'test log message' }, client, undefined); expect(_INTERNAL_getLogBuffer(client)).toHaveLength(1); expect(_INTERNAL_getLogBuffer(client)?.[0]).toEqual( expect.objectContaining({ @@ -94,7 +94,7 @@ describe('captureLog', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); - captureLog({ level: 'info', message: 'test log message' }, undefined, client); + _INTERNAL_captureLog({ level: 'info', message: 'test log message' }, client, undefined); expect(logWarnSpy).toHaveBeenCalledWith('logging option not enabled, log will not be captured.'); expect(_INTERNAL_getLogBuffer(client)).toBeUndefined(); @@ -111,7 +111,7 @@ describe('captureLog', () => { sampleRand: 1, }); - captureLog({ level: 'error', message: 'test log with trace' }, scope, client); + _INTERNAL_captureLog({ level: 'error', message: 'test log with trace' }, client, scope); expect(_INTERNAL_getLogBuffer(client)?.[0]).toEqual( expect.objectContaining({ @@ -129,7 +129,7 @@ describe('captureLog', () => { }); const client = new TestClient(options); - captureLog({ level: 'info', message: 'test log with metadata' }, undefined, client); + _INTERNAL_captureLog({ level: 'info', message: 'test log with metadata' }, client, undefined); const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; expect(logAttributes).toEqual( @@ -144,14 +144,14 @@ describe('captureLog', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } }); const client = new TestClient(options); - captureLog( + _INTERNAL_captureLog( { level: 'info', message: 'test log with custom attributes', attributes: { userId: '123', component: 'auth' }, }, - undefined, client, + undefined, ); const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; @@ -169,13 +169,13 @@ describe('captureLog', () => { // Fill the buffer to max size (100 is the MAX_LOG_BUFFER_SIZE constant in client.ts) for (let i = 0; i < 100; i++) { - captureLog({ level: 'info', message: `log message ${i}` }, undefined, client); + _INTERNAL_captureLog({ level: 'info', message: `log message ${i}` }, client, undefined); } expect(_INTERNAL_getLogBuffer(client)).toHaveLength(100); // Add one more to trigger flush - captureLog({ level: 'info', message: 'trigger flush' }, undefined, client); + _INTERNAL_captureLog({ level: 'info', message: 'trigger flush' }, client, undefined); expect(_INTERNAL_getLogBuffer(client)).toEqual([]); }); From ba5993caaa2bef98e85e1c2d82ce47e2817bff8c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 21 Mar 2025 11:29:02 -0400 Subject: [PATCH 09/15] feat(core): Add `parseStringToURL` method (#15768) Now that we support ES2020 (aka not IE11 anymore) and Node.js 18+, we can get rid of `parseUrl` in favor of a method that just uses the built-in URL object. This will save us some bundle size (given we can remove that regex), and we get performance benefits from using native code. Instead of just blanket replacing `parseUrl`, we'll slowly convert all it's usages to using a new helper, `parseStringToURL`. Given we are updating the core package, I also went ahead and converted `parseUrl` usage in `packages/core/src/fetch.ts` --- packages/core/src/fetch.ts | 37 ++++++++++++---------- packages/core/src/utils-hoist/url.ts | 27 ++++++++++++++++ packages/core/test/utils-hoist/url.test.ts | 19 ++++++++++- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index a96d421d0023..650cad83effa 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -1,3 +1,4 @@ +/* eslint-disable complexity */ import { getClient } from './currentScopes'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes'; import { SPAN_STATUS_ERROR, setHttpStatus, startInactiveSpan } from './tracing'; @@ -5,7 +6,7 @@ import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan'; import type { FetchBreadcrumbHint, HandlerDataFetch, Span, SpanOrigin } from './types-hoist'; import { SENTRY_BAGGAGE_KEY_PREFIX } from './utils-hoist/baggage'; import { isInstanceOf } from './utils-hoist/is'; -import { parseUrl, stripUrlQueryAndFragment } from './utils-hoist/url'; +import { parseStringToURL, stripUrlQueryAndFragment } from './utils-hoist/url'; import { hasSpansEnabled } from './utils/hasSpansEnabled'; import { getActiveSpan } from './utils/spanUtils'; import { getTraceData } from './utils/traceData'; @@ -53,8 +54,20 @@ export function instrumentFetchRequest( return undefined; } - const fullUrl = getFullURL(url); - const parsedUrl = fullUrl ? parseUrl(fullUrl) : parseUrl(url); + // Curious about `thismessage:/`? See: https://www.rfc-editor.org/rfc/rfc2557.html + // > When the methods above do not yield an absolute URI, a base URL + // > of "thismessage:/" MUST be employed. This base URL has been + // > defined for the sole purpose of resolving relative references + // > within a multipart/related structure when no other base URI is + // > specified. + // + // We need to provide a base URL to `parseStringToURL` because the fetch API gives us a + // relative URL sometimes. + // + // This is the only case where we need to provide a base URL to `parseStringToURL` + // because the relative URL is not valid on its own. + const parsedUrl = url.startsWith('/') ? parseStringToURL(url, 'thismessage:/') : parseStringToURL(url); + const fullUrl = url.startsWith('/') ? undefined : parsedUrl?.href; const hasParent = !!getActiveSpan(); @@ -66,12 +79,13 @@ export function instrumentFetchRequest( url, type: 'fetch', 'http.method': method, - 'http.url': fullUrl, - 'server.address': parsedUrl?.host, + 'http.url': parsedUrl?.href, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', - ...(parsedUrl?.search && { 'http.query': parsedUrl?.search }), - ...(parsedUrl?.hash && { 'http.fragment': parsedUrl?.hash }), + ...(fullUrl && { 'http.url': fullUrl }), + ...(fullUrl && parsedUrl?.host && { 'server.address': parsedUrl.host }), + ...(parsedUrl?.search && { 'http.query': parsedUrl.search }), + ...(parsedUrl?.hash && { 'http.fragment': parsedUrl.hash }), }, }) : new SentryNonRecordingSpan(); @@ -215,15 +229,6 @@ function _addTracingHeadersToFetchRequest( } } -function getFullURL(url: string): string | undefined { - try { - const parsed = new URL(url); - return parsed.href; - } catch { - return undefined; - } -} - function endSpan(span: Span, handlerData: HandlerDataFetch): void { if (handlerData.response) { setHttpStatus(span, handlerData.response.status); diff --git a/packages/core/src/utils-hoist/url.ts b/packages/core/src/utils-hoist/url.ts index e62d22f05e26..0b542cf14d6c 100644 --- a/packages/core/src/utils-hoist/url.ts +++ b/packages/core/src/utils-hoist/url.ts @@ -7,6 +7,33 @@ type PartialURL = { hash?: string; }; +interface URLwithCanParse extends URL { + canParse: (url: string, base?: string | URL | undefined) => boolean; +} + +/** + * Parses string to a URL object + * + * @param url - The URL to parse + * @returns The parsed URL object or undefined if the URL is invalid + */ +export function parseStringToURL(url: string, base?: string | URL | undefined): URL | undefined { + try { + // Use `canParse` to short-circuit the URL constructor if it's not a valid URL + // This is faster than trying to construct the URL and catching the error + // Node 20+, Chrome 120+, Firefox 115+, Safari 17+ + if ('canParse' in URL && !(URL as unknown as URLwithCanParse).canParse(url, base)) { + return undefined; + } + + return new URL(url, base); + } catch { + // empty body + } + + return undefined; +} + /** * Parses string form of URL into an object * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B diff --git a/packages/core/test/utils-hoist/url.test.ts b/packages/core/test/utils-hoist/url.test.ts index 5126c2647bb7..8cb0a945c2d5 100644 --- a/packages/core/test/utils-hoist/url.test.ts +++ b/packages/core/test/utils-hoist/url.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url'; +import { getSanitizedUrlString, parseStringToURL, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url'; describe('stripQueryStringAndFragment', () => { const urlString = 'http://dogs.are.great:1231/yay/'; @@ -196,3 +196,20 @@ describe('parseUrl', () => { expect(parseUrl(url)).toEqual(expected); }); }); + +describe('parseStringToURL', () => { + it('returns undefined for invalid URLs', () => { + expect(parseStringToURL('invalid-url')).toBeUndefined(); + }); + + it('returns a URL object for valid URLs', () => { + expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL); + }); + + it('does not throw an error if URl.canParse is not defined', () => { + const canParse = (URL as any).canParse; + delete (URL as any).canParse; + expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL); + (URL as any).canParse = canParse; + }); +}); From 50d2514a032fc2dd3d51dc9266bd40b9a4621d0e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Sat, 22 Mar 2025 00:31:34 -0400 Subject: [PATCH 10/15] feat(node): Add fastify `shouldHandleError` (#15771) Supercedes https://github.com/getsentry/sentry-javascript/pull/13198 resolves https://github.com/getsentry/sentry-javascript/issues/13197 Aligns fastify error handler with the express one. 1. Adds `shouldHandleError` to allow users to configure if errors should be captured 2. Makes sure the default `shouldHandleError` does not capture errors for 4xx and 3xx status codes. ## Usage ```js setupFastifyErrorHandler(app, { shouldHandleError(_error, _request, reply) { return statusCode >= 500 || statusCode <= 399; }, }); ``` --- .../test-applications/node-fastify/src/app.ts | 10 ++ .../node-fastify/tests/errors.test.ts | 22 ++++ .../node/src/integrations/tracing/fastify.ts | 103 +++++++++++++++--- 3 files changed, 118 insertions(+), 17 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/src/app.ts b/dev-packages/e2e-tests/test-applications/node-fastify/src/app.ts index 275dfa786ca3..0f37bc33b90a 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify/src/app.ts @@ -81,6 +81,16 @@ app.get('/test-error', async function (req, res) { res.send({ exceptionId }); }); +app.get('/test-4xx-error', async function (req, res) { + res.code(400); + throw new Error('This is a 4xx error'); +}); + +app.get('/test-5xx-error', async function (req, res) { + res.code(500); + throw new Error('This is a 5xx error'); +}); + app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { throw new Error(`This is an exception with id ${req.params.id}`); }); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts index 1b63fe0e0c55..1ac1d3d88234 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts @@ -28,3 +28,25 @@ test('Sends correct error event', async ({ baseURL }) => { parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), }); }); + +test('Does not send 4xx errors by default', async ({ baseURL }) => { + // Define our test approach: we'll send both a 5xx and a 4xx request + // We should only see the 5xx error captured due to shouldHandleError's default behavior + + // Create a promise to wait for the 500 error + const serverErrorPromise = waitForError('node-fastify', event => { + // Looking for a 500 error that should be captured + return !!event.exception?.values?.[0]?.value?.includes('This is a 5xx error'); + }); + + // Make a request that will trigger a 400 error + const notFoundResponse = await fetch(`${baseURL}/test-4xx-error`); + expect(notFoundResponse.status).toBe(400); + + // Make a request that will trigger a 500 error + await fetch(`${baseURL}/test-5xx-error`); + + // Verify we receive the 500 error + const errorEvent = await serverErrorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toContain('This is a 5xx error'); +}); diff --git a/packages/node/src/integrations/tracing/fastify.ts b/packages/node/src/integrations/tracing/fastify.ts index 2920b134d82d..220428cf91d4 100644 --- a/packages/node/src/integrations/tracing/fastify.ts +++ b/packages/node/src/integrations/tracing/fastify.ts @@ -12,19 +12,14 @@ import type { IntegrationFn, Span } from '@sentry/core'; import { generateInstrumentOnce } from '../../otel/instrument'; import { ensureIsWrapped } from '../../utils/ensureIsWrapped'; -// We inline the types we care about here -interface Fastify { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - register: (plugin: any) => void; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void; -} - /** * Minimal request type containing properties around route information. * Works for Fastify 3, 4 and presumably 5. + * + * Based on https://github.com/fastify/fastify/blob/ce3811f5f718be278bbcd4392c615d64230065a6/types/request.d.ts */ -interface FastifyRequestRouteInfo { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +interface MinimalFastifyRequest extends Record { method?: string; // since fastify@4.10.0 routeOptions?: { @@ -33,6 +28,66 @@ interface FastifyRequestRouteInfo { routerPath?: string; } +/** + * Minimal reply type containing properties needed for error handling. + * + * Based on https://github.com/fastify/fastify/blob/ce3811f5f718be278bbcd4392c615d64230065a6/types/reply.d.ts + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +interface MinimalFastifyReply extends Record { + statusCode: number; +} + +// We inline the types we care about here +interface Fastify { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + register: (plugin: any) => void; + addHook: (hook: string, handler: (...params: unknown[]) => void) => void; +} + +interface FastifyWithHooks extends Omit { + addHook( + hook: 'onError', + handler: (request: MinimalFastifyRequest, reply: MinimalFastifyReply, error: Error) => void, + ): void; + addHook(hook: 'onRequest', handler: (request: MinimalFastifyRequest, reply: MinimalFastifyReply) => void): void; +} + +interface FastifyHandlerOptions { + /** + * Callback method deciding whether error should be captured and sent to Sentry + * + * @param error Captured Fastify error + * @param request Fastify request (or any object containing at least method, routeOptions.url, and routerPath) + * @param reply Fastify reply (or any object containing at least statusCode) + * + * @example + * + * ```javascript + * setupFastifyErrorHandler(app, { + * shouldHandleError(_error, _request, reply) { + * return reply.statusCode >= 400; + * }, + * }); + * ``` + * + * If using TypeScript, you can cast the request and reply to get full type safety. + * + * ```typescript + * import type { FastifyRequest, FastifyReply } from 'fastify'; + * + * setupFastifyErrorHandler(app, { + * shouldHandleError(error, minimalRequest, minimalReply) { + * const request = minimalRequest as FastifyRequest; + * const reply = minimalReply as FastifyReply; + * return reply.statusCode >= 500; + * }, + * }); + * ``` + */ + shouldHandleError: (error: Error, request: MinimalFastifyRequest, reply: MinimalFastifyReply) => boolean; +} + const INTEGRATION_NAME = 'Fastify'; export const instrumentFastify = generateInstrumentOnce( @@ -73,10 +128,22 @@ const _fastifyIntegration = (() => { */ export const fastifyIntegration = defineIntegration(_fastifyIntegration); +/** + * Default function to determine if an error should be sent to Sentry + * + * 3xx and 4xx errors are not sent by default. + */ +function defaultShouldHandleError(_error: Error, _request: MinimalFastifyRequest, reply: MinimalFastifyReply): boolean { + const statusCode = reply.statusCode; + // 3xx and 4xx errors are not sent by default. + return statusCode >= 500 || statusCode <= 299; +} + /** * Add an Fastify error handler to capture errors to Sentry. * * @param fastify The Fastify instance to which to add the error handler + * @param options Configuration options for the handler * * @example * ```javascript @@ -92,23 +159,25 @@ export const fastifyIntegration = defineIntegration(_fastifyIntegration); * app.listen({ port: 3000 }); * ``` */ -export function setupFastifyErrorHandler(fastify: Fastify): void { +export function setupFastifyErrorHandler(fastify: Fastify, options?: Partial): void { + const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; + const plugin = Object.assign( - function (fastify: Fastify, _options: unknown, done: () => void): void { - fastify.addHook('onError', async (_request, _reply, error) => { - captureException(error); + function (fastify: FastifyWithHooks, _options: unknown, done: () => void): void { + fastify.addHook('onError', async (request, reply, error) => { + if (shouldHandleError(error, request, reply)) { + captureException(error); + } }); // registering `onRequest` hook here instead of using Otel `onRequest` callback b/c `onRequest` hook // is ironically called in the fastify `preHandler` hook which is called later in the lifecycle: // https://fastify.dev/docs/latest/Reference/Lifecycle/ fastify.addHook('onRequest', async (request, _reply) => { - const reqWithRouteInfo = request as FastifyRequestRouteInfo; - // Taken from Otel Fastify instrumentation: // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts#L94-L96 - const routeName = reqWithRouteInfo.routeOptions?.url || reqWithRouteInfo.routerPath; - const method = reqWithRouteInfo.method || 'GET'; + const routeName = request.routeOptions?.url || request.routerPath; + const method = request.method || 'GET'; getIsolationScope().setTransactionName(`${method} ${routeName}`); }); From 57f04e0385ef403d188d9fb79c78398cfc50942e Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Mon, 24 Mar 2025 10:39:25 +0100 Subject: [PATCH 11/15] ref: Remove some usages of `dropUndefinedKeys()` (#15781) In some places this should not really be needed, so we can skip this. --- packages/astro/src/integration/index.ts | 54 +++++++++---------- packages/browser-utils/src/metrics/inp.ts | 5 +- packages/core/src/checkin.ts | 3 +- packages/core/src/feedback.ts | 5 +- packages/core/src/session.ts | 6 +-- .../src/tracing/dynamicSamplingContext.ts | 2 +- .../src/coreHandlers/util/networkUtils.ts | 6 +-- .../src/client/browserTracingIntegration.ts | 6 +-- 8 files changed, 41 insertions(+), 46 deletions(-) diff --git a/packages/astro/src/integration/index.ts b/packages/astro/src/integration/index.ts index fb26e956daeb..e413bb4cf67b 100644 --- a/packages/astro/src/integration/index.ts +++ b/packages/astro/src/integration/index.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { sentryVitePlugin } from '@sentry/vite-plugin'; import type { AstroConfig, AstroIntegration } from 'astro'; -import { consoleSandbox, dropUndefinedKeys } from '@sentry/core'; +import { consoleSandbox } from '@sentry/core'; import { buildClientSnippet, buildSdkInitFileImportSnippet, buildServerSnippet } from './snippets'; import type { SentryOptions } from './types'; @@ -62,34 +62,32 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { sourcemap: computedSourceMapSettings.updatedSourceMapSetting, }, plugins: [ - sentryVitePlugin( - dropUndefinedKeys({ - org: uploadOptions.org ?? env.SENTRY_ORG, - project: uploadOptions.project ?? env.SENTRY_PROJECT, - authToken: uploadOptions.authToken ?? env.SENTRY_AUTH_TOKEN, - telemetry: uploadOptions.telemetry ?? true, - _metaOptions: { - telemetry: { - metaFramework: 'astro', - }, + sentryVitePlugin({ + org: uploadOptions.org ?? env.SENTRY_ORG, + project: uploadOptions.project ?? env.SENTRY_PROJECT, + authToken: uploadOptions.authToken ?? env.SENTRY_AUTH_TOKEN, + telemetry: uploadOptions.telemetry ?? true, + _metaOptions: { + telemetry: { + metaFramework: 'astro', }, - ...unstable_sentryVitePluginOptions, - debug: options.debug ?? false, - sourcemaps: { - assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], - filesToDeleteAfterUpload: - uploadOptions?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload, - ...unstable_sentryVitePluginOptions?.sourcemaps, - }, - bundleSizeOptimizations: { - ...options.bundleSizeOptimizations, - // TODO: with a future version of the vite plugin (probably 2.22.0) this re-mapping is not needed anymore - // ref: https://github.com/getsentry/sentry-javascript-bundler-plugins/pull/582 - excludePerformanceMonitoring: options.bundleSizeOptimizations?.excludeTracing, - ...unstable_sentryVitePluginOptions?.bundleSizeOptimizations, - }, - }), - ), + }, + ...unstable_sentryVitePluginOptions, + debug: options.debug ?? false, + sourcemaps: { + assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], + filesToDeleteAfterUpload: + uploadOptions?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload, + ...unstable_sentryVitePluginOptions?.sourcemaps, + }, + bundleSizeOptimizations: { + ...options.bundleSizeOptimizations, + // TODO: with a future version of the vite plugin (probably 2.22.0) this re-mapping is not needed anymore + // ref: https://github.com/getsentry/sentry-javascript-bundler-plugins/pull/582 + excludePerformanceMonitoring: options.bundleSizeOptimizations?.excludeTracing, + ...unstable_sentryVitePluginOptions?.bundleSizeOptimizations, + }, + }), ], }, }); diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 64ea9cccaca0..0392be622355 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -6,7 +6,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, browserPerformanceTimeOrigin, - dropUndefinedKeys, getActiveSpan, getCurrentScope, getRootSpan, @@ -101,11 +100,11 @@ function _trackINP(): () => void { const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName; const name = htmlTreeAsString(entry.target); - const attributes: SpanAttributes = dropUndefinedKeys({ + const attributes: SpanAttributes = { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`, [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, - }); + }; const span = startStandaloneWebVitalSpan({ name, diff --git a/packages/core/src/checkin.ts b/packages/core/src/checkin.ts index 44b460376916..c182c676117e 100644 --- a/packages/core/src/checkin.ts +++ b/packages/core/src/checkin.ts @@ -8,7 +8,6 @@ import type { } from './types-hoist'; import { dsnToString } from './utils-hoist/dsn'; import { createEnvelope } from './utils-hoist/envelope'; -import { dropUndefinedKeys } from './utils-hoist/object'; /** * Create envelope from check in item. @@ -36,7 +35,7 @@ export function createCheckInEnvelope( } if (dynamicSamplingContext) { - headers.trace = dropUndefinedKeys(dynamicSamplingContext) as DynamicSamplingContext; + headers.trace = dynamicSamplingContext as DynamicSamplingContext; } const item = createCheckInEnvelopeItem(checkIn); diff --git a/packages/core/src/feedback.ts b/packages/core/src/feedback.ts index 088248102012..7cfcd8b28654 100644 --- a/packages/core/src/feedback.ts +++ b/packages/core/src/feedback.ts @@ -1,6 +1,5 @@ import { getClient, getCurrentScope } from './currentScopes'; import type { EventHint, FeedbackEvent, SendFeedbackParams } from './types-hoist'; -import { dropUndefinedKeys } from './utils-hoist/object'; /** * Send user feedback to Sentry. @@ -14,14 +13,14 @@ export function captureFeedback( const feedbackEvent: FeedbackEvent = { contexts: { - feedback: dropUndefinedKeys({ + feedback: { contact_email: email, name, message, url, source, associated_event_id: associatedEventId, - }), + }, }, type: 'feedback', level: 'info', diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index 860dec52b386..a1ea29448fa4 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -1,5 +1,5 @@ import type { SerializedSession, Session, SessionContext, SessionStatus } from './types-hoist'; -import { dropUndefinedKeys, timestampInSeconds, uuid4 } from './utils-hoist'; +import { timestampInSeconds, uuid4 } from './utils-hoist'; /** * Creates a new `Session` object by setting certain default parameters. If optional @param context @@ -137,7 +137,7 @@ export function closeSession(session: Session, status?: Exclude Date: Mon, 24 Mar 2025 10:45:02 +0100 Subject: [PATCH 12/15] ref(nextjs): Fix Next.js vercel-edge runtime package information (#15789) Noticed this was "incorrect" here, for all other places it seems correct. --- packages/nextjs/src/edge/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 24ec193df0d1..77469cfdf9dc 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -50,7 +50,7 @@ export function init(options: VercelEdgeOptions = {}): void { ...options, }; - applySdkMetadata(opts, 'nextjs'); + applySdkMetadata(opts, 'nextjs', ['nextjs', 'vercel-edge']); const client = vercelEdgeInit(opts); From 79ff8f27513829be53f4ebd168dc7e207bfd2828 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 24 Mar 2025 10:58:25 +0100 Subject: [PATCH 13/15] fix(nuxt): Delete no longer needed Nitro 'close' hook (#15790) Due to PR https://github.com/getsentry/sentry-javascript/pull/15710 the Sentry server config is only processed inside the Nitro-part of Nuxt and the server config file is emitted through the Rollup plugin. It is not needed anymore to copy-paste the file manually (this has been the case before as the file was added to the `.nuxt` folder - as it was processed by Nuxt, not Nitro-only). This fixes the "no such file" error [posted in this comment](https://github.com/getsentry/sentry-javascript/issues/13330#issuecomment-2743702460). closes https://github.com/getsentry/sentry-javascript/issues/13330 --- packages/nuxt/src/vite/addServerConfig.ts | 40 ++--------------------- 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/packages/nuxt/src/vite/addServerConfig.ts b/packages/nuxt/src/vite/addServerConfig.ts index 3ee59a210cdd..9495f4ac2f4a 100644 --- a/packages/nuxt/src/vite/addServerConfig.ts +++ b/packages/nuxt/src/vite/addServerConfig.ts @@ -1,6 +1,6 @@ import * as fs from 'fs'; import { createResolver } from '@nuxt/kit'; -import { consoleSandbox, logger } from '@sentry/core'; +import { logger } from '@sentry/core'; import type { Nitro } from 'nitropack'; import type { InputPluginOption } from 'rollup'; import type { SentryNuxtModuleOptions } from '../common/types'; @@ -21,8 +21,7 @@ const SERVER_CONFIG_FILENAME = 'sentry.server.config'; /** * Adds the `sentry.server.config.ts` file as `sentry.server.config.mjs` to the `.output` directory to be able to reference this file in the node --import option. * - * 1. Adding the file as a rollup import, so it is included in the build (automatically transpiles the file). - * 2. Copying the file to the `.output` directory after the build process is finished. + * By adding a Rollup plugin to the Nitro Rollup options, the Sentry server config is transpiled and emitted to the server build. */ export function addServerConfigToBuild( moduleOptions: SentryNuxtModuleOptions, @@ -39,41 +38,6 @@ export function addServerConfigToBuild( rollupConfig.plugins.push(injectServerConfigPlugin(nitro, serverConfigFile, moduleOptions.debug)); }); - - /** - * When the build process is finished, copy the `sentry.server.config` file to the `.output` directory. - * This is necessary because we need to reference this file path in the node --import option. - */ - nitro.hooks.hook('close', async () => { - const buildDirResolver = createResolver(nitro.options.buildDir); - const serverDirResolver = createResolver(nitro.options.output.serverDir); - const source = buildDirResolver.resolve(`dist/server/${SERVER_CONFIG_FILENAME}.mjs`); - const destination = serverDirResolver.resolve(`${SERVER_CONFIG_FILENAME}.mjs`); - - try { - await fs.promises.access(source, fs.constants.F_OK); - await fs.promises.copyFile(source, destination); - - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log( - `[Sentry] Successfully added the content of the \`${serverConfigFile}\` file to \`${destination}\``, - ); - }); - } - } catch (error) { - if (moduleOptions.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] An error occurred when trying to add the \`${serverConfigFile}\` file to the \`.output\` directory`, - error, - ); - }); - } - } - }); } /** From 0cf8ec2637cff9cff12c34e2996fd146172f6d82 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Mon, 24 Mar 2025 11:21:00 +0100 Subject: [PATCH 14/15] feat(browser): Attach host as part of error message to "Failed to fetch" errors (#15729) Closes https://github.com/getsentry/sentry-javascript/issues/15709 This also adds tests for the various types of TypeErrors that fetch can produce. --- .size-limit.js | 2 +- .../suites/errors/fetch/init.js | 13 + .../suites/errors/fetch/subject.js | 45 +++ .../suites/errors/fetch/test.ts | 288 ++++++++++++++++++ .../core/src/utils-hoist/instrument/fetch.ts | 19 ++ 5 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/errors/fetch/init.js create mode 100644 dev-packages/browser-integration-tests/suites/errors/fetch/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/errors/fetch/test.ts diff --git a/.size-limit.js b/.size-limit.js index 7ecd54ab92f4..7bab8160966b 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -40,7 +40,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '37.5 KB', + limit: '38 KB', }, { name: '@sentry/browser (incl. Tracing, Replay)', diff --git a/dev-packages/browser-integration-tests/suites/errors/fetch/init.js b/dev-packages/browser-integration-tests/suites/errors/fetch/init.js new file mode 100644 index 000000000000..ce283e32d303 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/errors/fetch/init.js @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transportOptions: { + fetchOptions: { + // See: https://github.com/microsoft/playwright/issues/34497 + keepalive: false, + }, + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/errors/fetch/subject.js b/dev-packages/browser-integration-tests/suites/errors/fetch/subject.js new file mode 100644 index 000000000000..8bae73df7b31 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/errors/fetch/subject.js @@ -0,0 +1,45 @@ +// Based on possible TypeError exceptions from https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch + +// Network error (e.g. ad-blocked, offline, page does not exist, ...) +window.networkError = () => { + fetch('http://sentry-test-external.io/does-not-exist'); +}; + +window.networkErrorSubdomain = () => { + fetch('http://subdomain.sentry-test-external.io/does-not-exist'); +}; + +// Invalid header also produces TypeError +window.invalidHeaderName = () => { + fetch('http://sentry-test-external.io/invalid-header-name', { headers: { 'C ontent-Type': 'text/xml' } }); +}; + +// Invalid header value also produces TypeError +window.invalidHeaderValue = () => { + fetch('http://sentry-test-external.io/invalid-header-value', { headers: ['Content-Type', 'text/html', 'extra'] }); +}; + +// Invalid URL scheme +window.invalidUrlScheme = () => { + fetch('blub://sentry-test-external.io/invalid-scheme'); +}; + +// URL includes credentials +window.credentialsInUrl = () => { + fetch('https://user:password@sentry-test-external.io/credentials-in-url'); +}; + +// Invalid mode +window.invalidMode = () => { + fetch('https://sentry-test-external.io/invalid-mode', { mode: 'navigate' }); +}; + +// Invalid request method +window.invalidMethod = () => { + fetch('http://sentry-test-external.io/invalid-method', { method: 'CONNECT' }); +}; + +// No-cors mode with cors-required method +window.noCorsMethod = () => { + fetch('http://sentry-test-external.io/no-cors-method', { mode: 'no-cors', method: 'PUT' }); +}; diff --git a/dev-packages/browser-integration-tests/suites/errors/fetch/test.ts b/dev-packages/browser-integration-tests/suites/errors/fetch/test.ts new file mode 100644 index 000000000000..f9b59dd07f60 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/errors/fetch/test.ts @@ -0,0 +1,288 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; + +sentryTest('handles fetch network errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('networkError()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: 'Failed to fetch (sentry-test-external.io)', + webkit: 'Load failed (sentry-test-external.io)', + firefox: 'NetworkError when attempting to fetch resource. (sentry-test-external.io)', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + }); +}); + + +sentryTest('handles fetch network errors on subdomains @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('networkErrorSubdomain()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: 'Failed to fetch (subdomain.sentry-test-external.io)', + webkit: 'Load failed (subdomain.sentry-test-external.io)', + firefox: 'NetworkError when attempting to fetch resource. (subdomain.sentry-test-external.io)', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + }); +}); + +sentryTest('handles fetch invalid header name errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('invalidHeaderName()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: "Failed to execute 'fetch' on 'Window': Invalid name", + webkit: "Invalid header name: 'C ontent-Type'", + firefox: 'Window.fetch: c ontent-type is an invalid header name.', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest('handles fetch invalid header value errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('invalidHeaderValue()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: + "Failed to execute 'fetch' on 'Window': Failed to read the 'headers' property from 'RequestInit': The provided value cannot be converted to a sequence.", + webkit: 'Value is not a sequence', + firefox: + "Window.fetch: Element of sequence> branch of (sequence> or record) can't be converted to a sequence.", + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest('handles fetch invalid URL scheme errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + await page.route('http://sentry-test-external.io/**', route => { + return route.fulfill({ + status: 200, + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('invalidUrlScheme()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: 'Failed to fetch (sentry-test-external.io)', + webkit: 'Load failed (sentry-test-external.io)', + firefox: 'NetworkError when attempting to fetch resource. (sentry-test-external.io)', + }; + + const error = errorMap[browserName]; + + /** + * This kind of error does show a helpful warning in the console, e.g.: + * Fetch API cannot load blub://sentry-test-external.io/invalid-scheme. URL scheme "blub" is not supported. + * But it seems we cannot really access this in the SDK :( + */ + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest('handles fetch credentials in url errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('credentialsInUrl()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: + "Failed to execute 'fetch' on 'Window': Request cannot be constructed from a URL that includes credentials: https://user:password@sentry-test-external.io/credentials-in-url", + webkit: 'URL is not valid or contains user credentials.', + firefox: + 'Window.fetch: https://user:password@sentry-test-external.io/credentials-in-url is an url with embedded credentials.', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest('handles fetch invalid mode errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('invalidMode()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: + "Failed to execute 'fetch' on 'Window': Cannot construct a Request with a RequestInit whose mode member is set as 'navigate'.", + webkit: 'Request constructor does not accept navigate fetch mode.', + firefox: 'Window.fetch: Invalid request mode navigate.', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest('handles fetch invalid request method errors @firefox', async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('invalidMethod()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: "Failed to execute 'fetch' on 'Window': 'CONNECT' HTTP method is unsupported.", + webkit: 'Method is forbidden.', + firefox: 'Window.fetch: Invalid request method CONNECT.', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); + +sentryTest( + 'handles fetch no-cors mode with cors-required method errors @firefox', + async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + const reqPromise = waitForErrorRequest(page); + await page.goto(url); + await page.evaluate('noCorsMethod()'); + + const eventData = envelopeRequestParser(await reqPromise); + + const errorMap: Record = { + chromium: "Failed to execute 'fetch' on 'Window': 'PUT' is unsupported in no-cors mode.", + webkit: 'Method must be GET, POST or HEAD in no-cors mode.', + firefox: 'Window.fetch: Invalid request method PUT.', + }; + + const error = errorMap[browserName]; + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'TypeError', + value: error, + mechanism: { + handled: false, + type: 'onunhandledrejection', + }, + stacktrace: { + frames: expect.any(Array), + }, + }); + }, +); diff --git a/packages/core/src/utils-hoist/instrument/fetch.ts b/packages/core/src/utils-hoist/instrument/fetch.ts index f3eee711d26d..71c5148fae9c 100644 --- a/packages/core/src/utils-hoist/instrument/fetch.ts +++ b/packages/core/src/utils-hoist/instrument/fetch.ts @@ -107,6 +107,25 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat addNonEnumerableProperty(error, 'framesToPop', 1); } + // We enhance the not-so-helpful "Failed to fetch" error messages with the host + // Possible messages we handle here: + // * "Failed to fetch" (chromium) + // * "Load failed" (webkit) + // * "NetworkError when attempting to fetch resource." (firefox) + if ( + error instanceof TypeError && + (error.message === 'Failed to fetch' || + error.message === 'Load failed' || + error.message === 'NetworkError when attempting to fetch resource.') + ) { + try { + const url = new URL(handlerData.fetchData.url); + error.message = `${error.message} (${url.host})`; + } catch { + // ignore it if errors happen here + } + } + // 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 bb358cf86ce303b4f320060ccee1b1d7c73676ba Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 24 Mar 2025 11:24:35 +0100 Subject: [PATCH 15/15] meta: Update changelog for 9.9.0 swap ordering --- CHANGELOG.md | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9552fa613b65..7adb951938a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,66 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 9.9.0 + +### Important Changes + +- **feat(nextjs): Support `instrumentation-client.ts` ([#15705](https://github.com/getsentry/sentry-javascript/pull/15705))** + + Next.js recently added a feature to support [client-side (browser) instrumentation via the `experimental.clientInstrumentationHook` flag and the `instrumentation-client.ts` file](https://nextjs.org/docs/app/api-reference/config/next-config-js/clientInstrumentationHook). + + To be forwards compatible, the Sentry Next.js SDK will now pick up `instrumentation-client.ts` files even on older Next.js versions and add them to your client bundles. + It is suggested that you either rename your `sentry.client.config.ts` file to `instrumentation-client.ts`, or if you already happen to have a `instrumentation-client.ts` file move the contents of `sentry.client.config.ts` to `instrumentation-client.ts`. + +- **feat(browser): Add `previous_trace` span links ([#15569](https://github.com/getsentry/sentry-javascript/pull/15569))** + + The `@sentry/browser` SDK and SDKs based on `@sentry/browser` now emits a link from the first root span of a newly started trace to the root span of a previously started trace. You can control this feature via an option in `browserTracingIntegration()`: + + ```js + Sentry.init({ + dsn: 'your-dsn-here' + integrations: [ + Sentry.browserTracingIntegration({ + // Available settings: + // - 'in-memory' (default): Stores previous trace information in memory + // - 'session-storage': Stores previous trace information in the browser's `sessionStorage` + // - 'off': Disable storing and sending previous trace information + linkPreviousTrace: 'in-memory', + }), + ], + }); + ``` + +- **feat(browser): Add `logger.X` methods to browser SDK ([#15763](https://github.com/getsentry/sentry-javascript/pull/15763))** + + For Sentry's [upcoming logging product](https://github.com/getsentry/sentry/discussions/86804), the SDK now supports sending logs via dedicated + + ```js + Sentry.init({ + dsn: 'your-dsn-here', + _experiments: { + enableLogs: true, // This is required to use the logging features + }, + }); + + Sentry.logger.info('This is a trace message', { userId: 123 }); + // See PR for better documentation + ``` + + Please note that the logs product is still in early access. See the link above for more information. + +### Other Changes + +- feat(browser): Attach host as part of error message to "Failed to fetch" errors ([#15729](https://github.com/getsentry/sentry-javascript/pull/15729)) +- feat(core): Add `parseStringToURL` method ([#15768](https://github.com/getsentry/sentry-javascript/pull/15768)) +- feat(core): Optimize `dropUndefinedKeys` ([#15760](https://github.com/getsentry/sentry-javascript/pull/15760)) +- feat(node): Add fastify `shouldHandleError` ([#15771](https://github.com/getsentry/sentry-javascript/pull/15771)) +- fix(nuxt): Delete no longer needed Nitro 'close' hook ([#15790](https://github.com/getsentry/sentry-javascript/pull/15790)) +- perf(nestjs): Remove usage of `addNonEnumerableProperty` ([#15766](https://github.com/getsentry/sentry-javascript/pull/15766)) +- ref: Avoid some usage of `dropUndefinedKeys()` ([#15757](https://github.com/getsentry/sentry-javascript/pull/15757)) +- ref: Remove some usages of `dropUndefinedKeys()` ([#15781](https://github.com/getsentry/sentry-javascript/pull/15781)) +- ref(nextjs): Fix Next.js vercel-edge runtime package information ([#15789](https://github.com/getsentry/sentry-javascript/pull/15789)) + ## 9.8.0 - feat(node): Implement new continuous profiling API spec ([#15635](https://github.com/getsentry/sentry-javascript/pull/15635))