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

"Error: null" in the captured error warning due to cross-origin issues #10321

Closed
gaearon opened this Issue Jul 29, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@gaearon
Member

gaearon commented Jul 29, 2017

Three points from discussion in reduxjs/react-redux#756:

  • Due to https://bugs.chromium.org/p/chromium/issues/detail?id=701371, environments like CodeSandbox show Error: null in our “React caught an error” warning. Let’s consider skipping both as they’re not very useful and look like a bug. Error: null looks like a separate error and draws attention away from the real error just above.

  • In any case we should probably not print The error was thrown at: if we don’t have a JS stack.

  • There is a related issue. What do we pass to error boundary if we caught null? It seems that most error boundaries will intuitively do something like if (this.state.error) or read error.message, which null will break. There are other falsy values too like '' and undefined. My proposal is we always wrap primitives into our own Error objects that we pass to the boundary.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jul 29, 2017

Member

I'm not a huge fan of the magical behavior of automatically wrapping. Also takes us further away from just being try/catch semantics.

If you have the if (this.state.error) fail, you'll probably hit an infinite loop error instead that you can fix.

Just build a proper HoC once and be done with it. :)

Member

sebmarkbage commented Jul 29, 2017

I'm not a huge fan of the magical behavior of automatically wrapping. Also takes us further away from just being try/catch semantics.

If you have the if (this.state.error) fail, you'll probably hit an infinite loop error instead that you can fix.

Just build a proper HoC once and be done with it. :)

@acdlite

This comment has been minimized.

Show comment
Hide comment
@acdlite

acdlite Jul 29, 2017

Member

Tend to agree with @sebmarkbage about automatic wrapping.

If you have the if (this.state.error) fail, you'll probably hit an infinite loop error instead that you can fix.

Isn't the more likely scenario that the error would propagate to the next boundary, and eventually to the root? Not sure where the infinite loop comes into play.

I do think we should use a custom error for cross-origin errors (!errorEvent.isTrusted) (Edit: nevermind, that doesn't mean what I thought it did), since that is DEV-only behavior and does not match try/catch semantics.

Member

acdlite commented Jul 29, 2017

Tend to agree with @sebmarkbage about automatic wrapping.

If you have the if (this.state.error) fail, you'll probably hit an infinite loop error instead that you can fix.

Isn't the more likely scenario that the error would propagate to the next boundary, and eventually to the root? Not sure where the infinite loop comes into play.

I do think we should use a custom error for cross-origin errors (!errorEvent.isTrusted) (Edit: nevermind, that doesn't mean what I thought it did), since that is DEV-only behavior and does not match try/catch semantics.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jul 29, 2017

Member

Ah, you're right. The bubbling should happen.

Agreed. If we can correctly differentiate between "null because not trusted" and "null because throw null/undefined" then we can pass a custom error in the former case.

Member

sebmarkbage commented Jul 29, 2017

Ah, you're right. The bubbling should happen.

Agreed. If we can correctly differentiate between "null because not trusted" and "null because throw null/undefined" then we can pass a custom error in the former case.

@gaearon gaearon referenced this issue Jul 30, 2017

Closed

React 16 RC #10294

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jul 30, 2017

Member

Another similar report, not clear to me yet if it’s caused by the same issue or not.
#10294 (comment)

Member

gaearon commented Jul 30, 2017

Another similar report, not clear to me yet if it’s caused by the same issue or not.
#10294 (comment)

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jul 30, 2017

Member

I assume serving scripts from file:/// might trigger this?

Member

sebmarkbage commented Jul 30, 2017

I assume serving scripts from file:/// might trigger this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment