From d54314fb757fa873b1d666cc2eec9f87376a672b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 30 Aug 2022 15:40:33 +0000 Subject: [PATCH 01/12] feat(nextjs): Connect trace between data-fetching methods and pageload --- .../wrappers/withSentryGetServerSideProps.ts | 31 +++++++++++++---- .../withSentryServerSideAppGetInitialProps.ts | 33 +++++++++++++++---- ...ithSentryServerSideErrorGetInitialProps.ts | 31 +++++++++++++---- .../withSentryServerSideGetInitialProps.ts | 17 ++++++++-- .../src/config/wrappers/wrapperUtils.ts | 22 ++++++++++--- packages/nextjs/src/performance/client.ts | 25 +++++++------- 6 files changed, 119 insertions(+), 40 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index 49fe55535d0e..0f18eb52187e 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,8 +1,9 @@ import { hasTracingEnabled } from '@sentry/tracing'; +import { serializeBaggage } from '@sentry/utils'; import { GetServerSideProps } from 'next'; import { isBuild } from '../../utils/isBuild'; -import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; +import { callTracedServerSideDataFetcher, getTransactionFromRequest, withErrorInstrumentation } from './wrapperUtils'; /** * Create a wrapped version of the user's exported `getServerSideProps` function @@ -28,13 +29,29 @@ export function withSentryGetServerSideProps( const errorWrappedGetServerSideProps = withErrorInstrumentation(origGetServerSideProps); if (hasTracingEnabled()) { - return callTracedServerSideDataFetcher(errorWrappedGetServerSideProps, getServerSidePropsArguments, req, res, { - dataFetcherRouteName: parameterizedRoute, - requestedRouteName: parameterizedRoute, - dataFetchingMethodName: 'getServerSideProps', - }); + const serverSideProps = await callTracedServerSideDataFetcher( + errorWrappedGetServerSideProps, + getServerSidePropsArguments, + req, + res, + { + dataFetcherRouteName: parameterizedRoute, + requestedRouteName: parameterizedRoute, + dataFetchingMethodName: 'getServerSideProps', + }, + ); + + if ('props' in serverSideProps) { + const requestTransaction = getTransactionFromRequest(req); + if (requestTransaction) { + serverSideProps.props._sentryGetServerSidePropsTraceData = requestTransaction.toTraceparent(); + serverSideProps.props._sentryGetServerSidePropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + } + } + + return serverSideProps; } else { - return errorWrappedGetServerSideProps(...getServerSidePropsArguments); + return origGetServerSideProps(...getServerSidePropsArguments); } }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts index 798ceeae2e8f..5477622844a1 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts @@ -1,8 +1,9 @@ import { hasTracingEnabled } from '@sentry/tracing'; +import { serializeBaggage } from '@sentry/utils'; import App from 'next/app'; import { isBuild } from '../../utils/isBuild'; -import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; +import { callTracedServerSideDataFetcher, getTransactionFromRequest, withErrorInstrumentation } from './wrapperUtils'; type AppGetInitialProps = typeof App['getInitialProps']; @@ -30,12 +31,30 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A if (hasTracingEnabled()) { // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return callTracedServerSideDataFetcher(errorWrappedAppGetInitialProps, appGetInitialPropsArguments, req!, res!, { - dataFetcherRouteName: '/_app', - requestedRouteName: context.ctx.pathname, - dataFetchingMethodName: 'getInitialProps', - }); + const appGetInitialProps: { + pageProps: { + _sentryGetInitialPropsTraceData?: string; + _sentryGetInitialPropsBaggage?: string; + }; + } = await callTracedServerSideDataFetcher( + errorWrappedAppGetInitialProps, + appGetInitialPropsArguments, + req!, + res!, + { + dataFetcherRouteName: '/_app', + requestedRouteName: context.ctx.pathname, + dataFetchingMethodName: 'getInitialProps', + }, + ); + + const requestTransaction = getTransactionFromRequest(req!); + if (requestTransaction) { + appGetInitialProps.pageProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); + appGetInitialProps.pageProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + } + + return appGetInitialProps; } else { return errorWrappedAppGetInitialProps(...appGetInitialPropsArguments); } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts index 379b18ad05f8..b2b08fd7e4a7 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts @@ -1,9 +1,10 @@ import { hasTracingEnabled } from '@sentry/tracing'; +import { serializeBaggage } from '@sentry/utils'; import { NextPageContext } from 'next'; import { ErrorProps } from 'next/error'; import { isBuild } from '../../utils/isBuild'; -import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; +import { callTracedServerSideDataFetcher, getTransactionFromRequest, withErrorInstrumentation } from './wrapperUtils'; type ErrorGetInitialProps = (context: NextPageContext) => Promise; @@ -33,12 +34,28 @@ export function withSentryServerSideErrorGetInitialProps( if (hasTracingEnabled()) { // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, errorGetInitialPropsArguments, req!, res!, { - dataFetcherRouteName: '/_error', - requestedRouteName: context.pathname, - dataFetchingMethodName: 'getInitialProps', - }); + const errorGetInitialProps: ErrorProps & { + _sentryGetInitialPropsTraceData?: string; + _sentryGetInitialPropsBaggage?: string; + } = await callTracedServerSideDataFetcher( + errorWrappedGetInitialProps, + errorGetInitialPropsArguments, + req!, + res!, + { + dataFetcherRouteName: '/_error', + requestedRouteName: context.pathname, + dataFetchingMethodName: 'getInitialProps', + }, + ); + + const requestTransaction = getTransactionFromRequest(req!); + if (requestTransaction) { + errorGetInitialProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); + errorGetInitialProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + } + + return errorGetInitialProps; } else { return errorWrappedGetInitialProps(...errorGetInitialPropsArguments); } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 458609a9646b..59d6dfd30198 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -1,8 +1,9 @@ import { hasTracingEnabled } from '@sentry/tracing'; +import { serializeBaggage } from '@sentry/utils'; import { NextPage } from 'next'; import { isBuild } from '../../utils/isBuild'; -import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; +import { callTracedServerSideDataFetcher, getTransactionFromRequest, withErrorInstrumentation } from './wrapperUtils'; type GetInitialProps = Required['getInitialProps']; @@ -29,12 +30,22 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit if (hasTracingEnabled()) { // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { + const initialProps: { + _sentryGetInitialPropsTraceData?: string; + _sentryGetInitialPropsBaggage?: string; + } = await callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { dataFetcherRouteName: context.pathname, requestedRouteName: context.pathname, dataFetchingMethodName: 'getInitialProps', }); + + const requestTransaction = getTransactionFromRequest(req!); + if (requestTransaction) { + initialProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); + initialProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + } + + return initialProps; } else { return errorWrappedGetInitialProps(...getInitialPropsArguments); } diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index 0042143b15f3..c29962a6321c 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -2,7 +2,7 @@ import { captureException, getCurrentHub, startTransaction } from '@sentry/core' import { addRequestDataToEvent } from '@sentry/node'; import { getActiveTransaction } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { fill } from '@sentry/utils'; +import { extractTraceparentData, fill, parseBaggageHeader } from '@sentry/utils'; import * as domain from 'domain'; import { IncomingMessage, ServerResponse } from 'http'; @@ -12,7 +12,14 @@ declare module 'http' { } } -function getTransactionFromRequest(req: IncomingMessage): Transaction | undefined { +/** + * Grabs a transaction off a Next.js datafetcher request object, if it was previously put there via + * `setTransactionOnRequest`. + * + * @param req The Next.js datafetcher request object + * @returns the Transaction on the request object if there is one, or `undefined` if the request object didn't have one. + */ +export function getTransactionFromRequest(req: IncomingMessage): Transaction | undefined { return req._sentryTransaction; } @@ -38,12 +45,15 @@ export function withErrorInstrumentation any>( return function (this: unknown, ...origFunctionArguments: Parameters): ReturnType { try { const potentialPromiseResult = origFunction.call(this, ...origFunctionArguments); + // First of all, we need to capture promise rejections so we have the following check, as well as the try-catch block. // Additionally, we do the following instead of `await`-ing so we do not change the method signature of the passed function from `() => unknown` to `() => Promise. Promise.resolve(potentialPromiseResult).catch(err => { // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. captureException(err); + throw err; }); + return potentialPromiseResult; } catch (e) { // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. @@ -85,13 +95,18 @@ export function callTracedServerSideDataFetcher Pr let requestTransaction: Transaction | undefined = getTransactionFromRequest(req); if (requestTransaction === undefined) { - // TODO: Extract trace data from `req` object (trace and baggage headers) and attach it to transaction + const sentryTraceHeader = req.headers['sentry-trace']; + const baggageHeader = req.headers.baggage; + const traceparentData = + typeof sentryTraceHeader === 'string' ? extractTraceparentData(sentryTraceHeader) : undefined; const newTransaction = startTransaction({ op: 'nextjs.data.server', name: options.requestedRouteName, + ...traceparentData, metadata: { source: 'route', + baggage: baggageHeader ? parseBaggageHeader(baggageHeader) : undefined, }, }); @@ -121,7 +136,6 @@ export function callTracedServerSideDataFetcher Pr } try { - // TODO: Inject trace data into returned props return await origFunction(...origFunctionArguments); } finally { dataFetcherSpan.finish(); diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index 5d2ccb3cb04f..0eba46b9034b 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -22,11 +22,10 @@ type StartTransactionCb = (context: TransactionContext) => Transaction | undefin * Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app. */ interface SentryEnhancedNextData extends NextData { - // contains props returned by `getInitialProps` - except for `pageProps`, these are the props that got returned by `getServerSideProps` or `getStaticProps` props: { - _sentryGetInitialPropsTraceData?: string; // trace parent info, if injected by server-side `getInitialProps` - _sentryGetInitialPropsBaggage?: string; // baggage, if injected by server-side `getInitialProps` pageProps?: { + _sentryGetInitialPropsTraceData?: string; // trace parent info, if injected by server-side `getInitialProps` + _sentryGetInitialPropsBaggage?: string; // baggage, if injected by server-side `getInitialProps` _sentryGetServerSidePropsTraceData?: string; // trace parent info, if injected by server-side `getServerSideProps` _sentryGetServerSidePropsBaggage?: string; // baggage, if injected by server-side `getServerSideProps` @@ -79,25 +78,27 @@ function extractNextDataTagInformation(): NextDataTagInfo { const { page, query, props } = nextData; - // `nextData.page` always contains the parameterized route + // `nextData.page` always contains the parameterized route - except for when an error occurs in a data fetching + // function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the + // parent transaction nextDataTagInfo.route = page; nextDataTagInfo.params = query; - if (props) { - const { pageProps } = props; + if (props && props.pageProps) { + const pageProps = props.pageProps; + const getInitialPropsBaggage = pageProps._sentryGetInitialPropsBaggage; + const getServerSidePropsBaggage = pageProps._sentryGetServerSidePropsBaggage; + const getStaticPropsBaggage = pageProps._sentryGetStaticPropsBaggage; - const getInitialPropsBaggage = props._sentryGetInitialPropsBaggage; - const getServerSidePropsBaggage = pageProps && pageProps._sentryGetServerSidePropsBaggage; - const getStaticPropsBaggage = pageProps && pageProps._sentryGetStaticPropsBaggage; // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. const baggage = getInitialPropsBaggage || getServerSidePropsBaggage || getStaticPropsBaggage; if (baggage) { nextDataTagInfo.baggage = baggage; } - const getInitialPropsTraceData = props._sentryGetInitialPropsTraceData; - const getServerSidePropsTraceData = pageProps && pageProps._sentryGetServerSidePropsTraceData; - const getStaticPropsTraceData = pageProps && pageProps._sentryGetStaticPropsTraceData; + const getInitialPropsTraceData = pageProps._sentryGetInitialPropsTraceData; + const getServerSidePropsTraceData = pageProps._sentryGetServerSidePropsTraceData; + const getStaticPropsTraceData = pageProps._sentryGetStaticPropsTraceData; // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. const traceData = getInitialPropsTraceData || getServerSidePropsTraceData || getStaticPropsTraceData; if (traceData) { From b91ff5024ce8504e338f35f62f518803b540334d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 30 Aug 2022 15:51:25 +0000 Subject: [PATCH 02/12] Parse baggage correctly --- packages/nextjs/src/config/wrappers/wrapperUtils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index c29962a6321c..1e575c19b349 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -2,7 +2,7 @@ import { captureException, getCurrentHub, startTransaction } from '@sentry/core' import { addRequestDataToEvent } from '@sentry/node'; import { getActiveTransaction } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { extractTraceparentData, fill, parseBaggageHeader } from '@sentry/utils'; +import { extractTraceparentData, fill, parseBaggageSetMutability, isString } from '@sentry/utils'; import * as domain from 'domain'; import { IncomingMessage, ServerResponse } from 'http'; @@ -96,17 +96,19 @@ export function callTracedServerSideDataFetcher Pr if (requestTransaction === undefined) { const sentryTraceHeader = req.headers['sentry-trace']; - const baggageHeader = req.headers.baggage; + const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage; const traceparentData = typeof sentryTraceHeader === 'string' ? extractTraceparentData(sentryTraceHeader) : undefined; + const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const newTransaction = startTransaction({ op: 'nextjs.data.server', name: options.requestedRouteName, ...traceparentData, metadata: { source: 'route', - baggage: baggageHeader ? parseBaggageHeader(baggageHeader) : undefined, + baggage, }, }); From 31c51e61fbeb01f14fed87646fcd6f67e2b64dfd Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 30 Aug 2022 15:52:14 +0000 Subject: [PATCH 03/12] Please linter --- packages/nextjs/src/config/wrappers/wrapperUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index 1e575c19b349..332808369947 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -2,7 +2,7 @@ import { captureException, getCurrentHub, startTransaction } from '@sentry/core' import { addRequestDataToEvent } from '@sentry/node'; import { getActiveTransaction } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { extractTraceparentData, fill, parseBaggageSetMutability, isString } from '@sentry/utils'; +import { extractTraceparentData, fill, isString, parseBaggageSetMutability } from '@sentry/utils'; import * as domain from 'domain'; import { IncomingMessage, ServerResponse } from 'http'; From 5b310dd750b01eec7f8c58862cf9231ccedbaa70 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 30 Aug 2022 19:27:06 +0000 Subject: [PATCH 04/12] Fix tests --- packages/nextjs/test/performance/client.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 2a90b7db45a1..7a0ba5d7be90 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -89,9 +89,11 @@ describe('client', () => { '/[user]/posts/[id]', { user: 'lforst', id: '1337', q: '42' }, { - _sentryGetInitialPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', - _sentryGetInitialPropsBaggage: - 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + pageProps: { + _sentryGetInitialPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryGetInitialPropsBaggage: + 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + }, }, true, { From faaba12c2dff125511fa9288adefd3c9d363d927 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 08:42:25 +0000 Subject: [PATCH 05/12] Remove redundant separate data flags --- .../wrappers/withSentryGetServerSideProps.ts | 6 +-- .../withSentryServerSideAppGetInitialProps.ts | 8 ++-- ...ithSentryServerSideErrorGetInitialProps.ts | 8 ++-- .../withSentryServerSideGetInitialProps.ts | 8 ++-- packages/nextjs/src/performance/client.ts | 31 +++---------- .../nextjs/test/performance/client.test.ts | 43 +++---------------- 6 files changed, 29 insertions(+), 75 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index 0f18eb52187e..dee5ce918c45 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -44,14 +44,14 @@ export function withSentryGetServerSideProps( if ('props' in serverSideProps) { const requestTransaction = getTransactionFromRequest(req); if (requestTransaction) { - serverSideProps.props._sentryGetServerSidePropsTraceData = requestTransaction.toTraceparent(); - serverSideProps.props._sentryGetServerSidePropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + serverSideProps.props._sentryTraceData = requestTransaction.toTraceparent(); + serverSideProps.props._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); } } return serverSideProps; } else { - return origGetServerSideProps(...getServerSidePropsArguments); + return errorWrappedGetServerSideProps(...getServerSidePropsArguments); } }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts index 5477622844a1..86cf92fb544e 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts @@ -33,8 +33,8 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object const appGetInitialProps: { pageProps: { - _sentryGetInitialPropsTraceData?: string; - _sentryGetInitialPropsBaggage?: string; + _sentryTraceData?: string; + _sentryBaggage?: string; }; } = await callTracedServerSideDataFetcher( errorWrappedAppGetInitialProps, @@ -50,8 +50,8 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { - appGetInitialProps.pageProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); - appGetInitialProps.pageProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + appGetInitialProps.pageProps._sentryTraceData = requestTransaction.toTraceparent(); + appGetInitialProps.pageProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); } return appGetInitialProps; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts index b2b08fd7e4a7..e7d4c7c5b46c 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts @@ -35,8 +35,8 @@ export function withSentryServerSideErrorGetInitialProps( // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object const errorGetInitialProps: ErrorProps & { - _sentryGetInitialPropsTraceData?: string; - _sentryGetInitialPropsBaggage?: string; + _sentryTraceData?: string; + _sentryBaggage?: string; } = await callTracedServerSideDataFetcher( errorWrappedGetInitialProps, errorGetInitialPropsArguments, @@ -51,8 +51,8 @@ export function withSentryServerSideErrorGetInitialProps( const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { - errorGetInitialProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); - errorGetInitialProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent(); + errorGetInitialProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); } return errorGetInitialProps; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 59d6dfd30198..8837ae121cf5 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -31,8 +31,8 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object const initialProps: { - _sentryGetInitialPropsTraceData?: string; - _sentryGetInitialPropsBaggage?: string; + _sentryTraceData?: string; + _sentryBaggage?: string; } = await callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { dataFetcherRouteName: context.pathname, requestedRouteName: context.pathname, @@ -41,8 +41,8 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { - initialProps._sentryGetInitialPropsTraceData = requestTransaction.toTraceparent(); - initialProps._sentryGetInitialPropsBaggage = serializeBaggage(requestTransaction.getBaggage()); + initialProps._sentryTraceData = requestTransaction.toTraceparent(); + initialProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); } return initialProps; diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index 0eba46b9034b..3b596a0479d7 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -24,17 +24,12 @@ type StartTransactionCb = (context: TransactionContext) => Transaction | undefin interface SentryEnhancedNextData extends NextData { props: { pageProps?: { - _sentryGetInitialPropsTraceData?: string; // trace parent info, if injected by server-side `getInitialProps` - _sentryGetInitialPropsBaggage?: string; // baggage, if injected by server-side `getInitialProps` - _sentryGetServerSidePropsTraceData?: string; // trace parent info, if injected by server-side `getServerSideProps` - _sentryGetServerSidePropsBaggage?: string; // baggage, if injected by server-side `getServerSideProps` - - // The following two values are only injected in a very special case with the following conditions: + _sentryTraceData?: string; // trace parent info, if injected by a data-fetcher + _sentryBaggage?: string; // baggage, if injected by a data-fetcher + // These two values are only injected by `getStaticProps` in a very special case with the following conditions: // 1. The page's `getStaticPaths` method must have returned `fallback: 'blocking'`. // 2. The requested page must be a "miss" in terms of "Incremental Static Regeneration", meaning the requested page has not been generated before. // In this case, a page is requested and only served when `getStaticProps` is done. There is not even a fallback page or similar. - _sentryGetStaticPropsTraceData?: string; // trace parent info, if injected by server-side `getStaticProps` - _sentryGetStaticPropsBaggage?: string; // baggage, if injected by server-side `getStaticProps` }; }; } @@ -85,24 +80,12 @@ function extractNextDataTagInformation(): NextDataTagInfo { nextDataTagInfo.params = query; if (props && props.pageProps) { - const pageProps = props.pageProps; - const getInitialPropsBaggage = pageProps._sentryGetInitialPropsBaggage; - const getServerSidePropsBaggage = pageProps._sentryGetServerSidePropsBaggage; - const getStaticPropsBaggage = pageProps._sentryGetStaticPropsBaggage; - - // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. - const baggage = getInitialPropsBaggage || getServerSidePropsBaggage || getStaticPropsBaggage; - if (baggage) { - nextDataTagInfo.baggage = baggage; + if (props.pageProps._sentryBaggage) { + nextDataTagInfo.baggage = props.pageProps._sentryBaggage; } - const getInitialPropsTraceData = pageProps._sentryGetInitialPropsTraceData; - const getServerSidePropsTraceData = pageProps._sentryGetServerSidePropsTraceData; - const getStaticPropsTraceData = pageProps._sentryGetStaticPropsTraceData; - // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. - const traceData = getInitialPropsTraceData || getServerSidePropsTraceData || getStaticPropsTraceData; - if (traceData) { - nextDataTagInfo.traceParentData = extractTraceparentData(traceData); + if (props.pageProps._sentryTraceData) { + nextDataTagInfo.traceParentData = extractTraceparentData(props.pageProps._sentryTraceData); } } diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 7a0ba5d7be90..279c39ed90fc 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -90,9 +90,8 @@ describe('client', () => { { user: 'lforst', id: '1337', q: '42' }, { pageProps: { - _sentryGetInitialPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', - _sentryGetInitialPropsBaggage: - 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + _sentryTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryBaggage: 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', }, }, true, @@ -112,46 +111,18 @@ describe('client', () => { }, ], [ - 'https://example.com/dynamic', - '/dynamic', + 'https://example.com/some-page', + '/some-page', {}, { pageProps: { - _sentryGetServerSidePropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', - _sentryGetServerSidePropsBaggage: - 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + _sentryTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryBaggage: 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', }, }, true, { - name: '/dynamic', - op: 'pageload', - tags: { - 'routing.instrumentation': 'next-router', - }, - metadata: { - source: 'route', - baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], - }, - traceId: 'c82b8554881b4d28ad977de04a4fb40a', - parentSpanId: 'a755953cd3394d5f', - parentSampled: true, - }, - ], - [ - 'https://example.com/static', - '/static', - {}, - { - pageProps: { - _sentryGetStaticPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', - _sentryGetStaticPropsBaggage: - 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', - }, - }, - true, - { - name: '/static', + name: '/some-page', op: 'pageload', tags: { 'routing.instrumentation': 'next-router', From 0f205ec3cde31324b76a26a52592677c20b73cb1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 09:03:10 +0000 Subject: [PATCH 06/12] Rename test --- .../test/integration/pages/withErrorServerSideProps.tsx | 7 +++++++ .../test/integration/test/server/errorServerSideProps.js | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 packages/nextjs/test/integration/pages/withErrorServerSideProps.tsx diff --git a/packages/nextjs/test/integration/pages/withErrorServerSideProps.tsx b/packages/nextjs/test/integration/pages/withErrorServerSideProps.tsx new file mode 100644 index 000000000000..dda4f8f31dd2 --- /dev/null +++ b/packages/nextjs/test/integration/pages/withErrorServerSideProps.tsx @@ -0,0 +1,7 @@ +const WithServerSidePropsPage = ({ data }: { data: string }) =>

WithServerSidePropsPage {data}

; + +export async function getServerSideProps() { + throw new Error('ServerSideProps Error'); +} + +export default WithServerSidePropsPage; diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.js b/packages/nextjs/test/integration/test/server/errorServerSideProps.js index a6c622de5db5..ab5f743486b5 100644 --- a/packages/nextjs/test/integration/test/server/errorServerSideProps.js +++ b/packages/nextjs/test/integration/test/server/errorServerSideProps.js @@ -4,7 +4,7 @@ const { sleep } = require('../utils/common'); const { getAsync, interceptEventRequest } = require('../utils/server'); module.exports = async ({ url: urlBase, argv }) => { - const url = `${urlBase}/withServerSideProps`; + const url = `${urlBase}/withErrorServerSideProps`; const capturedRequest = interceptEventRequest( { From df1d3d95daa96d97111f2357dd454500dd7f8754 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 09:03:43 +0000 Subject: [PATCH 07/12] Add webpack plugin experiment to integration tests --- packages/nextjs/test/integration/next.config.js | 3 +++ packages/nextjs/test/integration/next10.config.template | 3 +++ packages/nextjs/test/integration/next11.config.template | 3 +++ 3 files changed, 9 insertions(+) diff --git a/packages/nextjs/test/integration/next.config.js b/packages/nextjs/test/integration/next.config.js index c6356f77e3bf..4d32445a3cef 100644 --- a/packages/nextjs/test/integration/next.config.js +++ b/packages/nextjs/test/integration/next.config.js @@ -4,6 +4,9 @@ const moduleExports = { eslint: { ignoreDuringBuilds: true, }, + sentry: { + experiments: { autoWrapDataFetchers: true }, + }, }; const SentryWebpackPluginOptions = { dryRun: true, diff --git a/packages/nextjs/test/integration/next10.config.template b/packages/nextjs/test/integration/next10.config.template index e9e52049a4f3..61aa3332bbc0 100644 --- a/packages/nextjs/test/integration/next10.config.template +++ b/packages/nextjs/test/integration/next10.config.template @@ -5,6 +5,9 @@ const moduleExports = { future: { webpack5: %RUN_WEBPACK_5%, }, + sentry: { + experiments: { autoWrapDataFetchers: true }, + }, }; const SentryWebpackPluginOptions = { diff --git a/packages/nextjs/test/integration/next11.config.template b/packages/nextjs/test/integration/next11.config.template index 5de08b436a67..454d300a8b7b 100644 --- a/packages/nextjs/test/integration/next11.config.template +++ b/packages/nextjs/test/integration/next11.config.template @@ -6,6 +6,9 @@ const moduleExports = { eslint: { ignoreDuringBuilds: true, }, + sentry: { + experiments: { autoWrapDataFetchers: true }, + }, }; const SentryWebpackPluginOptions = { From 3f192f7634522d47152aa389ac3260b831c251c5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 09:26:30 +0000 Subject: [PATCH 08/12] Add clientside getServerSideProps integration test --- .../integration/pages/withServerSideProps.tsx | 2 +- .../client/tracingClientGetServerSideProps.js | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js diff --git a/packages/nextjs/test/integration/pages/withServerSideProps.tsx b/packages/nextjs/test/integration/pages/withServerSideProps.tsx index dda4f8f31dd2..e0220a577586 100644 --- a/packages/nextjs/test/integration/pages/withServerSideProps.tsx +++ b/packages/nextjs/test/integration/pages/withServerSideProps.tsx @@ -1,7 +1,7 @@ const WithServerSidePropsPage = ({ data }: { data: string }) =>

WithServerSidePropsPage {data}

; export async function getServerSideProps() { - throw new Error('ServerSideProps Error'); + return { props: { data: '[some serverSidePropsData data]' } }; } export default WithServerSidePropsPage; diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js new file mode 100644 index 000000000000..4b3f4a5b8337 --- /dev/null +++ b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js @@ -0,0 +1,45 @@ +const { + expectRequestCount, + isTransactionRequest, + expectTransaction, + extractEnvelopeFromRequest, +} = require('../utils/client'); +const assert = require('assert').strict; + +module.exports = async ({ page, url, requests }) => { + await page.goto(`${url}/withServerSideProps`); + await page.waitForRequest(isTransactionRequest); + + const transactionEnvelope = extractEnvelopeFromRequest(requests.transactions[0]); + + expectTransaction(requests.transactions[0], { + contexts: { + trace: { + op: 'pageload', + }, + }, + }); + + const nextDataTag = await page.waitForSelector('#__NEXT_DATA__'); + const nextDataTagValue = JSON.parse(await nextDataTag.evaluate(tag => tag.innerText)); + + assert.strictEqual( + nextDataTagValue.props.pageProps.data, + '[some serverSidePropsData data]', + 'Returned data must contain original data returned from getServerSideProps.', + ); + + assert.strictEqual( + nextDataTagValue.props.pageProps._sentryTraceData.split('-')[0], + transactionEnvelope.envelopeHeader.trace.trace_id, + 'Trace id in envelope header must be the same as in trace parent data returned from getServerSideProps', + ); + + assert.strictEqual( + nextDataTagValue.props.pageProps._sentryBaggage.match(/sentry-trace_id=([a-f0-9]*),/)[1], + transactionEnvelope.envelopeHeader.trace.trace_id, + 'Trace id in envelope header must be the same as in baggage returned from getServerSideProps', + ); + + await expectRequestCount(requests, { transactions: 1 }); +}; From 5a767098cab9759347d6435b0fa0dcf4a304eebc Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 09:38:29 +0000 Subject: [PATCH 09/12] Add clientside getInitialProps integration test --- .../integration/pages/withInitialProps.tsx | 7 +++ .../integration/pages/withServerSideProps.tsx | 2 +- .../client/tracingClientGetInitialProps.js | 45 +++++++++++++++++++ .../client/tracingClientGetServerSideProps.js | 2 +- 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 packages/nextjs/test/integration/pages/withInitialProps.tsx create mode 100644 packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js diff --git a/packages/nextjs/test/integration/pages/withInitialProps.tsx b/packages/nextjs/test/integration/pages/withInitialProps.tsx new file mode 100644 index 000000000000..01b557bdd09f --- /dev/null +++ b/packages/nextjs/test/integration/pages/withInitialProps.tsx @@ -0,0 +1,7 @@ +const WithInitialPropsPage = ({ data }: { data: string }) =>

WithInitialPropsPage {data}

; + +WithInitialPropsPage.getInitialProps = () => { + return { data: '[some getInitialProps data]' }; +}; + +export default WithInitialPropsPage; diff --git a/packages/nextjs/test/integration/pages/withServerSideProps.tsx b/packages/nextjs/test/integration/pages/withServerSideProps.tsx index e0220a577586..486313a9b9c3 100644 --- a/packages/nextjs/test/integration/pages/withServerSideProps.tsx +++ b/packages/nextjs/test/integration/pages/withServerSideProps.tsx @@ -1,7 +1,7 @@ const WithServerSidePropsPage = ({ data }: { data: string }) =>

WithServerSidePropsPage {data}

; export async function getServerSideProps() { - return { props: { data: '[some serverSidePropsData data]' } }; + return { props: { data: '[some getServerSideProps data]' } }; } export default WithServerSidePropsPage; diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js new file mode 100644 index 000000000000..90d7eeceef4a --- /dev/null +++ b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js @@ -0,0 +1,45 @@ +const { + expectRequestCount, + isTransactionRequest, + expectTransaction, + extractEnvelopeFromRequest, +} = require('../utils/client'); +const assert = require('assert').strict; + +module.exports = async ({ page, url, requests }) => { + await page.goto(`${url}/withInitialProps`); + await page.waitForRequest(isTransactionRequest); + + const transactionEnvelope = extractEnvelopeFromRequest(requests.transactions[0]); + + expectTransaction(requests.transactions[0], { + contexts: { + trace: { + op: 'pageload', + }, + }, + }); + + const nextDataTag = await page.waitForSelector('#__NEXT_DATA__'); + const nextDataTagValue = JSON.parse(await nextDataTag.evaluate(tag => tag.innerText)); + + assert.strictEqual( + nextDataTagValue.props.pageProps.data, + '[some getInitialProps data]', + 'Returned data must contain original data returned from getInitialProps.', + ); + + assert.strictEqual( + nextDataTagValue.props.pageProps._sentryTraceData.split('-')[0], + transactionEnvelope.envelopeHeader.trace.trace_id, + 'Trace id in envelope header must be the same as in trace parent data returned from getInitialProps', + ); + + assert.strictEqual( + nextDataTagValue.props.pageProps._sentryBaggage.match(/sentry-trace_id=([a-f0-9]*),/)[1], + transactionEnvelope.envelopeHeader.trace.trace_id, + 'Trace id in envelope header must be the same as in baggage returned from getInitialProps', + ); + + await expectRequestCount(requests, { transactions: 1 }); +}; diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js index 4b3f4a5b8337..0f9ae8e07bac 100644 --- a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js +++ b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js @@ -25,7 +25,7 @@ module.exports = async ({ page, url, requests }) => { assert.strictEqual( nextDataTagValue.props.pageProps.data, - '[some serverSidePropsData data]', + '[some getServerSideProps data]', 'Returned data must contain original data returned from getServerSideProps.', ); From 663f76529c673480ed3779bbaf660e4252811556 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 10:00:26 +0000 Subject: [PATCH 10/12] Add serverside integration tests --- .../pages/{ => [id]}/withInitialProps.tsx | 0 .../pages/{ => [id]}/withServerSideProps.tsx | 0 .../client/tracingClientGetInitialProps.js | 2 +- .../client/tracingClientGetServerSideProps.js | 2 +- .../server/tracingServerGetInitialProps.js | 33 +++++++++++++++++++ .../server/tracingServerGetServerSideProps.js | 33 +++++++++++++++++++ 6 files changed, 68 insertions(+), 2 deletions(-) rename packages/nextjs/test/integration/pages/{ => [id]}/withInitialProps.tsx (100%) rename packages/nextjs/test/integration/pages/{ => [id]}/withServerSideProps.tsx (100%) create mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js create mode 100644 packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js diff --git a/packages/nextjs/test/integration/pages/withInitialProps.tsx b/packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx similarity index 100% rename from packages/nextjs/test/integration/pages/withInitialProps.tsx rename to packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx diff --git a/packages/nextjs/test/integration/pages/withServerSideProps.tsx b/packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx similarity index 100% rename from packages/nextjs/test/integration/pages/withServerSideProps.tsx rename to packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js index 90d7eeceef4a..fae23261b144 100644 --- a/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js +++ b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js @@ -7,7 +7,7 @@ const { const assert = require('assert').strict; module.exports = async ({ page, url, requests }) => { - await page.goto(`${url}/withInitialProps`); + await page.goto(`${url}/42/withInitialProps`); await page.waitForRequest(isTransactionRequest); const transactionEnvelope = extractEnvelopeFromRequest(requests.transactions[0]); diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js index 0f9ae8e07bac..2936ac5420fa 100644 --- a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js +++ b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js @@ -7,7 +7,7 @@ const { const assert = require('assert').strict; module.exports = async ({ page, url, requests }) => { - await page.goto(`${url}/withServerSideProps`); + await page.goto(`${url}/1337/withServerSideProps`); await page.waitForRequest(isTransactionRequest); const transactionEnvelope = extractEnvelopeFromRequest(requests.transactions[0]); diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js new file mode 100644 index 000000000000..368ccb26a952 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js @@ -0,0 +1,33 @@ +const assert = require('assert'); + +const { sleep } = require('../utils/common'); +const { getAsync, interceptTracingRequest } = require('../utils/server'); + +module.exports = async ({ url: urlBase, argv }) => { + const url = `${urlBase}/239/withInitialProps`; + + const capturedRequest = interceptTracingRequest( + { + contexts: { + trace: { + op: 'nextjs.data.server', + }, + }, + transaction: '/[id]/withInitialProps', + transaction_info: { + source: 'route', + }, + type: 'transaction', + request: { + url, + }, + }, + argv, + 'tracingGetInitialProps', + ); + + await getAsync(url); + await sleep(250); + + assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); +}; diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js new file mode 100644 index 000000000000..8f195d337168 --- /dev/null +++ b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js @@ -0,0 +1,33 @@ +const assert = require('assert'); + +const { sleep } = require('../utils/common'); +const { getAsync, interceptTracingRequest } = require('../utils/server'); + +module.exports = async ({ url: urlBase, argv }) => { + const url = `${urlBase}/193/withServerSideProps`; + + const capturedRequest = interceptTracingRequest( + { + contexts: { + trace: { + op: 'nextjs.data.server', + }, + }, + transaction: '/[id]/withServerSideProps', + transaction_info: { + source: 'route', + }, + type: 'transaction', + request: { + url, + }, + }, + argv, + 'tracingServerGetServerSideProps', + ); + + await getAsync(url); + await sleep(250); + + assert.ok(capturedRequest.isDone(), 'Did not intercept expected request'); +}; From cee6dc3f86bdf5c232d0c5886071dd1afd1333ba Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 31 Aug 2022 14:02:22 +0000 Subject: [PATCH 11/12] Add additional assertions --- .../integration/test/client/tracingClientGetInitialProps.js | 3 +++ .../integration/test/client/tracingClientGetServerSideProps.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js index fae23261b144..3c53a0f87654 100644 --- a/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js +++ b/packages/nextjs/test/integration/test/client/tracingClientGetInitialProps.js @@ -29,6 +29,9 @@ module.exports = async ({ page, url, requests }) => { 'Returned data must contain original data returned from getInitialProps.', ); + assert.ok(nextDataTagValue.props.pageProps._sentryTraceData, '_sentryTraceData must exist in __NEXT_DATA__ tag'); + assert.ok(nextDataTagValue.props.pageProps._sentryBaggage, '_sentryBaggage must exist in __NEXT_DATA__ tag'); + assert.strictEqual( nextDataTagValue.props.pageProps._sentryTraceData.split('-')[0], transactionEnvelope.envelopeHeader.trace.trace_id, diff --git a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js index 2936ac5420fa..59c1613ff30f 100644 --- a/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js +++ b/packages/nextjs/test/integration/test/client/tracingClientGetServerSideProps.js @@ -29,6 +29,9 @@ module.exports = async ({ page, url, requests }) => { 'Returned data must contain original data returned from getServerSideProps.', ); + assert.ok(nextDataTagValue.props.pageProps._sentryTraceData, '_sentryTraceData must exist in __NEXT_DATA__ tag'); + assert.ok(nextDataTagValue.props.pageProps._sentryBaggage, '_sentryBaggage must exist in __NEXT_DATA__ tag'); + assert.strictEqual( nextDataTagValue.props.pageProps._sentryTraceData.split('-')[0], transactionEnvelope.envelopeHeader.trace.trace_id, From 019751b1e8b779b738817bce8cae86fb54ac70bb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 1 Sep 2022 09:05:31 +0000 Subject: [PATCH 12/12] Fix error capturing --- .../nextjs/src/config/wrappers/wrapperUtils.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index 332808369947..a7be6784dbf6 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -38,23 +38,15 @@ function autoEndTransactionOnResponseEnd(transaction: Transaction, res: ServerRe /** * Wraps a function that potentially throws. If it does, the error is passed to `captureException` and rethrown. + * + * Note: This function turns the wrapped function into an asynchronous one. */ export function withErrorInstrumentation any>( origFunction: F, -): (...params: Parameters) => ReturnType { - return function (this: unknown, ...origFunctionArguments: Parameters): ReturnType { +): (...params: Parameters) => Promise> { + return async function (this: unknown, ...origFunctionArguments: Parameters): Promise> { try { - const potentialPromiseResult = origFunction.call(this, ...origFunctionArguments); - - // First of all, we need to capture promise rejections so we have the following check, as well as the try-catch block. - // Additionally, we do the following instead of `await`-ing so we do not change the method signature of the passed function from `() => unknown` to `() => Promise. - Promise.resolve(potentialPromiseResult).catch(err => { - // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. - captureException(err); - throw err; - }); - - return potentialPromiseResult; + return await origFunction.call(this, ...origFunctionArguments); } catch (e) { // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. captureException(e);