-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Comments
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: componentDidMount() {
this.props.dispatch({ type: 'FETCH_USER' }).then((result) => {
// Might want to do someting here, or simply let reducers handle the result
})
} However, if Is there something wrong with this pattern ? To me it's seems ok that some promises get rejected. |
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.
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.
they are caught for a reason( probably). So can be considered handled. otherwise we can get a redbox for expected code. |
Regarding the question of using I would be glad to help in any way I can to make this solution based on What would you deem necessary to make this work? |
I think 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 |
@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? |
@agrcrobles I'm not entirely sure what you are asking, can you clarify the question? |
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
|
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). |
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. |
@volkanunsal We'll be adding additional logging to the console commissure/redbox-react#77 |
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 ... |
@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. |
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:
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. |
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. |
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. |
Hmm then it wasn't rethrowing them. Shouldn't be an issue in #1101 since it uses |
I don't call |
@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. |
@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. |
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? |
Yes, we plan to add this. |
Awesome, if there are some actionable items of some sort I wouldn't mind trying to help. |
@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. |
Merged into master via #1101. |
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:
Open questions:
window.onerror
provideError
objects in modern browsers now?setState()
orReactDOM.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.
The text was updated successfully, but these errors were encountered: