From b299be764f60188dbf4030acba9aca1493001e04 Mon Sep 17 00:00:00 2001 From: Vladimir Gorej Date: Thu, 25 Nov 2021 13:47:22 +0100 Subject: [PATCH] fix: introduce Error Boundaries to handle unexpected failures (#7671) Two new components have been updated via plugin system: ErrorBoundary and Fallback. These components can be overridden by user plugins. Refs #7647 --- config/jest/jest.unit.config.js | 1 + src/core/components/layouts/base.jsx | 10 +-- src/core/plugins/view/error-boundary.jsx | 45 +++++++++++++ src/core/plugins/view/fallback.jsx | 13 ++++ src/core/plugins/view/index.js | 9 ++- src/core/plugins/view/root-injects.jsx | 86 +++++++++--------------- src/standalone/layout.jsx | 32 ++++----- test/unit/core/system/system.jsx | 50 ++++++++++---- 8 files changed, 157 insertions(+), 89 deletions(-) create mode 100644 src/core/plugins/view/error-boundary.jsx create mode 100644 src/core/plugins/view/fallback.jsx diff --git a/config/jest/jest.unit.config.js b/config/jest/jest.unit.config.js index 7c5305c2100..15cfc9c3af7 100644 --- a/config/jest/jest.unit.config.js +++ b/config/jest/jest.unit.config.js @@ -18,6 +18,7 @@ module.exports = { '/test/unit/components/online-validator-badge.jsx', '/test/unit/components/live-response.jsx', ], + silent: true, // set to `false` to allow console.* calls to be printed transformIgnorePatterns: [ '/node_modules/(?!(react-syntax-highlighter)/)' ] diff --git a/src/core/components/layouts/base.jsx b/src/core/components/layouts/base.jsx index 1709a362c10..505d2956b9b 100644 --- a/src/core/components/layouts/base.jsx +++ b/src/core/components/layouts/base.jsx @@ -28,6 +28,7 @@ export default class BaseLayout extends React.Component { const SchemesContainer = getComponent("SchemesContainer", true) const AuthorizeBtnContainer = getComponent("AuthorizeBtnContainer", true) const FilterContainer = getComponent("FilterContainer", true) + const ErrorBoundary = getComponent("ErrorBoundary", true) let isSwagger2 = specSelectors.isSwagger2() let isOAS3 = specSelectors.isOAS3() @@ -36,7 +37,7 @@ export default class BaseLayout extends React.Component { const loadingStatus = specSelectors.loadingStatus() let loadingMessage = null - + if(loadingStatus === "loading") { loadingMessage =
@@ -85,8 +86,8 @@ export default class BaseLayout extends React.Component { const hasSecurityDefinitions = !!specSelectors.securityDefinitions() return ( -
+ }> @@ -119,7 +120,8 @@ export default class BaseLayout extends React.Component { -
- ) + +
+ ) } } diff --git a/src/core/plugins/view/error-boundary.jsx b/src/core/plugins/view/error-boundary.jsx new file mode 100644 index 00000000000..a3f3524ebee --- /dev/null +++ b/src/core/plugins/view/error-boundary.jsx @@ -0,0 +1,45 @@ +import PropTypes from "prop-types" +import React, { Component } from "react" + +import Fallback from "./fallback" + +export class ErrorBoundary extends Component { + constructor(props) { + super(props) + this.state = { hasError: false, error: null } + } + + static getDerivedStateFromError(error) { + return { hasError: true, error } + } + + componentDidCatch(error, errorInfo) { + console.error(error, errorInfo) // eslint-disable-line no-console + } + + render() { + const { getComponent, targetName, children } = this.props + const FallbackComponent = getComponent("Fallback") + + if (this.state.hasError) { + return + } + + return children + } +} +ErrorBoundary.propTypes = { + targetName: PropTypes.string, + getComponent: PropTypes.func, + children: PropTypes.oneOfType([ + PropTypes.arrayOf(PropTypes.node), + PropTypes.node, + ]) +} +ErrorBoundary.defaultProps = { + targetName: "this component", + getComponent: () => Fallback, + children: null, +} + +export default ErrorBoundary diff --git a/src/core/plugins/view/fallback.jsx b/src/core/plugins/view/fallback.jsx new file mode 100644 index 00000000000..0b020da0e73 --- /dev/null +++ b/src/core/plugins/view/fallback.jsx @@ -0,0 +1,13 @@ +import React from "react" +import PropTypes from "prop-types" + +const Fallback = ({ name }) => ( +
+ 😱 Could not render { name === "t" ? "this component" : name }, see the console. +
+) +Fallback.propTypes = { + name: PropTypes.string.isRequired, +} + +export default Fallback diff --git a/src/core/plugins/view/index.js b/src/core/plugins/view/index.js index fb8cd748872..e376f884638 100644 --- a/src/core/plugins/view/index.js +++ b/src/core/plugins/view/index.js @@ -1,6 +1,9 @@ import * as rootInjects from "./root-injects" import { memoize } from "core/utils" +import ErrorBoundary from "./error-boundary" +import Fallback from "./fallback" + export default function({getComponents, getStore, getSystem}) { let { getComponent, render, makeMappedContainer } = rootInjects @@ -14,6 +17,10 @@ export default function({getComponents, getStore, getSystem}) { getComponent: memGetComponent, makeMappedContainer: memMakeMappedContainer, render: render.bind(null, getSystem, getStore, getComponent, getComponents), - } + }, + components: { + ErrorBoundary, + Fallback, + }, } } diff --git a/src/core/plugins/view/root-injects.jsx b/src/core/plugins/view/root-injects.jsx index 7fdb8fe0bde..fdc0d20da42 100644 --- a/src/core/plugins/view/root-injects.jsx +++ b/src/core/plugins/view/root-injects.jsx @@ -1,20 +1,24 @@ import React, { Component } from "react" -import PropTypes from "prop-types" import ReactDOM from "react-dom" import { connect, Provider } from "react-redux" import omit from "lodash/omit" const SystemWrapper = (getSystem, ComponentToWrap ) => class extends Component { render() { - return + return } } -const RootWrapper = (reduxStore, ComponentToWrap) => class extends Component { +const RootWrapper = (getSystem, reduxStore, ComponentToWrap) => class extends Component { render() { + const { getComponent } = getSystem() + const ErrorBoundary = getComponent("ErrorBoundary", true) + return ( - + + + ) } @@ -30,7 +34,7 @@ const makeContainer = (getSystem, component, reduxStore) => { let wrappedWithSystem = SystemWrapper(getSystem, component, reduxStore) let connected = connect( mapStateToProps )(wrappedWithSystem) if(reduxStore) - return RootWrapper(reduxStore, connected) + return RootWrapper(getSystem, reduxStore, connected) return connected } @@ -66,73 +70,43 @@ export const makeMappedContainer = (getSystem, getStore, memGetComponent, getCom } export const render = (getSystem, getStore, getComponent, getComponents, domNode) => { - let App = (getComponent(getSystem, getStore, getComponents, "App", "root")) - ReactDOM.render(( ), domNode) + let App = getComponent(getSystem, getStore, getComponents, "App", "root") + ReactDOM.render(, domNode) } -class ErrorBoundary extends Component { - constructor(props) { - super(props) - this.state = { hasError: false, error: null } - } - - static getDerivedStateFromError(error) { - return { hasError: true, error } - } - - componentDidCatch(error, errorInfo) { - console.error(error, errorInfo) // eslint-disable-line no-console - } - +/** + * Creates a class component from a stateless one and wrap it with Error Boundary + * to handle errors coming from a stateless component. + */ +const createClass = (getSystem, OriginalComponent) => class extends Component { render() { - if (this.state.hasError) { - return - } + const { getComponent } = getSystem() + const ErrorBoundary = getComponent("ErrorBoundary") - return this.props.children - } -} -ErrorBoundary.propTypes = { - targetName: PropTypes.string, - children: PropTypes.oneOfType([ - PropTypes.arrayOf(PropTypes.node), - PropTypes.node, - ]) -} -ErrorBoundary.defaultProps = { - targetName: "this component", - children: null, -} - -const Fallback = ({ name }) => ( -
- 😱 Could not render { name === "t" ? "this component" : name }, see the console. -
-) -Fallback.propTypes = { - name: PropTypes.string.isRequired, -} - -// Render try/catch wrapper -const createClass = OriginalComponent => class extends Component { - render() { return ( - + ) } } -const wrapRender = (component) => { +const wrapRender = (getSystem, component) => { const isStateless = component => !(component.prototype && component.prototype.isReactComponent) - const target = isStateless(component) ? createClass(component) : component + const target = isStateless(component) ? createClass(getSystem, component) : component const { render: oriRender} = target.prototype + /** + * This render method override handles errors that are throw in render method + * of class components. + */ target.prototype.render = function render(...args) { try { return oriRender.apply(this, args) } catch (error) { + const { getComponent } = getSystem() + const Fallback = getComponent("Fallback") + console.error(error) // eslint-disable-line no-console return } @@ -159,11 +133,11 @@ export const getComponent = (getSystem, getStore, getComponents, componentName, } if(!container) - return wrapRender(component) + return wrapRender(getSystem, component) if(container === "root") return makeContainer(getSystem, component, getStore()) // container == truthy - return makeContainer(getSystem, wrapRender(component)) + return makeContainer(getSystem, wrapRender(getSystem, component)) } diff --git a/src/standalone/layout.jsx b/src/standalone/layout.jsx index b91bb4dc412..0afd65e0db0 100644 --- a/src/standalone/layout.jsx +++ b/src/standalone/layout.jsx @@ -1,5 +1,3 @@ - - import React from "react" import PropTypes from "prop-types" @@ -16,27 +14,29 @@ export default class StandaloneLayout extends React.Component { } render() { - let { getComponent } = this.props - - let Container = getComponent("Container") - let Row = getComponent("Row") - let Col = getComponent("Col") - + const { getComponent } = this.props + const Container = getComponent("Container") + const Row = getComponent("Row") + const Col = getComponent("Col") const Topbar = getComponent("Topbar", true) const BaseLayout = getComponent("BaseLayout", true) const OnlineValidatorBadge = getComponent("onlineValidatorBadge", true) + const ErrorBoundary = getComponent("ErrorBoundary", true) return ( - - {Topbar ? : null} - - - - - - + + {Topbar ? : null} + + + + + + + + + ) } diff --git a/test/unit/core/system/system.jsx b/test/unit/core/system/system.jsx index 43ded037ee0..6366ff42f03 100644 --- a/test/unit/core/system/system.jsx +++ b/test/unit/core/system/system.jsx @@ -940,7 +940,7 @@ describe("bound system", function(){ expect(renderedComponent.text()).toEqual("😱 Could not render BrokenComponent, see the console.") }) - it("should catch errors thrown inside of pure component render methods", function() { + it("should catch errors thrown inside of pure component", function() { // Given class BrokenComponent extends PureComponent { // eslint-disable-next-line react/require-render-return @@ -962,16 +962,16 @@ describe("bound system", function(){ // When let Component = system.getSystem().getComponent("BrokenComponent") - const renderedComponent = render() + const wrapper = mount() // Then - expect(renderedComponent.text()).toEqual("😱 Could not render BrokenComponent, see the console.") + expect(wrapper.text()).toEqual("😱 Could not render BrokenComponent, see the console.") }) - it("should catch errors thrown inside of stateless component functions", function() { + it("should catch errors thrown inside of stateless component function", function() { // Given // eslint-disable-next-line react/require-render-return - let BrokenComponent = function BrokenComponent() { throw new Error("This component is broken") } + const BrokenComponent = () => { throw new Error("This component is broken") } const system = new System({ plugins: [ ViewPlugin, @@ -985,15 +985,14 @@ describe("bound system", function(){ // When const Component = system.getSystem().getComponent("BrokenComponent") - const renderedComponent = mount() + const wrapper = mount() - expect(renderedComponent.text().startsWith("😱 Could not render")).toEqual(true) + expect(wrapper.text()).toEqual("😱 Could not render BrokenComponent, see the console.") }) - it("should catch errors thrown inside of container components", function() { + it("should catch errors thrown inside of container created from class component", function() { // Given class BrokenComponent extends React.Component { - // eslint-disable-next-line react/require-render-return render() { throw new Error("This component is broken") } @@ -1011,15 +1010,42 @@ describe("bound system", function(){ }) // When - let Component = system.getSystem().getComponent("BrokenComponent", true) - const renderedComponent = render( + const Component = system.getSystem().getComponent("BrokenComponent", true) + const wrapper = render( ) // Then - expect(renderedComponent.text()).toEqual("😱 Could not render BrokenComponent, see the console.") + expect(wrapper.text()).toEqual("😱 Could not render BrokenComponent, see the console.") + }) + + it("should catch errors thrown inside of container created from stateless component function", function() { + // Given + const BrokenComponent = () => { throw new Error("This component is broken") } + + const system = new System({ + plugins: [ + ViewPlugin, + { + components: { + BrokenComponent + } + } + ] + }) + + // When + const Component = system.getSystem().getComponent("BrokenComponent", true) + const wrapper = mount( + + + + ) + + // Then + expect(wrapper.text()).toEqual("😱 Could not render BrokenComponent, see the console.") }) }) })