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

Client error handler should not run on initial render #994

Closed
bryanrsmith opened this issue Feb 5, 2017 · 17 comments
Closed

Client error handler should not run on initial render #994

bryanrsmith opened this issue Feb 5, 2017 · 17 comments

Comments

@bryanrsmith
Copy link
Contributor

I have a site that only supports modern browsers. Old browsers can still access the content thanks to server rendering, which is great. The problem is that Next currently renders an error page when an exception is thrown during render.

So, in an old browser:

  1. Navigate to the site
  2. Server renders the page
  3. Client displays the server-rendered page
  4. Client runs the client script. The page component uses an API like Object.assign, causing an exception to be thrown in the old browser.
  5. Next switches to the error view, hiding the original content.

My preference would be for Next to ignore exceptions during the initial page load so that old browsers will continue displaying the server-rendered content. I can work around this by loading polyfills (that I don't need for my target users), but it'd be nice if I could avoid this.

FWIW, I ran into this because Googlebot apparently runs a very old version of Chrome, and was seeing the error page after one of my views tried to use Object.assign.

@timneutkens
Copy link
Member

I'm wondering why Object.assign would not work on older browsers since we use babel to transpile it 🤔

@bryanrsmith
Copy link
Contributor Author

bryanrsmith commented Feb 5, 2017 via email

@timneutkens
Copy link
Member

cc @arunoda

@arunoda
Copy link
Contributor

arunoda commented Feb 6, 2017

@bryanrsmith @timneutkens Object.assign issue is pretty old and related to glamor. It's being fixed.
(May be you need to use 2.0.0-beta.)

@arunoda
Copy link
Contributor

arunoda commented Feb 6, 2017

If you just wanna display client side code for older browsers, we've an option.
See: https://github.com/zeit/next.js#custom-document

With this, you can identify the older browsers on the server and do not include <NextScript /> for those browsers.

@arunoda arunoda closed this as completed Feb 6, 2017
@bryanrsmith
Copy link
Contributor Author

bryanrsmith commented Feb 6, 2017 via email

@arunoda
Copy link
Contributor

arunoda commented Feb 6, 2017

@bryanrsmith Sure.
In that case, you could try this: #994 (comment)

If that didn't work for you, go ahead and re-open this issue with more info.

@bryanrsmith
Copy link
Contributor Author

@arunoda Thanks, but I can accomplish the desired result by tracking router events and throwing within a custom error handler to prevent it from displaying on initial render. I just opened this issue because I think the default behavior is undesirable, and it's not entirely trivial to avoid. If you think the current behavior should remain it's no problem for me :-)

@sedubois
Copy link
Contributor

sedubois commented Feb 6, 2017

I do think it would be a shame to display by default an error page in the browser when the browser initially receives valid HTML but then generates an error it actually can't do anything about. The current behavior sounds more like a bug.

@arunoda
Copy link
Contributor

arunoda commented Feb 6, 2017

@sedubois It's not like that.
That means, Next.js cannot run on that browser.
So, in the first place you should not allow Next.js to run on that browser.

We've ways to do that: #994 (comment)

Anyway if we got an error, we need to display it.

@sedubois
Copy link
Contributor

sedubois commented Feb 6, 2017

Ok, I understand.

@bryanrsmith
Copy link
Contributor Author

bryanrsmith commented Feb 6, 2017

That means, Next.js cannot run on that browser.

Actually, Next.js can run just fine. It's app code in the page component that's failing.

you can identify the older browsers on the server and do not include <NextScript /> for those browsers.

Sorry, but I don't think this is a viable option. This would require tracking which JS/browser APIs are required by the application, and looking up the UA in a browser support table to check compatibility on every request... that's pretty complicated and probably unnecessary.

Anyway if we got an error, we need to display it.

I disagree. We would need to display the error if it prevented the content from being shown to the user, but in this case the user was shown the server-rendered content for a moment and then slapped with an error page. That's a bad user experience.

Here's the custom error handler I'm using to avoid this issue: https://gist.github.com/bryanrsmith/36b0faab1ec11bdf677ec982b2abce89

I totally understand if you don't want to change the default behavior of Next.js, but I wonder if anything could be done to make it easier to accomplish this. Maybe Next.js could pass a value to the error handler's getInitialProps that could be used to determine if the error happened during the initial client render (which would let me delete all the silly Router event handler code at the top). It might also be nice to document a mechanism for preventing the error view from running-- I don't know if throwing an exception there could cause future problems.

@tvararu
Copy link

tvararu commented Apr 24, 2017

So, in the first place you should not allow Next.js to run on that browser.

I agree with @bryanrsmith that this isn't a great solution.

The current approach that Next.js adopts isn't great for progressive enhancement. Users get perfectly working HTML, but if their browser encounters a runtime JavaScript error, then the rug is pulled from underneath, and the entire page is replaced with a full screen error. Users with JavaScript turned off get a better experience in this situation!

Is there an up to date workaround to disable this behaviour? The throw err inside getInitialProps hack that Bryan showed doesn't work in the latest version of Next.js.

@hyperborea
Copy link

Sad to see this ticket has been closed with no real solution. I've been running into missing polyfills a couple of times now and have found no good way to debug / error log "an unexpected error has occurred" exceptions caused by issues in client side rendering. It would be nice to at least fallback to the SSR version in these cases.

I understand as well if you don't want to change default behaviour of next, but some toggle or escape hatch would be really appreciated.

@lkbr
Copy link

lkbr commented Dec 18, 2017

Agree with @hyperborea. Also having challenges with this. We have audiences in emerging markets using browsers like opera mini and UC browser which are already challenging enough to debug.

@hansinhu
Copy link

That's a bad user experience.

@vzaidman
Copy link
Contributor

vzaidman commented Jun 22, 2018

maybe this is related #2468

@lock lock bot locked as resolved and limited conversation to collaborators Jun 22, 2019
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

9 participants