From 5cd11c2137653fa81f2d181fbc92d2ce8aeff5cc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 May 2023 15:17:03 +0200 Subject: [PATCH 1/5] fix(sveltekit): Avoid creating the Sentry Vite plugin in dev mode (#8065) Don't call the Vite plugin factory function in dev mode to keep the bundler plugin from leaking its telemetry tags to users' Sentry instance. We don't want to upload source maps in dev mode anyway. --- packages/sveltekit/src/vite/sentryVitePlugins.ts | 2 +- .../test/vite/sentrySvelteKitPlugins.test.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 0f2461a419de..1f983d7a9653 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -78,7 +78,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} ); } - if (mergedOptions.autoUploadSourceMaps) { + if (mergedOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') { const pluginOptions = { ...mergedOptions.sourceMapsUploadOptions, debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index f606b5ee6e9a..963844bdef70 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -38,6 +38,19 @@ describe('sentryVite()', () => { expect(plugins).toHaveLength(1); }); + it("doesn't return the custom sentry source maps plugin if `NODE_ENV` is development", async () => { + const previousEnv = process.env.NODE_ENV; + + process.env.NODE_ENV = 'development'; + const plugins = await sentrySvelteKit({ autoUploadSourceMaps: true, autoInstrument: true }); + const instrumentPlugin = plugins[0]; + + expect(plugins).toHaveLength(1); + expect(instrumentPlugin.name).toEqual('sentry-auto-instrumentation'); + + process.env.NODE_ENV = previousEnv; + }); + it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => { const plugins = await sentrySvelteKit({ autoInstrument: false }); expect(plugins).toHaveLength(1); From e3ba0ade9f234f5720c53f672587bfdb859dec9f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 May 2023 16:15:50 +0200 Subject: [PATCH 2/5] test(e2e): Add canary tests for react e2e tests (#8068) --- .../create-react-app/test-recipe.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test-applications/create-react-app/test-recipe.json b/packages/e2e-tests/test-applications/create-react-app/test-recipe.json index a2b1bd98ea61..a45dde0d9c07 100644 --- a/packages/e2e-tests/test-applications/create-react-app/test-recipe.json +++ b/packages/e2e-tests/test-applications/create-react-app/test-recipe.json @@ -2,5 +2,13 @@ "$schema": "../../test-recipe-schema.json", "testApplicationName": "create-react-app", "buildCommand": "pnpm install && pnpm build", - "tests": [] + "tests": [], + "canaryVersions": [ + { + "dependencyOverrides": { + "react": "canary", + "react-dom": "canary" + } + } + ] } From 17f11e820e1964f795b0ed7b33d64ee312c24543 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 May 2023 16:24:43 +0200 Subject: [PATCH 3/5] chore: Make job_browser_loader_tests a required test (#8064) --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f457b1bffa2c..cac9a0da588a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -808,6 +808,7 @@ jobs: job_node_integration_tests, job_browser_playwright_tests, job_browser_integration_tests, + job_browser_loader_tests, job_remix_integration_tests, job_e2e_tests, ] From 4d7ac23282c71d4601de3b1380ca10d6fdd14cff Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 May 2023 19:29:25 +0200 Subject: [PATCH 4/5] fix(nextjs): Continue traces in data fetchers when there is an already active transaction on the hub (#8073) --- .../nextjs/src/server/utils/wrapperUtils.ts | 25 ++++++++++--------- .../wrapAppGetInitialPropsWithSentry.ts | 5 ++-- .../wrapErrorGetInitialPropsWithSentry.ts | 5 ++-- .../server/wrapGetInitialPropsWithSentry.ts | 5 ++-- .../wrapGetServerSidePropsWithSentry.ts | 5 ++-- packages/nextjs/test/config/wrappers.test.ts | 16 ++++++++---- 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/packages/nextjs/src/server/utils/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts index 44b193038011..c0fb4037c58b 100644 --- a/packages/nextjs/src/server/utils/wrapperUtils.ts +++ b/packages/nextjs/src/server/utils/wrapperUtils.ts @@ -5,7 +5,7 @@ import { runWithAsyncContext, startTransaction, } from '@sentry/core'; -import type { Transaction } from '@sentry/types'; +import type { Span, Transaction } from '@sentry/types'; import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; import type { IncomingMessage, ServerResponse } from 'http'; @@ -82,7 +82,8 @@ export function withTracedServerSideDataFetcher Pr return async function (this: unknown, ...args: Parameters): Promise> { return runWithAsyncContext(async () => { const hub = getCurrentHub(); - let requestTransaction: Transaction | undefined = getTransactionFromRequest(req); + const scope = hub.getScope(); + const previousSpan: Span | undefined = getTransactionFromRequest(req) ?? scope.getSpan(); let dataFetcherSpan; const sentryTraceHeader = req.headers['sentry-trace']; @@ -93,7 +94,8 @@ export function withTracedServerSideDataFetcher Pr const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(rawBaggageString); if (platformSupportsStreaming()) { - if (requestTransaction === undefined) { + let spanToContinue: Span; + if (previousSpan === undefined) { const newTransaction = startTransaction( { op: 'http.server', @@ -109,8 +111,6 @@ export function withTracedServerSideDataFetcher Pr { request: req }, ); - requestTransaction = newTransaction; - if (platformSupportsStreaming()) { // On platforms that don't support streaming, doing things after res.end() is unreliable. autoEndTransactionOnResponseEnd(newTransaction, res); @@ -119,9 +119,12 @@ export function withTracedServerSideDataFetcher Pr // Link the transaction and the request together, so that when we would normally only have access to one, it's // still possible to grab the other. setTransactionOnRequest(newTransaction, req); + spanToContinue = newTransaction; + } else { + spanToContinue = previousSpan; } - dataFetcherSpan = requestTransaction.startChild({ + dataFetcherSpan = spanToContinue.startChild({ op: 'function.nextjs', description: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`, status: 'ok', @@ -140,11 +143,8 @@ export function withTracedServerSideDataFetcher Pr }); } - const currentScope = hub.getScope(); - if (currentScope) { - currentScope.setSpan(dataFetcherSpan); - currentScope.setSDKProcessingMetadata({ request: req }); - } + scope.setSpan(dataFetcherSpan); + scope.setSDKProcessingMetadata({ request: req }); try { return await origDataFetcher.apply(this, args); @@ -152,10 +152,11 @@ export function withTracedServerSideDataFetcher Pr // Since we finish the span before the error can bubble up and trigger the handlers in `registerErrorInstrumentation` // that set the transaction status, we need to manually set the status of the span & transaction dataFetcherSpan.setStatus('internal_error'); - requestTransaction?.setStatus('internal_error'); + previousSpan?.setStatus('internal_error'); throw e; } finally { dataFetcherSpan.finish(); + scope.setSpan(previousSpan); if (!platformSupportsStreaming()) { await flushQueue(); } diff --git a/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts index 178e7617866c..e1ecf50dad54 100644 --- a/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts @@ -31,7 +31,8 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI const { req, res } = context.ctx; const errorWrappedAppGetInitialProps = withErrorInstrumentation(wrappingTarget); - const options = getCurrentHub().getClient()?.getOptions(); + const hub = getCurrentHub(); + const options = hub.getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object @@ -51,7 +52,7 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI }; } = await tracedGetInitialProps.apply(thisArg, args); - const requestTransaction = getTransactionFromRequest(req); + const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction(); // Per definition, `pageProps` is not optional, however an increased amount of users doesn't seem to call // `App.getInitialProps(appContext)` in their custom `_app` pages which is required as per diff --git a/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts index 176474f0e0c8..b99b04524d46 100644 --- a/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts @@ -34,7 +34,8 @@ export function wrapErrorGetInitialPropsWithSentry( const { req, res } = context; const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget); - const options = getCurrentHub().getClient()?.getOptions(); + const hub = getCurrentHub(); + const options = hub.getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object @@ -52,7 +53,7 @@ export function wrapErrorGetInitialPropsWithSentry( _sentryBaggage?: string; } = await tracedGetInitialProps.apply(thisArg, args); - const requestTransaction = getTransactionFromRequest(req); + const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction(); if (requestTransaction) { errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent(); diff --git a/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts index 466a1438468b..d1613a80d2c9 100644 --- a/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts @@ -30,7 +30,8 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro const { req, res } = context; const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget); - const options = getCurrentHub().getClient()?.getOptions(); + const hub = getCurrentHub(); + const options = hub.getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object @@ -48,7 +49,7 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro _sentryBaggage?: string; } = await tracedGetInitialProps.apply(thisArg, args); - const requestTransaction = getTransactionFromRequest(req); + const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction(); if (requestTransaction) { initialProps._sentryTraceData = requestTransaction.toTraceparent(); diff --git a/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts b/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts index d58be90464e1..f37068bae206 100644 --- a/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts @@ -31,7 +31,8 @@ export function wrapGetServerSidePropsWithSentry( const { req, res } = context; const errorWrappedGetServerSideProps = withErrorInstrumentation(wrappingTarget); - const options = getCurrentHub().getClient()?.getOptions(); + const hub = getCurrentHub(); + const options = hub.getClient()?.getOptions(); if (hasTracingEnabled() && options?.instrumenter === 'sentry') { const tracedGetServerSideProps = withTracedServerSideDataFetcher(errorWrappedGetServerSideProps, req, res, { @@ -45,7 +46,7 @@ export function wrapGetServerSidePropsWithSentry( >); if (serverSideProps && 'props' in serverSideProps) { - const requestTransaction = getTransactionFromRequest(req); + const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction(); if (requestTransaction) { serverSideProps.props._sentryTraceData = requestTransaction.toTraceparent(); diff --git a/packages/nextjs/test/config/wrappers.test.ts b/packages/nextjs/test/config/wrappers.test.ts index 444d45513ccb..9195154991be 100644 --- a/packages/nextjs/test/config/wrappers.test.ts +++ b/packages/nextjs/test/config/wrappers.test.ts @@ -6,6 +6,7 @@ import type { IncomingMessage, ServerResponse } from 'http'; import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/server'; const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction'); +const originalGetCurrentHub = jest.requireActual('@sentry/node').getCurrentHub; // The wrap* functions require the hub to have tracing extensions. This is normally called by the NodeClient // constructor but the client isn't used in these tests. @@ -21,13 +22,18 @@ describe('data-fetching function wrappers', () => { req = { headers: {}, url: 'http://dogs.are.great/tricks/kangaroo' } as IncomingMessage; res = { end: jest.fn() } as unknown as ServerResponse; - jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValueOnce(true); - jest.spyOn(SentryNode, 'getCurrentHub').mockReturnValueOnce({ - getClient: () => + jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValue(true); + jest.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => { + const hub = originalGetCurrentHub(); + + hub.getClient = () => ({ getOptions: () => ({ instrumenter: 'sentry' }), - } as any), - } as any); + getDsn: () => {}, + } as any); + + return hub; + }); }); afterEach(() => { From bfb44fef075f87812abb140d37e82da3ee8c9b6d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 8 May 2023 17:34:14 +0000 Subject: [PATCH 5/5] meta(changelog): Update changelog for 7.51.2 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e405fcbdd7a5..29a0a10d095e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.51.2 + +- fix(nextjs): Continue traces in data fetchers when there is an already active transaction on the hub (#8073) +- fix(sveltekit): Avoid creating the Sentry Vite plugin in dev mode (#8065) + ## 7.51.1 - feat(replay): Add event to capture options on checkouts (#8011)