From 35c84c929ae6866177ea41179da4cb8c3853cbe2 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 13 Mar 2019 11:54:33 +0100 Subject: [PATCH 1/6] Add Error Boundary for Pages --- .../src/errorboundary/PageErrorBoundary.js | 32 +++++++++++++++++++ .../packages/ui-components/src/layout/Page.js | 17 ++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js diff --git a/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js b/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js new file mode 100644 index 0000000000..2be112455e --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js @@ -0,0 +1,32 @@ +//@flow + +import * as React from "react"; + + +class PageErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + + /** + * This lifecycle is invoked after an error has been thrown by a descendant component. + * It receives the error that was thrown as a parameter and should return a value to update state. + * @param error + * @returns {{hasError: boolean}} + */ + static getDerivedStateFromError(error) { + // Update state so the next render will show the fallback UI. + return { hasError: true }; + } + + render() { + if (this.state.hasError) { + return

Something went wrong.

; + } + + return this.props.children; + } +} + +export default PageErrorBoundary; diff --git a/scm-ui-components/packages/ui-components/src/layout/Page.js b/scm-ui-components/packages/ui-components/src/layout/Page.js index eea9f66f72..37913a009c 100644 --- a/scm-ui-components/packages/ui-components/src/layout/Page.js +++ b/scm-ui-components/packages/ui-components/src/layout/Page.js @@ -7,6 +7,7 @@ import Subtitle from "./Subtitle"; import injectSheet from "react-jss"; import classNames from "classnames"; import PageActions from "./PageActions"; +import PageErrorBoundary from "../errorboundary/PageErrorBoundary"; type Props = { title?: string, @@ -31,13 +32,15 @@ class Page extends React.Component { render() { const { error } = this.props; return ( -
-
- {this.renderPageHeader()} - - {this.renderContent()} -
-
+ +
+
+ {this.renderPageHeader()} + + {this.renderContent()} +
+
+ ); } From 13e7ae24bd0c5f4fb4fa52847d8f0b16396f0b1d Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 13 Mar 2019 21:41:08 +0100 Subject: [PATCH 2/6] Add Error Boundary for - the Page component - the Main component --- .../ui-components/src/ErrorBoundary.js | 28 +++ .../src/errorboundary/PageErrorBoundary.js | 32 --- .../packages/ui-components/src/index.js | 1 + .../packages/ui-components/src/layout/Page.js | 6 +- scm-ui/src/containers/Main.js | 182 +++++++++--------- 5 files changed, 124 insertions(+), 125 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/ErrorBoundary.js delete mode 100644 scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js diff --git a/scm-ui-components/packages/ui-components/src/ErrorBoundary.js b/scm-ui-components/packages/ui-components/src/ErrorBoundary.js new file mode 100644 index 0000000000..f955f03c49 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/ErrorBoundary.js @@ -0,0 +1,28 @@ +// @flow +import React from "react"; +import ErrorNotification from "./ErrorNotification"; + +class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = { error: null, errorInfo: null }; + } + + componentDidCatch(error, errorInfo) { + // Catch errors in any components below and re-render with error message + this.setState({ + error: error, + errorInfo: errorInfo + }); + } + + render() { + if (this.state.errorInfo) { + return ( + + ); + } + return this.props.children; + } +} +export default ErrorBoundary; diff --git a/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js b/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js deleted file mode 100644 index 2be112455e..0000000000 --- a/scm-ui-components/packages/ui-components/src/errorboundary/PageErrorBoundary.js +++ /dev/null @@ -1,32 +0,0 @@ -//@flow - -import * as React from "react"; - - -class PageErrorBoundary extends React.Component { - constructor(props) { - super(props); - this.state = { hasError: false }; - } - - /** - * This lifecycle is invoked after an error has been thrown by a descendant component. - * It receives the error that was thrown as a parameter and should return a value to update state. - * @param error - * @returns {{hasError: boolean}} - */ - static getDerivedStateFromError(error) { - // Update state so the next render will show the fallback UI. - return { hasError: true }; - } - - render() { - if (this.state.hasError) { - return

Something went wrong.

; - } - - return this.props.children; - } -} - -export default PageErrorBoundary; diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index 39f3769441..61a40eddda 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -25,6 +25,7 @@ export { default as Tooltip } from "./Tooltip"; export { getPageFromMatch } from "./urls"; export { default as Autocomplete} from "./Autocomplete"; export { default as BranchSelector } from "./BranchSelector"; +export { default as ErrorBoundary } from "./ErrorBoundary"; export { apiClient } from "./apiclient.js"; export * from "./errors"; diff --git a/scm-ui-components/packages/ui-components/src/layout/Page.js b/scm-ui-components/packages/ui-components/src/layout/Page.js index 37913a009c..2cab7a1b0a 100644 --- a/scm-ui-components/packages/ui-components/src/layout/Page.js +++ b/scm-ui-components/packages/ui-components/src/layout/Page.js @@ -7,7 +7,7 @@ import Subtitle from "./Subtitle"; import injectSheet from "react-jss"; import classNames from "classnames"; import PageActions from "./PageActions"; -import PageErrorBoundary from "../errorboundary/PageErrorBoundary"; +import ErrorBoundary from "../ErrorBoundary"; type Props = { title?: string, @@ -32,7 +32,7 @@ class Page extends React.Component { render() { const { error } = this.props; return ( - +
{this.renderPageHeader()} @@ -40,7 +40,7 @@ class Page extends React.Component { {this.renderContent()}
- + ); } diff --git a/scm-ui/src/containers/Main.js b/scm-ui/src/containers/Main.js index 6ce4e982fa..9036f769f2 100644 --- a/scm-ui/src/containers/Main.js +++ b/scm-ui/src/containers/Main.js @@ -9,7 +9,7 @@ import Users from "../users/containers/Users"; import Login from "../containers/Login"; import Logout from "../containers/Logout"; -import { ProtectedRoute } from "@scm-manager/ui-components"; +import {ErrorBoundary, ProtectedRoute} from "@scm-manager/ui-components"; import {binder, ExtensionPoint } from "@scm-manager/ui-extensions"; import AddUser from "../users/containers/AddUser"; @@ -38,97 +38,99 @@ class Main extends React.Component { url = redirectUrlFactory(this.props); } return ( -
- - - - - - - - - - - - + +
+ + + + + + + + + + + + - - - - - - + + + + + + - - -
+ + +
+ ); } } From d2c99b9b8bd7ef37ae6a154f3c744945f5b2f56f Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 14 Mar 2019 10:08:34 +0100 Subject: [PATCH 3/6] adds proper flow annotation and an option to specify a fallback component --- .../ui-components/src/ErrorBoundary.js | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/ErrorBoundary.js b/scm-ui-components/packages/ui-components/src/ErrorBoundary.js index f955f03c49..3dd7e7bbd2 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorBoundary.js +++ b/scm-ui-components/packages/ui-components/src/ErrorBoundary.js @@ -1,26 +1,48 @@ // @flow -import React from "react"; +import * as React from "react"; import ErrorNotification from "./ErrorNotification"; -class ErrorBoundary extends React.Component { - constructor(props) { +type Props = { + fallback?: React.ComponentType, + children: React.Node +}; + +type ErrorInfo = { + componentStack: string +}; + +type State = { + error?: Error, + errorInfo?: ErrorInfo +}; + +class ErrorBoundary extends React.Component { + constructor(props: Props) { super(props); - this.state = { error: null, errorInfo: null }; + this.state = {}; } - componentDidCatch(error, errorInfo) { + componentDidCatch(error: Error, errorInfo: ErrorInfo) { // Catch errors in any components below and re-render with error message this.setState({ - error: error, - errorInfo: errorInfo + error, + errorInfo }); } + renderError = () => { + let FallbackComponent = this.props.fallback; + if (!FallbackComponent) { + FallbackComponent = ErrorNotification; + } + + return ; + }; + render() { - if (this.state.errorInfo) { - return ( - - ); + const { error } = this.state; + if (error) { + return this.renderError(); } return this.props.children; } From cce3ddc394207877c7a7581d1c6d4341f5b66c51 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 14 Mar 2019 10:12:34 +0100 Subject: [PATCH 4/6] improve location of ErrorBoundaries --- .../packages/ui-components/src/layout/Page.js | 25 +-- scm-ui/src/containers/Index.js | 29 ++- scm-ui/src/containers/IndexErrorPage.js | 26 +++ scm-ui/src/containers/Main.js | 182 +++++++++--------- 4 files changed, 142 insertions(+), 120 deletions(-) create mode 100644 scm-ui/src/containers/IndexErrorPage.js diff --git a/scm-ui-components/packages/ui-components/src/layout/Page.js b/scm-ui-components/packages/ui-components/src/layout/Page.js index 2cab7a1b0a..655b1fe986 100644 --- a/scm-ui-components/packages/ui-components/src/layout/Page.js +++ b/scm-ui-components/packages/ui-components/src/layout/Page.js @@ -32,15 +32,16 @@ class Page extends React.Component { render() { const { error } = this.props; return ( - -
-
- {this.renderPageHeader()} - +
+
+ {this.renderPageHeader()} + + {this.renderContent()} -
-
- + +
+
+ ); } @@ -67,15 +68,15 @@ class Page extends React.Component { } }); let underline = pageActionsExists ? ( -
+
) : null; return ( <>
- - <Subtitle subtitle={subtitle} /> + <Title title={title}/> + <Subtitle subtitle={subtitle}/> </div> {pageActions} </div> @@ -91,7 +92,7 @@ class Page extends React.Component<Props> { return null; } if (loading) { - return <Loading />; + return <Loading/>; } let content = []; diff --git a/scm-ui/src/containers/Index.js b/scm-ui/src/containers/Index.js index 1878195ece..af8c2a9e7f 100644 --- a/scm-ui/src/containers/Index.js +++ b/scm-ui/src/containers/Index.js @@ -5,7 +5,7 @@ import { connect } from "react-redux"; import { translate } from "react-i18next"; import { withRouter } from "react-router-dom"; -import { Loading, ErrorPage } from "@scm-manager/ui-components"; +import { Loading, ErrorBoundary } from "@scm-manager/ui-components"; import { fetchIndexResources, getFetchIndexResourcesFailure, @@ -15,6 +15,7 @@ import { import PluginLoader from "./PluginLoader"; import type { IndexResources } from "@scm-manager/ui-types"; import ScrollToTop from "./ScrollToTop"; +import IndexErrorPage from "./IndexErrorPage"; type Props = { error: Error, @@ -55,25 +56,21 @@ class Index extends Component<Props, State> { const { pluginsLoaded } = this.state; if (error) { - return ( - <ErrorPage - title={t("app.error.title")} - subtitle={t("app.error.subtitle")} - error={error} - /> - ); + return <IndexErrorPage error={error}/>; } else if (loading || !indexResources) { return <Loading />; } else { return ( - <ScrollToTop> - <PluginLoader - loaded={pluginsLoaded} - callback={this.pluginLoaderCallback} - > - <App /> - </PluginLoader> - </ScrollToTop> + <ErrorBoundary fallback={IndexErrorPage}> + <ScrollToTop> + <PluginLoader + loaded={pluginsLoaded} + callback={this.pluginLoaderCallback} + > + <App /> + </PluginLoader> + </ScrollToTop> + </ErrorBoundary> ); } } diff --git a/scm-ui/src/containers/IndexErrorPage.js b/scm-ui/src/containers/IndexErrorPage.js new file mode 100644 index 0000000000..389d17c754 --- /dev/null +++ b/scm-ui/src/containers/IndexErrorPage.js @@ -0,0 +1,26 @@ +//@flow +import React from "react"; +import { translate, type TFunction } from "react-i18next"; +import { ErrorPage } from "@scm-manager/ui-components"; + +type Props = { + error: Error, + t: TFunction +} + +class IndexErrorPage extends React.Component<Props> { + + render() { + const { error, t } = this.props; + return ( + <ErrorPage + title={t("app.error.title")} + subtitle={t("app.error.subtitle")} + error={error} + /> + ); + } + +} + +export default translate("commons")(IndexErrorPage); diff --git a/scm-ui/src/containers/Main.js b/scm-ui/src/containers/Main.js index 9036f769f2..570455d18d 100644 --- a/scm-ui/src/containers/Main.js +++ b/scm-ui/src/containers/Main.js @@ -9,7 +9,7 @@ import Users from "../users/containers/Users"; import Login from "../containers/Login"; import Logout from "../containers/Logout"; -import {ErrorBoundary, ProtectedRoute} from "@scm-manager/ui-components"; +import {ProtectedRoute} from "@scm-manager/ui-components"; import {binder, ExtensionPoint } from "@scm-manager/ui-extensions"; import AddUser from "../users/containers/AddUser"; @@ -38,99 +38,97 @@ class Main extends React.Component<Props> { url = redirectUrlFactory(this.props); } return ( - <ErrorBoundary> - <div className="main"> - <Switch> - <Redirect exact from="/" to={url}/> - <Route exact path="/login" component={Login} /> - <Route path="/logout" component={Logout} /> - <ProtectedRoute - exact - path="/repos" - component={Overview} - authenticated={authenticated} - /> - <ProtectedRoute - exact - path="/repos/create" - component={Create} - authenticated={authenticated} - /> - <ProtectedRoute - exact - path="/repos/:page" - component={Overview} - authenticated={authenticated} - /> - <ProtectedRoute - path="/repo/:namespace/:name" - component={RepositoryRoot} - authenticated={authenticated} - /> - <ProtectedRoute - exact - path="/users" - component={Users} - authenticated={authenticated} - /> - <ProtectedRoute - authenticated={authenticated} - path="/users/add" - component={AddUser} - /> - <ProtectedRoute - exact - path="/users/:page" - component={Users} - authenticated={authenticated} - /> - <ProtectedRoute - authenticated={authenticated} - path="/user/:name" - component={SingleUser} - /> + <div className="main"> + <Switch> + <Redirect exact from="/" to={url}/> + <Route exact path="/login" component={Login} /> + <Route path="/logout" component={Logout} /> + <ProtectedRoute + exact + path="/repos" + component={Overview} + authenticated={authenticated} + /> + <ProtectedRoute + exact + path="/repos/create" + component={Create} + authenticated={authenticated} + /> + <ProtectedRoute + exact + path="/repos/:page" + component={Overview} + authenticated={authenticated} + /> + <ProtectedRoute + path="/repo/:namespace/:name" + component={RepositoryRoot} + authenticated={authenticated} + /> + <ProtectedRoute + exact + path="/users" + component={Users} + authenticated={authenticated} + /> + <ProtectedRoute + authenticated={authenticated} + path="/users/add" + component={AddUser} + /> + <ProtectedRoute + exact + path="/users/:page" + component={Users} + authenticated={authenticated} + /> + <ProtectedRoute + authenticated={authenticated} + path="/user/:name" + component={SingleUser} + /> - <ProtectedRoute - exact - path="/groups" - component={Groups} - authenticated={authenticated} - /> - <ProtectedRoute - authenticated={authenticated} - path="/group/:name" - component={SingleGroup} - /> - <ProtectedRoute - authenticated={authenticated} - path="/groups/add" - component={AddGroup} - /> - <ProtectedRoute - exact - path="/groups/:page" - component={Groups} - authenticated={authenticated} - /> - <ProtectedRoute - path="/config" - component={Config} - authenticated={authenticated} - /> - <ProtectedRoute - path="/me" - component={Profile} - authenticated={authenticated} - /> + <ProtectedRoute + exact + path="/groups" + component={Groups} + authenticated={authenticated} + /> + <ProtectedRoute + authenticated={authenticated} + path="/group/:name" + component={SingleGroup} + /> + <ProtectedRoute + authenticated={authenticated} + path="/groups/add" + component={AddGroup} + /> + <ProtectedRoute + exact + path="/groups/:page" + component={Groups} + authenticated={authenticated} + /> + <ProtectedRoute + path="/config" + component={Config} + authenticated={authenticated} + /> + <ProtectedRoute + path="/me" + component={Profile} + authenticated={authenticated} + /> - <ExtensionPoint - name="main.route" - renderAll={true} - props={{authenticated, links}} - /> - </Switch> - </div> - </ErrorBoundary> + <ExtensionPoint + name="main.route" + renderAll={true} + props={{authenticated, links}} + /> + </Switch> + </div> ); } } From ba3f3acc7b5e99d1f0f0c4f2ce7fdf8d0046b169 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com> Date: Thu, 14 Mar 2019 10:15:15 +0100 Subject: [PATCH 5/6] fix wrong line endings --- .../src/repos/components/RepositoryDetails.js | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/scm-ui/src/repos/components/RepositoryDetails.js b/scm-ui/src/repos/components/RepositoryDetails.js index 02d7f2f5ac..15216eb7e9 100644 --- a/scm-ui/src/repos/components/RepositoryDetails.js +++ b/scm-ui/src/repos/components/RepositoryDetails.js @@ -1,30 +1,30 @@ -//@flow -import React from "react"; -import type { Repository } from "@scm-manager/ui-types"; -import RepositoryDetailTable from "./RepositoryDetailTable"; -import { ExtensionPoint } from "@scm-manager/ui-extensions"; - -type Props = { - repository: Repository -}; - -class RepositoryDetails extends React.Component<Props> { - render() { - const { repository } = this.props; - return ( - <div> - <RepositoryDetailTable repository={repository} /> - <hr /> - <div className="content"> - <ExtensionPoint - name="repos.repository-details.information" - renderAll={true} - props={{ repository }} - /> - </div> - </div> - ); - } -} - -export default RepositoryDetails; +//@flow +import React from "react"; +import type { Repository } from "@scm-manager/ui-types"; +import RepositoryDetailTable from "./RepositoryDetailTable"; +import { ExtensionPoint } from "@scm-manager/ui-extensions"; + +type Props = { + repository: Repository +}; + +class RepositoryDetails extends React.Component<Props> { + render() { + const { repository } = this.props; + return ( + <div> + <RepositoryDetailTable repository={repository}/> + <hr/> + <div className="content"> + <ExtensionPoint + name="repos.repository-details.information" + renderAll={true} + props={{ repository }} + /> + </div> + </div> + ); + } +} + +export default RepositoryDetails; From 867e73b4993cb753f708de620e3453d5a114e686 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com> Date: Thu, 14 Mar 2019 09:58:27 +0000 Subject: [PATCH 6/6] Close branch feature/error_boundary