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, ] 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) 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" + } + } + ] } 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(() => { 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);