Skip to content
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

If a "page prop" has a circular structure, a silent unhandled promise rejection will occur in NextScript#render #4812

Closed
vjpr opened this issue Jul 20, 2018 · 9 comments

Comments

@vjpr
Copy link

vjpr commented Jul 20, 2018

Bug report

Describe the bug

If a "page prop" has a circular structure, a silent unhandled promise rejection will occur in NextScript#render

To Reproduce

To reproduce, add a custom _error page, and return the err object in getInitialProps. This object has a circular structure. Or create an object with circular structure.

export default class Page extends React.Component {
  static getInitialProps({res, err}) {
    const statusCode = res ? res.statusCode : err ? err.statusCode : null
    return {statusCode, err}
  }

Use a custom server, and add require('hard-rejection')() to the start of your server code.

Then make a syntax error on a page.

You should see the following message:

TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at module.exports (xxx/node_modules/.registry.npmjs.org/htmlescape/1.1.1/node_modules/htmlescape/htmlescape.js:32:15)
    at NextScript.render (xxx/node_modules/.registry.npmjs.org/@vjpr/next/6.1.0-vjpr.1/node_modules/@vjpr/next/dist/server/document.js:300:81)
    at processChild (xxx/node_modules/.registry.npmjs.org/react-dom/16.4.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2204:18)
    at resolve (xxx/node_modules/.registry.npmjs.org/react-dom/16.4.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2061:5)
    at ReactDOMServerRenderer.render (xxx/node_modules/.registry.npmjs.org/react-dom/16.4.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2380:22)
    at ReactDOMServerRenderer.read (xxx/node_modules/.registry.npmjs.org/react-dom/16.4.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2354:19)
    at renderToStaticMarkup (xxx/node_modules/.registry.npmjs.org/react-dom/16.4.1/node_modules/react-dom/cjs/react-dom-server.node.development.js:2737:25)
    at _callee3$ (xxx/node_modules/.registry.npmjs.org/@vjpr/next/6.1.0-vjpr.1/node_modules/@vjpr/next/dist/server/render.js:341:100)
    at tryCatch (xxx/node_modules/.registry.npmjs.org/regenerator-runtime/0.11.1/node_modules/regenerator-runtime/runtime.js:62:40)
    at Generator.invoke [as _invoke] (xxx/node_modules/.registry.npmjs.org/regenerator-runtime/0.11.1/node_modules/regenerator-runtime/runtime.js:296:22)
    at Generator.prototype.(anonymous function) [as next] (xxx/node_modules/.registry.npmjs.org/regenerator-runtime/0.11.1/node_modules/regenerator-runtime/runtime.js:114:21)
    at step (xxx/node_modules/.registry.npmjs.org/@babel/runtime/7.0.0-beta.49/node_modules/@babel/runtime/helpers/asyncToGenerator.js:12:30)
    at _next (xxx/node_modules/.registry.npmjs.org/@babel/runtime/7.0.0-beta.49/node_modules/@babel/runtime/helpers/asyncToGenerator.js:27:9)
    at <anonymous> {caller: [args (xxx/packages/public/framework/server-init/index.js:144:13)]}

I tried to catch it in _document.js#componentDidCatch but it did not work.

It was very difficult to trace down the cause of this...and hence why my error page would never show (I didn't realize it was broken).

Expected behavior

Helpful message should be shown indicating there is a circular dep issue, and what property is the cause of it.

Also should not be an unhandled promise rejection...

Potential solution

Try/catch should be placed around htmlescape, and an error message should be

PS. Why doesn't componentDidCatch in _document catch this? Is it something to do with _document only running SS?

System information

  • OS: [e.g. macOS, Windows] macOS
  • Browser (if applies) [e.g. chrome, safari] chrome
  • Version of Next.js: [e.g. 6.0.2] 6.1
@timneutkens
Copy link
Member

componentDidCatch has no support for SSR, that's why handling it there doesn't work.

@timneutkens
Copy link
Member

timneutkens commented Jul 20, 2018

I wonder why err would be a circular structure. It'd mean that there's something non-standard on the err object 🤔

Either way we should catch errors when rendering _error, so this is a bug

@vjpr
Copy link
Author

vjpr commented Jul 20, 2018

{ ModuleBuildError: Module build failed: SyntaxError: xxx/pages/with-react-router.js:
...
 module:
   NormalModule {
     errors: [ [Circular] ],
     error: [Circular],

webpackDevMiddleware config - turned off quiet and noInfo

{
  "publicPath": "/_next/webpack/",
  "noInfo": false,
  "quiet": false,
  "clientLogLevel": "info",
  "watchOptions": {
    "ignored": [
      {},
      {}
    ]
  }
}

@timneutkens
Copy link
Member

🤔 that's a compilation error coming from webpack, I wonder how it would call _error, as that shouldn't be even rendered in development. So that'd mean the issue might be fixed by #4639 as I changed a lot of the logic behind rendering the error page.

@vjpr
Copy link
Author

vjpr commented Jul 20, 2018

componentDidCatch has no support for SSR, that's why handling it there doesn't work.

Maybe we can check this and alert the user.

@vjpr
Copy link
Author

vjpr commented Jul 21, 2018

This might be useful for dealing with circular stuff: https://www.npmjs.com/package/circular-json

@vjpr
Copy link
Author

vjpr commented Aug 1, 2018

Ran into this again with the nextI18n example. Its very easy to accidentally pass a circular json when working with getInitialProps.

@timneutkens
Copy link
Member

@nisargrthakkar you're not providing any context, it's unlikely you're running into the same issue as Vaughan, and if that is the case GitHub has the 👍 feature to make that know 🙏

@timneutkens
Copy link
Member

@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants