Skip to content

Commit 889f02d

Browse files
committed
remove workaround
1 parent d2800c1 commit 889f02d

File tree

1 file changed

+11
-20
lines changed

1 file changed

+11
-20
lines changed

packages/nextjs/src/utils/_error.ts

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { captureException, withScope } from '@sentry/core';
22
import { getCurrentHub } from '@sentry/hub';
3-
import { addExceptionMechanism, addRequestDataToEvent, objectify } from '@sentry/utils';
3+
import { addExceptionMechanism, addRequestDataToEvent } from '@sentry/utils';
44
import { NextPageContext } from 'next';
55

66
type ContextOrProps = {
@@ -31,15 +31,14 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
3131
return Promise.resolve();
3232
}
3333

34-
// Nextjs only passes the pathname in the context data given to `getInitialProps`, not the main render function, but
35-
// unlike `req` and `res`, for which that also applies, it passes it on both server and client.
36-
//
37-
// TODO: This check is only necessary because of the workaround for https://github.com/vercel/next.js/issues/8592
38-
// explained below. Once that's fixed, we'll have to keep the `inGetInitialProps` check, because lots of people will
39-
// still call this function in their custom error component's `render` function, but we can get rid of the check for
40-
// `err` and just always bail if we're not in `getInitialProps`.
41-
const inGetInitialProps = contextOrProps.pathname !== undefined;
42-
if (!inGetInitialProps && !err) {
34+
// In previous versions of the suggested `_error.js` page in which this function is meant to be used, there was a
35+
// workaround for https://github.com/vercel/next.js/issues/8592 which involved an extra call to this function, in the
36+
// custom error component's `render` method, just in case it hadn't been called by `getInitialProps`. Now that that
37+
// issue has been fixed, the second call is unnecessary, but since it lives in user code rather than our code, users
38+
// have to be the ones to get rid of it, and guaraneteedly, not all of them will. So, rather than capture the error
39+
// twice, we just bail if we sense we're in that now-extraneous second call. (We can tell which function we're in
40+
// because Nextjs passes `pathname` to `getInitialProps` but not to `render`.)
41+
if (!contextOrProps.pathname) {
4342
return Promise.resolve();
4443
}
4544

@@ -49,8 +48,7 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
4948
type: 'instrument',
5049
handled: true,
5150
data: {
52-
// TODO: Get rid of second half of ternary once https://github.com/vercel/next.js/issues/8592 is fixed.
53-
function: inGetInitialProps ? '_error.getInitialProps' : '_error.customErrorComponent',
51+
function: '_error.getInitialProps',
5452
},
5553
});
5654
return event;
@@ -62,14 +60,7 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
6260

6361
// If third-party libraries (or users themselves) throw something falsy, we want to capture it as a message (which
6462
// is what passing a string to `captureException` will wind up doing)
65-
const finalError = err || `_error.js called with falsy error (${err})`;
66-
67-
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
68-
// store a seen flag on it. (Because of https://github.com/vercel/next.js/issues/8592, it can happen that the custom
69-
// error component's `getInitialProps` won't have run, so we have people call this function in their error
70-
// component's main render function in addition to in its `getInitialProps`, just in case. By forcing it to be an
71-
// object, we can flag it as seen, so that if we hit this a second time, we can no-op.)
72-
captureException(objectify(finalError));
63+
captureException(err || `_error.js called with falsy error (${err})`);
7364
});
7465

7566
// In case this is being run as part of a serverless function (as is the case with the server half of nextjs apps

0 commit comments

Comments
 (0)