Conversation
This was referenced Aug 18, 2022
Closed
getInitialProps of _app, _document and errorgetInitialProps of _app, _document and _error
lforst
commented
Aug 18, 2022
Contributor
size-limit report 📦
|
lobsterkatie
approved these changes
Aug 19, 2022
| withSentryServerSideErrorGetInitialProps, | ||
| } from "@sentry/nextjs";`; | ||
|
|
||
| if (parameterizedRouteName === '/_app') { |
Member
There was a problem hiding this comment.
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'];
}
Contributor
Author
| origGetInitialProps: GetInitialProps, | ||
| parameterizedRoute: string, | ||
| ): GetInitialProps { | ||
| export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInitialProps): GetInitialProps { |
Member
There was a problem hiding this comment.
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.
Comment on lines
+75
to
+81
| /** 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; |
Member
There was a problem hiding this comment.
Nice - this makes it really clear!
3141759 to
a561697
Compare
d2a0a55 to
0a4271b
Compare
Base automatically changed from
lforst-create-transactions-for-getinitialprops-and-getserversideprops
to
master
August 19, 2022 14:51
00a8058 to
d3f0311
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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!: