Add React Error boundary

A JavaScript error in a part of the UI shouldn’t break the whole app. To solve this problem for React users, React 16 introduces a new concept of an “error boundary”.

Error boundaries are React components that catch JavaScript errors anywhere in their child component tree, log those errors, and display a fallback UI instead of the component tree that crashed. Error boundaries catch errors during rendering, in lifecycle methods, and in constructors of the whole tree below them.

I’ve used the Empty State design from patternfly-react: https://www.patternfly.org/v4/components/empty-state/design-guidelines#back-end-failure

Here is the PR: https://github.com/theforeman/foreman/pull/8671

and this is how it looks like at the moment:

any suggestions or concerns?

Thanks,
Ron

3 Likes

Much needed functionality :+1:

Added also a copy-to-clipboard section with the logs:

@tbrisker also suggested that we would use this error template for sever-side errors.
which we will do as a follow-up, opened redmine: Refactor #33052: Use React error boundary template for server side errors - Foreman

1 Like

Will this be able to catch errors such as the dreaded e[t] is undefined[1]?

[1] - Bug #27345: Products page sometimes does not load - Katello - Foreman

Also the non-react page talks about request UUID and a rake task to fetch more details. See foreman/500.html.erb at develop · theforeman/foreman · GitHub for what it all contains. Should we keep similar information there? While this error is on the frontend side, it may be interesting to connect it to related backend request (if there was some), OTOH it may also be misleading, if you performed many client side steps meanwhile.

the React error boundary works as a try/catch in React render methods,
so if that error (which actually seems like a bundle error (?)) causes a React component not to render,
than the error boundary should catch it.

OTOH it may also be misleading, if you performed many client side steps meanwhile.

imho if the server returned the error I think we should display only server message,

Should we keep similar information there?

I think it would be great if we can pass the actual server logs into the component, can we run that rake task and return its log output for the user?

Well the reason we added the rake task was, some users complained they don’t want to show backtraces to users. So we instead started to give instructions on how to get such traceback when they report an issue (and requires shell access to the machine, which typically sysadmins have)

makes sense, then I guess that if there’s an immediate error we should display that,
additionally, we can mention how to get the rest of the logs.