-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): Instrument server-side getInitialProps of _app, _document and _error
#5604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
getInitialProps of _app, _document and errorgetInitialProps of _app, _document and _error
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| withSentryServerSideErrorGetInitialProps, | ||
| } from "@sentry/nextjs";`; | ||
|
|
||
| if (parameterizedRouteName === '/_app') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this is moved over to the proxy template, what would think of using a map for all of these checks? Something like
const getInitialPropsWrappers = {
"/_app": Sentry.withSentryServerSideAppGetInitialProps,
"/_document": Sentry.withSentryServerSideDocumentGetInitialProps,
"/_error": Sentry.withSentryServerSideAppGetInitialProps,
}
const getInitialPropsWrapper = getInitialPropsWrappers["__ROUTE__"] || Sentry.withSentryServerSideErrorGetInitialProps
if (typeof origGetInitialProps === 'function') {
pageComponent.getInitialProps = getInitialPropsWrapper(
origGetInitialProps,
'__ROUTE__',
) as NextPageComponent['getInitialProps'];
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| origGetInitialProps: GetInitialProps, | ||
| parameterizedRoute: string, | ||
| ): GetInitialProps { | ||
| export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInitialProps): GetInitialProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, maybe this will solve the problem I was having here, which forced me to do the typecast. I spent entirely too much time trying to bend TS to my will on that one and failed entirely.
| /** Parameterized route of the request - will be used for naming the transaction. */ | ||
| requestedRouteName: string; | ||
| /** Name of the route the data fetcher was defined in - will be used for describing the data fetcher's span. */ | ||
| dataFetcherRouteName: string; | ||
| /** Name of the data fetching method - will be used for describing the data fetcher's span. */ | ||
| dataFetchingMethodName: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice - this makes it really clear!
3141759 to
a561697
Compare
d2a0a55 to
0a4271b
Compare
00a8058 to
d3f0311
Compare
Ref: #5505
This PR adds wrappers for the "special page's" (
_app,_documentanderror)getInitialProps. Luckily we have access to the parameterized route ingetInitialPropsso we can reuse the logic introduced in #5593.Allows for transactions like the following:
And correctly captures exceptions when different errors are thrown in different methods!: