From 016e2381c535d2f5242235f36f2d0ee0a72d100a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sat, 1 Oct 2022 18:25:20 +0000 Subject: [PATCH 1/2] fix(nextjs): Don't assert existence of `req` and `res` in data-fetching wrappers --- .../withSentryServerSideAppGetInitialProps.ts | 26 ++++++++----------- ...SentryServerSideDocumentGetInitialProps.ts | 26 +++++++------------ ...ithSentryServerSideErrorGetInitialProps.ts | 26 ++++++++----------- .../withSentryServerSideGetInitialProps.ts | 12 +++++---- 4 files changed, 39 insertions(+), 51 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts index 74e811f8f224..00a81ae4850a 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts @@ -28,27 +28,23 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A const errorWrappedAppGetInitialProps = withErrorInstrumentation(origAppGetInitialProps); - 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 + // 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 + // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher + // span with eachother when there are no req or res objects, we simply do not trace them at all here. + if (hasTracingEnabled() && req && res) { const appGetInitialProps: { pageProps: { _sentryTraceData?: string; _sentryBaggage?: string; }; - } = await callTracedServerSideDataFetcher( - errorWrappedAppGetInitialProps, - appGetInitialPropsArguments, - req!, - res!, - { - dataFetcherRouteName: '/_app', - requestedRouteName: context.ctx.pathname, - dataFetchingMethodName: 'getInitialProps', - }, - ); + } = await callTracedServerSideDataFetcher(errorWrappedAppGetInitialProps, appGetInitialPropsArguments, req, res, { + dataFetcherRouteName: '/_app', + requestedRouteName: context.ctx.pathname, + dataFetchingMethodName: 'getInitialProps', + }); - const requestTransaction = getTransactionFromRequest(req!); + const requestTransaction = getTransactionFromRequest(req); if (requestTransaction) { appGetInitialProps.pageProps._sentryTraceData = requestTransaction.toTraceparent(); diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts index f914a43a3916..cf20cfd5e42d 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts @@ -29,22 +29,16 @@ export function withSentryServerSideDocumentGetInitialProps( const errorWrappedGetInitialProps = withErrorInstrumentation(origDocumentGetInitialProps); - 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 - return callTracedServerSideDataFetcher( - errorWrappedGetInitialProps, - documentGetInitialPropsArguments, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - req!, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - res!, - { - dataFetcherRouteName: '/_document', - requestedRouteName: context.pathname, - dataFetchingMethodName: 'getInitialProps', - }, - ); + // 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 + // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher + // span with eachother when there are no req or res objects, we simply do not trace them at all here. + if (hasTracingEnabled() && req && res) { + return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, documentGetInitialPropsArguments, req, res, { + dataFetcherRouteName: '/_document', + requestedRouteName: context.pathname, + dataFetchingMethodName: 'getInitialProps', + }); } else { return errorWrappedGetInitialProps(...documentGetInitialPropsArguments); } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts index 8950fcf720a0..66e2a465f643 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts @@ -31,25 +31,21 @@ export function withSentryServerSideErrorGetInitialProps( const errorWrappedGetInitialProps = withErrorInstrumentation(origErrorGetInitialProps); - 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 + // 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 + // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher + // span with eachother when there are no req or res objects, we simply do not trace them at all here. + if (hasTracingEnabled() && req && res) { const errorGetInitialProps: ErrorProps & { _sentryTraceData?: string; _sentryBaggage?: string; - } = await callTracedServerSideDataFetcher( - errorWrappedGetInitialProps, - errorGetInitialPropsArguments, - req!, - res!, - { - dataFetcherRouteName: '/_error', - requestedRouteName: context.pathname, - dataFetchingMethodName: 'getInitialProps', - }, - ); + } = await callTracedServerSideDataFetcher(errorWrappedGetInitialProps, errorGetInitialPropsArguments, req, res, { + dataFetcherRouteName: '/_error', + requestedRouteName: context.pathname, + dataFetchingMethodName: 'getInitialProps', + }); - const requestTransaction = getTransactionFromRequest(req!); + const requestTransaction = getTransactionFromRequest(req); if (requestTransaction) { errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent(); diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index f591a4048c85..31d08e336d7c 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -27,19 +27,21 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit const errorWrappedGetInitialProps = withErrorInstrumentation(origGetInitialProps); - 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 + // 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 + // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher + // span with eachother when there are no req or res objects, we simply do not trace them at all here. + if (hasTracingEnabled() && req && res) { const initialProps: { _sentryTraceData?: string; _sentryBaggage?: string; - } = await callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { + } = await callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req, res, { dataFetcherRouteName: context.pathname, requestedRouteName: context.pathname, dataFetchingMethodName: 'getInitialProps', }); - const requestTransaction = getTransactionFromRequest(req!); + const requestTransaction = getTransactionFromRequest(req); if (requestTransaction) { initialProps._sentryTraceData = requestTransaction.toTraceparent(); From 57410ed4d8fddc4f186887b9f39688e1340cc93f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 3 Oct 2022 07:38:00 +0000 Subject: [PATCH 2/2] grammar mistakes man --- .../config/wrappers/withSentryServerSideAppGetInitialProps.ts | 2 +- .../wrappers/withSentryServerSideDocumentGetInitialProps.ts | 2 +- .../config/wrappers/withSentryServerSideErrorGetInitialProps.ts | 2 +- .../src/config/wrappers/withSentryServerSideGetInitialProps.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts index 00a81ae4850a..3af686c18618 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts @@ -31,7 +31,7 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A // 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 // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher - // span with eachother when there are no req or res objects, we simply do not trace them at all here. + // span with each other when there are no req or res objects, we simply do not trace them at all here. if (hasTracingEnabled() && req && res) { const appGetInitialProps: { pageProps: { diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts index cf20cfd5e42d..31c16ec247a7 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts @@ -32,7 +32,7 @@ export function withSentryServerSideDocumentGetInitialProps( // 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 // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher - // span with eachother when there are no req or res objects, we simply do not trace them at all here. + // span with each other when there are no req or res objects, we simply do not trace them at all here. if (hasTracingEnabled() && req && res) { return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, documentGetInitialPropsArguments, req, res, { dataFetcherRouteName: '/_document', diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts index 66e2a465f643..7b4be0cde29d 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts @@ -34,7 +34,7 @@ export function withSentryServerSideErrorGetInitialProps( // 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 // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher - // span with eachother when there are no req or res objects, we simply do not trace them at all here. + // span with each other when there are no req or res objects, we simply do not trace them at all here. if (hasTracingEnabled() && req && res) { const errorGetInitialProps: ErrorProps & { _sentryTraceData?: string; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 31d08e336d7c..8f8a0b4b8608 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -30,7 +30,7 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit // 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 // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher - // span with eachother when there are no req or res objects, we simply do not trace them at all here. + // span with each other when there are no req or res objects, we simply do not trace them at all here. if (hasTracingEnabled() && req && res) { const initialProps: { _sentryTraceData?: string;