From 796044782f45c40f842f305fd10924e8daadf4a5 Mon Sep 17 00:00:00 2001 From: Boris Sekachev <40690378+bsekachev@users.noreply.github.com> Date: Mon, 23 Mar 2020 13:15:36 +0300 Subject: [PATCH] React UI: Better exception handling (#1297) --- cvat-core/package.json | 1 + cvat-core/src/enums.js | 2 + cvat-core/src/log.js | 43 ++++ cvat-core/src/logger-storage.js | 12 +- cvat-ui/package-lock.json | 19 +- cvat-ui/package.json | 1 + cvat-ui/src/actions/annotation-actions.ts | 6 +- cvat-ui/src/actions/boundaries-actions.ts | 88 +++++++ .../attribute-annotation-sidebar.tsx | 4 +- cvat-ui/src/components/cvat-app.tsx | 79 ++++--- .../global-error-boundary.tsx | 223 ++++++++++++++++++ .../global-error-boundary/styles.scss | 17 ++ cvat-ui/src/index.tsx | 50 +++- cvat-ui/src/reducers/about-reducer.ts | 6 +- cvat-ui/src/reducers/annotation-reducer.ts | 20 +- cvat-ui/src/reducers/auth-reducer.ts | 6 +- cvat-ui/src/reducers/formats-reducer.ts | 8 +- cvat-ui/src/reducers/interfaces.ts | 3 + cvat-ui/src/reducers/models-reducer.ts | 11 +- cvat-ui/src/reducers/notifications-reducer.ts | 24 +- cvat-ui/src/reducers/settings-reducer.ts | 7 + cvat-ui/src/reducers/share-reducer.ts | 8 +- cvat-ui/src/reducers/shortcuts-reducer.ts | 11 +- cvat-ui/src/reducers/tasks-reducer.ts | 6 +- cvat-ui/src/reducers/users-reducer.ts | 8 +- cvat-ui/src/utils/redux.ts | 5 +- 26 files changed, 578 insertions(+), 90 deletions(-) create mode 100644 cvat-ui/src/actions/boundaries-actions.ts create mode 100644 cvat-ui/src/components/global-error-boundary/global-error-boundary.tsx create mode 100644 cvat-ui/src/components/global-error-boundary/styles.scss diff --git a/cvat-core/package.json b/cvat-core/package.json index d07e81aa..8859adc5 100644 --- a/cvat-core/package.json +++ b/cvat-core/package.json @@ -34,6 +34,7 @@ "dependencies": { "axios": "^0.18.0", "browser-or-node": "^1.2.1", + "detect-browser": "^5.0.0", "error-stack-parser": "^2.0.2", "form-data": "^2.5.0", "jest-config": "^24.8.0", diff --git a/cvat-core/src/enums.js b/cvat-core/src/enums.js index 8b6c86fc..feef4825 100644 --- a/cvat-core/src/enums.js +++ b/cvat-core/src/enums.js @@ -109,6 +109,7 @@ * @memberof module:API.cvat.enums * @property {string} loadJob Load job * @property {string} saveJob Save job + * @property {string} restoreJob Restore job * @property {string} uploadAnnotations Upload annotations * @property {string} sendUserActivity Send user activity * @property {string} sendException Send exception @@ -142,6 +143,7 @@ const LogType = Object.freeze({ loadJob: 'Load job', saveJob: 'Save job', + restoreJob: 'Restore job', uploadAnnotations: 'Upload annotations', sendUserActivity: 'Send user activity', sendException: 'Send exception', diff --git a/cvat-core/src/log.js b/cvat-core/src/log.js index 56d9592d..68cce5c5 100644 --- a/cvat-core/src/log.js +++ b/cvat-core/src/log.js @@ -6,6 +6,7 @@ require:false */ +const { detect } = require('detect-browser'); const PluginRegistry = require('./plugins'); const { ArgumentError } = require('./exceptions'); const { LogType } = require('./enums'); @@ -179,6 +180,48 @@ class LogWithExceptionInfo extends Log { + 'It must be a number'; throw new ArgumentError(message); } + + if (typeof (this.payload.column) !== 'number') { + const message = `The field "column" is required for ${this.type} log. ` + + 'It must be a number'; + throw new ArgumentError(message); + } + + if (typeof (this.payload.stack) !== 'string') { + const message = `The field "stack" is required for ${this.type} log. ` + + 'It must be a string'; + throw new ArgumentError(message); + } + } + + dump() { + const payload = { ...this.payload }; + const client = detect(); + const body = { + client_id: payload.client_id, + name: this.type, + time: this.time.toISOString(), + message: payload.message, + filename: payload.filename, + line: payload.line, + column: payload.column, + stack: payload.stack, + system: client.os, + client: client.name, + version: client.version, + }; + + delete payload.client_id; + delete payload.message; + delete payload.filename; + delete payload.line; + delete payload.column; + delete payload.stack; + + return { + ...body, + payload, + }; } } diff --git a/cvat-core/src/logger-storage.js b/cvat-core/src/logger-storage.js index e8e24d7d..ca6860af 100644 --- a/cvat-core/src/logger-storage.js +++ b/cvat-core/src/logger-storage.js @@ -7,7 +7,7 @@ */ const PluginRegistry = require('./plugins'); -const server = require('./server-proxy'); +const serverProxy = require('./server-proxy'); const logFactory = require('./log'); const { ArgumentError } = require('./exceptions'); const { LogType } = require('./enums'); @@ -128,6 +128,14 @@ LoggerStorage.prototype.log.implementation = function (logType, payload, wait) { this.collection.push(log); }; + if (log.type === LogType.sendException) { + serverProxy.server.exception(log.dump()).catch(() => { + pushEvent(); + }); + + return log; + } + if (wait) { log.onClose(pushEvent); } else { @@ -156,7 +164,7 @@ LoggerStorage.prototype.save.implementation = async function () { const userActivityLog = logFactory(LogType.sendUserActivity, logPayload); collectionToSend.push(userActivityLog); - await server.logs.save(collectionToSend.map((log) => log.dump())); + await serverProxy.logs.save(collectionToSend.map((log) => log.dump())); for (const rule of Object.values(this.ignoreRules)) { rule.lastLog = null; diff --git a/cvat-ui/package-lock.json b/cvat-ui/package-lock.json index f2453993..3fea00e5 100644 --- a/cvat-ui/package-lock.json +++ b/cvat-ui/package-lock.json @@ -3815,6 +3815,14 @@ "is-arrayish": "^0.2.1" } }, + "error-stack-parser": { + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/error-stack-parser/-/error-stack-parser-2.0.6.tgz", + "integrity": "sha512-d51brTeqC+BHlwF0BhPtcYgF5nlzf9ZZ0ZIUQNZpc9ZB9qw5IJ2diTrBY9jlCJkTLITYPjmiX6OWCwH+fuyNgQ==", + "requires": { + "stackframe": "^1.1.1" + } + }, "es-abstract": { "version": "1.16.0", "resolved": "https://registry.npmjs.org/es-abstract/-/es-abstract-1.16.0.tgz", @@ -11203,6 +11211,11 @@ "integrity": "sha512-ji9qxRnOVfcuLDySj9qzhGSEFVobyt1kIOSkj1qZzYLzq7Tos/oUUWvotUPQLlrsidqsK6tBH89Bc9kL5zHA6w==", "dev": true }, + "stackframe": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/stackframe/-/stackframe-1.1.1.tgz", + "integrity": "sha512-0PlYhdKh6AfFxRyK/v+6/k+/mMfyiEBbTM5L94D0ZytQnJ166wuwoTYLHFWGbs2dpA8Rgq763KGWmN1EQEYHRQ==" + }, "static-extend": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/static-extend/-/static-extend-0.1.2.tgz", @@ -11981,9 +11994,9 @@ "dev": true }, "ua-parser-js": { - "version": "0.7.20", - "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-0.7.20.tgz", - "integrity": "sha512-8OaIKfzL5cpx8eCMAhhvTlft8GYF8b2eQr6JkCyVdrgjcytyOmPCXrqXFcUnhonRpLlh5yxEZVohm6mzaowUOw==" + "version": "0.7.21", + "resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-0.7.21.tgz", + "integrity": "sha512-+O8/qh/Qj8CgC6eYBVBykMrNtp5Gebn4dlGD/kKXVkJNDwyrAwSIqwz8CDf+tsAIWVycKcku6gIXJ0qwx/ZXaQ==" }, "uglify-js": { "version": "3.4.10", diff --git a/cvat-ui/package.json b/cvat-ui/package.json index 60f6aa30..bdf21526 100644 --- a/cvat-ui/package.json +++ b/cvat-ui/package.json @@ -57,6 +57,7 @@ "antd": "^3.25.2", "copy-to-clipboard": "^3.2.0", "dotenv-webpack": "^1.7.0", + "error-stack-parser": "^2.0.6", "moment": "^2.24.0", "prop-types": "^15.7.2", "react": "^16.9.0", diff --git a/cvat-ui/src/actions/annotation-actions.ts b/cvat-ui/src/actions/annotation-actions.ts index b80c0a0f..678323c6 100644 --- a/cvat-ui/src/actions/annotation-actions.ts +++ b/cvat-ui/src/actions/annotation-actions.ts @@ -78,10 +78,14 @@ function receiveAnnotationsParameters(): AnnotationsParameters { }; } -function computeZRange(states: any[]): number[] { +export function computeZRange(states: any[]): number[] { let minZ = states.length ? states[0].zOrder : 0; let maxZ = states.length ? states[0].zOrder : 0; states.forEach((state: any): void => { + if (state.objectType === ObjectType.TAG) { + return; + } + minZ = Math.min(minZ, state.zOrder); maxZ = Math.max(maxZ, state.zOrder); }); diff --git a/cvat-ui/src/actions/boundaries-actions.ts b/cvat-ui/src/actions/boundaries-actions.ts new file mode 100644 index 00000000..fa2faba0 --- /dev/null +++ b/cvat-ui/src/actions/boundaries-actions.ts @@ -0,0 +1,88 @@ +// Copyright (C) 2020 Intel Corporation +// +// SPDX-License-Identifier: MIT + +import { + ActionUnion, + createAction, + ThunkAction, + ThunkDispatch, +} from 'utils/redux'; +import getCore from 'cvat-core'; +import { LogType } from 'cvat-logger'; +import { computeZRange } from './annotation-actions'; + +const cvat = getCore(); + +export enum BoundariesActionTypes { + RESET_AFTER_ERROR = 'RESET_AFTER_ERROR', + THROW_RESET_ERROR = 'THROW_RESET_ERROR', +} + +export const boundariesActions = { + resetAfterError: ( + job: any, + states: any[], + frameNumber: number, + frameData: any | null, + minZ: number, + maxZ: number, + colors: string[], + ) => createAction(BoundariesActionTypes.RESET_AFTER_ERROR, { + job, + states, + frameNumber, + frameData, + minZ, + maxZ, + colors, + }), + throwResetError: () => createAction(BoundariesActionTypes.THROW_RESET_ERROR), +}; + +export function resetAfterErrorAsync(): ThunkAction { + return async (dispatch: ThunkDispatch, getState): Promise => { + try { + const state = getState(); + const job = state.annotation.job.instance; + + if (job) { + const currentFrame = state.annotation.player.frame.number; + const { showAllInterpolationTracks } = state.settings.workspace; + const frameNumber = Math.max(Math.min(job.stopFrame, currentFrame), job.startFrame); + + const states = await job.annotations + .get(frameNumber, showAllInterpolationTracks, []); + const frameData = await job.frames.get(frameNumber); + const [minZ, maxZ] = computeZRange(states); + const colors = [...cvat.enums.colors]; + + await job.logger.log(LogType.restoreJob); + + dispatch(boundariesActions.resetAfterError( + job, + states, + frameNumber, + frameData, + minZ, + maxZ, + colors, + )); + } else { + dispatch(boundariesActions.resetAfterError( + null, + [], + 0, + null, + 0, + 0, + [], + )); + } + } catch (error) { + dispatch(boundariesActions.throwResetError()); + } + }; +} + +export type boundariesActions = ActionUnion; diff --git a/cvat-ui/src/components/annotation-page/attribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx b/cvat-ui/src/components/annotation-page/attribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx index 49d3b499..74cd0bec 100644 --- a/cvat-ui/src/components/annotation-page/attribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx +++ b/cvat-ui/src/components/annotation-page/attribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx @@ -5,6 +5,8 @@ import React, { useState, useEffect } from 'react'; import { GlobalHotKeys, KeyMap } from 'react-hotkeys'; import { connect } from 'react-redux'; +import { Action } from 'redux'; +import { ThunkDispatch } from 'redux-thunk'; import Layout, { SiderProps } from 'antd/lib/layout'; import { SelectValue } from 'antd/lib/select'; import { CheckboxChangeEvent } from 'antd/lib/checkbox'; @@ -65,7 +67,7 @@ function mapStateToProps(state: CombinedState): StateToProps { }; } -function mapDispatchToProps(dispatch: any): DispatchToProps { +function mapDispatchToProps(dispatch: ThunkDispatch): DispatchToProps { return { activateObject(clientID: number, attrID: number): void { dispatch(activateObjectAction(clientID, attrID)); diff --git a/cvat-ui/src/components/cvat-app.tsx b/cvat-ui/src/components/cvat-app.tsx index bab5887a..c168a028 100644 --- a/cvat-ui/src/components/cvat-app.tsx +++ b/cvat-ui/src/components/cvat-app.tsx @@ -8,13 +8,11 @@ import React from 'react'; import { Switch, Route, Redirect } from 'react-router'; import { withRouter, RouteComponentProps } from 'react-router-dom'; import { GlobalHotKeys, KeyMap, configure } from 'react-hotkeys'; +import Spin from 'antd/lib/spin'; +import Layout from 'antd/lib/layout'; +import notification from 'antd/lib/notification'; -import { - Spin, - Layout, - notification, -} from 'antd'; - +import GlobalErrorBoundary from 'components/global-error-boundary/global-error-boundary'; import ShorcutsDialog from 'components/shortcuts-dialog/shortcuts-dialog'; import SettingsPageContainer from 'containers/settings-page/settings-page'; import TasksPageContainer from 'containers/tasks-page/tasks-page'; @@ -40,6 +38,7 @@ interface CVATAppProps { resetMessages: () => void; switchShortcutsDialog: () => void; userInitialized: boolean; + userFetching: boolean; pluginsInitialized: boolean; pluginsFetching: boolean; formatsInitialized: boolean; @@ -73,11 +72,13 @@ class CVATApplication extends React.PureComponent - - - - - - - - - - - {withModels - && } - {installedAutoAnnotation - && } - - - - {/* eslint-disable-next-line */} - - - + + + + + + + + + + + + + {withModels + && } + {installedAutoAnnotation + && } + + + + {/* eslint-disable-next-line */} + + + + ); } return ( - - - - - + + + + + + + ); } diff --git a/cvat-ui/src/components/global-error-boundary/global-error-boundary.tsx b/cvat-ui/src/components/global-error-boundary/global-error-boundary.tsx new file mode 100644 index 00000000..3c132fd9 --- /dev/null +++ b/cvat-ui/src/components/global-error-boundary/global-error-boundary.tsx @@ -0,0 +1,223 @@ +// Copyright (C) 2020 Intel Corporation +// +// SPDX-License-Identifier: MIT + +import './styles.scss'; +import React from 'react'; +import { connect } from 'react-redux'; +import { Action } from 'redux'; +import { ThunkDispatch } from 'redux-thunk'; +import Result from 'antd/lib/result'; +import Text from 'antd/lib/typography/Text'; +import Paragraph from 'antd/lib/typography/Paragraph'; +import Collapse from 'antd/lib/collapse'; +import TextArea from 'antd/lib/input/TextArea'; +import Tooltip from 'antd/lib/tooltip'; +import copy from 'copy-to-clipboard'; +import ErrorStackParser from 'error-stack-parser'; + +import { resetAfterErrorAsync } from 'actions/boundaries-actions'; +import { CombinedState } from 'reducers/interfaces'; +import logger, { LogType } from 'cvat-logger'; + +interface StateToProps { + job: any | null; + serverVersion: string; + coreVersion: string; + canvasVersion: string; + uiVersion: string; +} + +interface DispatchToProps { + restore(): void; +} + +interface State { + hasError: boolean; + error: Error | null; +} + +function mapStateToProps(state: CombinedState): StateToProps { + const { + annotation: { + job: { + instance: job, + }, + }, + about: { + server, + packageVersion, + }, + } = state; + + return { + job, + serverVersion: server.version as string, + coreVersion: packageVersion.core, + canvasVersion: packageVersion.canvas, + uiVersion: packageVersion.ui, + }; +} + +function mapDispatchToProps(dispatch: ThunkDispatch): DispatchToProps { + return { + restore(): void { + dispatch(resetAfterErrorAsync()); + }, + }; +} + + +type Props = StateToProps & DispatchToProps; +class GlobalErrorBoundary extends React.PureComponent { + public constructor(props: Props) { + super(props); + this.state = { + hasError: false, + error: null, + }; + } + + static getDerivedStateFromError(error: Error): State { + return { + hasError: true, + error, + }; + } + + public componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { + const { job } = this.props; + const parsed = ErrorStackParser.parse(error); + + const logPayload = { + filename: parsed[0].fileName, + line: parsed[0].lineNumber, + message: error.message, + column: parsed[0].columnNumber, + stack: error.stack, + componentStack: errorInfo.componentStack, + }; + + if (job) { + job.logger.log(LogType.sendException, logPayload); + } else { + logger.log(LogType.sendException, logPayload); + } + } + + public render(): React.ReactNode { + const { + restore, + job, + serverVersion, + coreVersion, + canvasVersion, + uiVersion, + } = this.props; + + const { hasError, error } = this.state; + + const restoreGlobalState = (): void => { + this.setState({ + error: null, + hasError: false, + }); + + restore(); + }; + + if (hasError && error) { + const message = `${error.name}\n${error.message}\n\n${error.stack}`; + return ( +
+ +
+ + What has happened? + Program error has just occured + + + +