_app getInitialProps not called when rendering error page

[seanconnollydev]

Bug report

Describe the bug

When the error page is rendered (either custom _error page or the default), the _app getInitialProps function is not called. As a result any rendering that expects props to be present behaves unexpectedly. This can be especially problematic in scenarios where _app is wrapped by higher order components (as demonstrated in the Apollo example)

To Reproduce

Repo (with steps to reproduce in Readme): https://github.com/goldenshun/next-error-page-props
Live Example: https://next-error-page-props-krqkeinaul.now.sh/

Expected behavior

I expect initial props for the _app to be be hydrated when rendering the error page.

Screenshots

System information

• macOS
• Chrome
• Version of Next.js: 6.0.3

Additional context

This unexpected behavior caused a bug for us where attempts to render the error page resulted in a subsequent error because the expectation was that the props passed via getInitialProps would be present. We have several higher order components wrapping our _app component, so we needed to add special error checking logic just for the scenario when the error page was being rendered.

[timneutkens]

Created a pull request: seanconnollydev/next-error-page-props#1

App is responsible for implementing getInitialProps of pages, this includes the error page, so if you don't implement it correctly, it breaks.

[seanconnollydev]

@timneutkens I appreciate the PR, but your changes do not fix the issue. I believe the root of this issue is that getInitialProps is not called when the error page is rendered.

This may be intentional given this block of code: https://github.com/zeit/next.js/blob/86d01706a67cee5c80796974d04c1e11cdff453a/client/index.js#L173

So is that your expectation? When rendering the error page, we should not expect getInitialProps to run on the app component?

[timneutkens]

That specific case you're mentioning is an edge case in development where the following events happen:

1. you throw an error
2. you fix it
3. page re-renders but there are no previous props

So for normal pages the following happens:

1. you request a page
2. page component is fetched (if not already prefetched)
3. loadGetInitialProps is called on App with Component being the page component. This is done in the router.
4. App implements getInitialProps of the page Component
5. We continue to render the App component with the result of loadGetInitialProps
6. Page shows

There is no case where getInitialProps gets called on anything other than the App component.

However, after fixing your example (the HOC) it seemed to work fine for me.

[seanconnollydev]

@timneutkens Just to clarify, when you say it worked fine, were you able to see the correct message after running npm run build and npm start? After clicking the button, I am still seeing this:

This should log a value of 123: undefined

but if what you say is true, I should see this:

This should log a value of 123: 123

[timneutkens]

I actually checked against the 404 page.

[jamesreggio]

I can confirm this issue, and I've dug in pretty deeply. I'm going to restate some things here from the initial issue description to help clarify and isolate the problematic code.

What's wrong

This problem is specific to errors that happen on the client after the initial mounting of the component. (The router has special logic to handle exceptions thrown in getInitialProps during a client-side navigation, and I've confirmed this logic is correct.)

Specifically, if the page is mounted, and you raise an exception on the page, the exception will cause the error page to be mounted without ever invoking getInitialProps on the new App/Error page pairing.

@goldenshun's original repro illustrates this nicely, though the use of a HOC is unnecessary.

Why is it broken

This regression was introduced two months ago in #4156, where the invocation of getInitialProps was removed from the app's top-level error handler. Specifically, this line was removed and replaced by a comment that says that "App will handle the calling of getInitialProps".

I believe the sentiment about "App will handle calling getInitialProps" is mistaken. In fact, it really doesn't make sense on its face, since it would require an instance lifecycle method of App (which is mounted immediately after the comment) to invoke the static getInitialProps method on the error page.

How to fix

I've fixed this in a fork by restoring Lines 146 – 148 that were removed in #4156. I think this is the right fix, but Next.js's handling of getInitialProps could certainly be improved. (The code in this conditional speaks to the unnecessary complexity around this.)

[jamesreggio]

Here's the fix that has been working for me: jamesreggio@e19ec0d

It causes a regression in a test, though.

[jamesreggio]

I fixed the regression and opened a PR here: #4764.