renderToHTML swallowing unhandled exceptions

[bringking]

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

While running in production mode, throwing an unhandled exception during getInitialProps should re-throw during renderToHTML

Current Behavior

when rendering a route using renderToHTML and an unhandled exception is throw in getInitialProps, renderToHTML should re-throw the error.

In many cases with a custom server (see SSR Caching example) you want to cache the HTML after it is generated. This is a common use case for renderToHTML. However, if an unhandled exception is thrown during that process, the HTML is still returned.

desired behavior

 try {
      const html = await app.renderToHTML(req, res, '/some-route', params);
      // cache
      doSomeCaching(html);
      // send
      res.send(html);
    } catch (e) {
      // Something happened during the render, pass it up the chain
      next(e);
    }

Steps to Reproduce (for bugs)

Here is a repository with a barebones recreation. Running under production mode, I would expect this method to catch errors.
https://github.com/bringking/error-example-nextjs/blob/master/server.js#L40

Context

What I was trying to accomplish is any unhandled exception thrown during server render would trigger an express error handler and render the _error.js page.

Your Environment

Tech	Version
next	4.1.4
node	8.9.0
OS	MacOS
browser
etc

[vjpr]

As a workaround, you could check res.statusCode == 500.

Here is how renderToHTML is implemented in Next.js for server-side:

  async renderToHTML (req, res, pathname, query) {
    try {
      const out = await renderToHTML(req, res, pathname, query, this.renderOpts)
      return out
    } catch (err) {
      if (err.code === 'ENOENT') {
        res.statusCode = 404
        return this.renderErrorToHTML(null, req, res, pathname, query)
      } else {
        if (!this.quiet) console.error(err)
        res.statusCode = 500
        return this.renderErrorToHTML(err, req, res, pathname, query)
      }
    }
  }

And next.js render method which is recommended when using a custom server.

  async render (req, res, pathname, query, parsedUrl) {
    if (isInternalUrl(req.url)) {
      return this.handleRequest(req, res, parsedUrl)
    }

    if (BLOCKED_PAGES.indexOf(pathname) !== -1) {
      return await this.render404(req, res, parsedUrl)
    }

    const html = await this.renderToHTML(req, res, pathname, query)
    if (isResSent(res)) {
      return
    }

    if (this.nextConfig.poweredByHeader) {
      res.setHeader('X-Powered-By', 'Next.js ' + process.env.NEXT_VERSION)
    }
    return sendHTML(req, res, html, req.method, this.renderOpts)
  }

---

renderToHTML should be named differently. It is confusing having this.renderToHTML and then importing renderToHTML.

---

renderToHTML and render should return {html, err}.

[briefguo]

I met the same, @vjpr have any example to catch the error when using a custom server ?

[briefguo]

http://localhost:3005/housekeeper/services works well
http://localhost:3005/housekeeper/services/ catch the error

Have you encountered such a problem? @vjpr

[timneutkens]

We're not planning to change the custom server usage at this point in time as we're working on covering all cases for custom servers without a custom server (which improves performance).

[balazsorban44]

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.