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

Add “red box” on any JavaScript errors in development #783

Closed
gaearon opened this issue Sep 27, 2016 · 25 comments
Closed

Add “red box” on any JavaScript errors in development #783

gaearon opened this issue Sep 27, 2016 · 25 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2016

This is likely to cause some controversy so I’d love to hear the arguments against doing this.

I want to enable “red box” on any JavaScript error in development, similar to the proposal in facebook/react#1753. When an uncaught exception is thrown, an error is shown full screen similar to our syntax overlay (#744). It shows the error message and the stack. The full-screen message can be dismissed by pressing “Escape” in case you still want to interact with a broken app. The error is also printed in the console, like before.

We can use https://github.com/commissure/redbox-react although I’d prefer to have full control over this box in our monorepo to quickly patch bugs and improve the output.

Why I want to do this:

  • React is currently not tolerant to errors thrown inside components. Its internal state often gets corrupted by this, so we want to surface errors early and prominently.
  • We already do this on React Native to great success.
  • In CRA we control the environment and we can do this.
  • This is a part of my plan to bring hot reloading to CRA: Status/Roadmap of the project gaearon/react-hot-boilerplate#97 (comment).

Open questions:

  • How to capture all errors with stacks in development. Does window.onerror provide Error objects in modern browsers now?
  • What to do about unhandled Promise rejections. Ideally we want to surface them given how common this problem and misunderstanding is.
  • What do do about errors inside React that accidentally get caught, like No error is shown when use a undefined variable react#7617 (comment). Even if the code is technically valid, React just doesn’t support this pattern. Any error inside setState() or ReactDOM.render() is going to mess it up. We could monkey-patch them to report errors, or add some hooks in React itself to surface them.

Feedback and thoughts welcome.

@gaelollivier
Copy link
Contributor

First: I love the idea and the overall "master plan" to build a fully integrated developer experience, that's the reason I'm using CRA and its future seems bright ☀️

I just have one use case where I wouldn't want unhandled Promise rejections to throw a big red error:
In my app, I have middlewares that handle some side effects such as fetching data to the server and globally handling "fatal errors" such as 500 errors (in this case we just explain the user something unexpectedly went wrong).
The middleware responsible for fetching the data also allows the caller to get the request Promise in case he wants to do something with it. So I may request data like this:

componentDidMount() {
   this.props.dispatch({ type: 'FETCH_USER' }).then((result) => {
     // Might want to do someting here, or simply let reducers handle the result
   })
}

However, if FETCH_USER fails with a "fatal error", the promise will reject and the rejection won't be handled anywhere. A middleware will handle the error and display a message, but I don't want to manually catch the error here because the handling of "unexpected errors" is the responsibility of the middleware.

Is there something wrong with this pattern ? To me it's seems ok that some promises get rejected.

@dlebedynskyi
Copy link

dlebedynskyi commented Sep 27, 2016

Open questions:
Does window.onerror provide Error objects in modern browsers now?

Don't think so. Since it is by spec https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onerror. Also there is CORS issues there.

What to do about unhandled Promise rejections. Ideally we want to surface them given how common this problem and misunderstanding is.

Check this http://www.2ality.com/2016/04/unhandled-rejections.html. Chrome ( at least https://googlechrome.github.io/samples/promise-rejection-events/) has support for unhandled rejections.

What do do about errors inside React that accidentally get caught,...

they are caught for a reason( probably). So can be considered handled. otherwise we can get a redbox for expected code.

@davidpfahler
Copy link

Regarding the question of using redbox-react or something custom stored in this monorepo:

I would be glad to help in any way I can to make this solution based on redbox-react. Monorepo or not, I don't think that fragmenting solutions for exactly the same problem is good for the community. Whatever steps we need to take to keep this solution universal, I'm willing to take (add contributors, etc.). Please let everybody benefit from the improvements redbox will receive from being integrated into CRA.

What would you deem necessary to make this work?

@gaearon
Copy link
Contributor Author

gaearon commented Sep 28, 2016

I think redbox-react might be a good start, and we can potentially look at other solutions if we find issues. One thing I‘m not confident about is whether we want the redbox to be based on React itself.

At this point it’s best if somebody can come up with a prototype for the integration that handle uncaught errors in browsers. I‘m not sure how to implement this without window.onerror. Do people use “zones” for it these days?

@agrcrobles
Copy link

@gaearon do you think that will it be possible to pre-render before emitting the error event in a similar way they do it in the storybook?
https://github.com/kadirahq/react-storybook/blob/master/src/client/preview/render.js#L23

@gaearon
Copy link
Contributor Author

gaearon commented Oct 10, 2016

@agrcrobles I'm not entirely sure what you are asking, can you clarify the question?

@agrcrobles
Copy link

I am just thinking ( not sure if possible ) that instead of subscribing to a window error event, wrap the component that is attempting to be rendered

const WrappedComponent = (...args) => {
    try {
        return renderComponentFunctionHere(...args);
    } catch (error) {
        return <RedBox {...props} error={error}/>;
    }
};
return <WrappedComponent />;
};

@gaearon
Copy link
Contributor Author

gaearon commented Oct 10, 2016

We want to surface all errors, not just ones that occurred during component rendering. It's also hard to statically find all components (I tried it before, and I don't think it's a good approach).

@volkanunsal
Copy link

Redbox can be very obnoxious. The color aside, I'd prefer to see the errors in the console and step through them. With Redbox, all I get is the line numbers, which aren't even right since cheap source maps devtool doesn't work in Chrome currently.

@davidpfahler
Copy link

@volkanunsal We'll be adding additional logging to the console commissure/redbox-react#77

@glennreyes
Copy link
Contributor

glennreyes commented Oct 20, 2016

How to capture all errors with stacks in development. Does window.onerror provide Error objects in modern browsers now?

What about putting the whole app into a try-catch block to get more control over all JS errors?

Something like:

src/index.js:

import React from 'react';
import ReactDOM from 'react-dom';
...

render(<App />, document.getElementById('root'));

document.body.something(); // let's throw this error

src/ErrorClient.js

try {
  require('./index.js'); // App entry point
} catch (error) {
  // Let's do here what ever we want with JS errors
  console.log(error); // TypeError: document.body.something is not a function(…)
}

Just throwing ideas ...

@davidpfahler
Copy link

@volkanunsal the console logging issues fell through, unfortunately. Do we still need this for THIS issue? I doubt it, we can always add that later.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 28, 2016

The color aside, I'd prefer to see the errors in the console and step through them. With Redbox, all I get is the line numbers, which aren't even right since cheap source maps devtool doesn't work in Chrome currently.

None of these are problems with the idea of catching errors and displaying them. These are problems with one particular way of doing so.

There is no reason why we couldn't:

  • Use a different color
  • Figure out a way to fix line numbers
  • Figure out a way to show more useful info, e.g. a code snippet around failing lines

Please unleash your fantasy 😉 . We don't need to think in terms of existing solutions, we need think about the desired experience, and then figure out a way to make it possible.

@volkanunsal
Copy link

The most important thing for me is that the error screen doesn't interfere with Chrome console. I recall that when I had Redbox enabled, I couldn't use the Chrome console at all. That's probably more important than color or code snippets around failing lines... Ideally, I would jump between the error screen and the Chrome console easily because that's where I do all of my debugging.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 28, 2016

Can you recall what was happening with Chrome console? That’s the first time I’m hearing about it. In any case @Timer is not using that same exact project in #1101 so I would be surprised if it could somehow break console.

@volkanunsal
Copy link

volkanunsal commented Nov 28, 2016

If memory serves right, the error messages were being swallowed and output to the RedBox screen. When I would go to the console and check off "Pause on exceptions", it wouldn't work.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 28, 2016

Hmm then it wasn't rethrowing them. Shouldn't be an issue in #1101 since it uses window.onerror.
Still, good to know, thanks for the information!

@Timer
Copy link
Contributor

Timer commented Nov 28, 2016

I don't call Error#preventDefault() or whatever mechanism(s) exists to prevent the error from going to the console. The error is displayed in the console and in the red box. I think it's important to leave the ability to debug from the console since it's so powerful.

@volkanunsal
Copy link

@Timer My memory of that might be wrong about that then. I think the reason I couldn't interact with the console is it was catching an earlier error than the one I was interested in. I'll try to reproduce it next weekend.

@Pajn
Copy link
Contributor

Pajn commented Nov 28, 2016

@volkanunsal You might have done the same thing as I the last time I tried redbox which is catching the errors and displaying them, this way you can not break on them and the doesn't show up in the console which is pretty bad. But as done in #1101 it only listens for uncaught exceptions so it should not have those downsides.

@brandonmikeska
Copy link

Could we also add a console to the browser? I know in rails you could start debugging right in the browser checking the variables. Just an idea for a feature?

Besides that idea, are we any closer to say yes this will be added or not it won't be?

@gaearon
Copy link
Contributor Author

gaearon commented Feb 24, 2017

Yes, we plan to add this.

@brandonmikeska
Copy link

Awesome, if there are some actionable items of some sort I wouldn't mind trying to help.

@Timer
Copy link
Contributor

Timer commented Feb 26, 2017

@brandonmikeska we're going to merge the first version of this soon into master, so I'm sure some stuff will come up during testing! 😄 Check in frequently.

@Timer
Copy link
Contributor

Timer commented Mar 7, 2017

Merged into master via #1101.

@Timer Timer closed this as completed Mar 7, 2017
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants