Handle errors loading annoation data more transparently with event bu…

…s and alerts. Add error boundary for js errors
cBioPortal · Dec 20, 2022 · 89c7c1a · 89c7c1a
1 parent 53d0bb9
commit 89c7c1a
Show file tree

Hide file tree

Showing 25 changed files with 614 additions and 112 deletions.
diff --git a/package.json b/package.json
@@ -194,6 +194,7 @@
     "jquery": "3.6.0",
     "jquery-migrate": "3.0.0",
     "js-combinatorics": "^0.5.2",
+    "js-event-bus": "^1.1.1",
     "json-fn": "^1.1.1",
     "jsonpath": "^1.1.1",
     "jspdf": "^1.3.3",

diff --git a/packages/react-mutation-mapper/src/store/DefaultMutationMapperStore.ts b/packages/react-mutation-mapper/src/store/DefaultMutationMapperStore.ts
@@ -536,6 +536,9 @@ class DefaultMutationMapperStore<T extends Mutation>
                     return undefined;
                 }
             },
+            onError: () => {
+                // allow client level handler to work
+            },
         },
         undefined
     );
@@ -975,7 +978,7 @@ class DefaultMutationMapperStore<T extends Mutation>
                             map: { [entrezGeneId: number]: boolean },
                             next: CancerGene
                         ) => {
-                            if (next.oncokbAnnotated) {
+                            if (next && next.oncokbAnnotated) {
                                 map[next.entrezGeneId] = true;
                             }
                             return map;

diff --git a/src/AppStore.ts b/src/AppStore.ts
@@ -10,11 +10,13 @@ import client from 'shared/api/cbioportalClientInstance';
 import { sendSentryMessage } from './shared/lib/tracking';
 import { FeatureFlagStore } from 'shared/FeatureFlagStore';
 
-export type SiteError = {
-    errorObj: any;
-    dismissed: boolean;
-    title?: string;
-};
+export class SiteError {
+    constructor(
+        public errorObj: any,
+        public displayType: 'alert' | 'site' = 'site',
+        public title?: string
+    ) {}
+}
 
 export class AppStore {
     constructor(
@@ -27,9 +29,9 @@ export class AppStore {
                 sendSentryMessage('ERRORHANDLER:' + error);
             } catch (ex) {}
 
-            if (error.status && /400|500|403/.test(error.status)) {
+            if (error.status && /400|500|5\d\d|403/.test(error.status)) {
                 sendSentryMessage('ERROR DIALOG SHOWN:' + error);
-                this.siteErrors.push({ errorObj: error, dismissed: false });
+                this.siteErrors.push(new SiteError(new Error(error)));
             }
         });
     }
@@ -44,7 +46,9 @@ export class AppStore {
 
     @observable private _appReady = false;
 
-    @observable siteErrors: SiteError[] = [];
+    siteErrors = observable.array<SiteError>();
+
+    alertErrors = observable.array<SiteError>();
 
     @observable.ref userName: string | undefined = undefined;
 
@@ -76,30 +80,29 @@ export class AppStore {
         }
     }
 
-    @computed get undismissedSiteErrors() {
-        return _.filter(this.siteErrors.slice(), err => !err.dismissed);
-    }
-
     @computed get isErrorCondition() {
-        return this.undismissedSiteErrors.length > 0;
+        return this.siteErrors.length > 0;
     }
 
     @action
     public dismissErrors() {
-        this.siteErrors = this.siteErrors.map(err => {
-            err.dismissed = true;
-            return err;
-        });
+        this.siteErrors.clear();
     }
 
-    @action public addError(err: String | SiteError) {
-        if (_.isString(err)) {
-            this.siteErrors.push({
-                errorObj: { message: err },
-                dismissed: false,
-            });
+    @action
+    public dismissError(err: SiteError) {
+        this.siteErrors.remove(err);
+    }
+
+    @action public addError(err: SiteError | string) {
+        if (typeof err === 'string') {
+            this.siteErrors.push(new SiteError(new Error(err)));
         } else {
-            this.siteErrors.push({ errorObj: err, dismissed: false });
+            if (err.displayType === 'alert') {
+                this.alertErrors.push(err);
+            } else {
+                this.siteErrors.push(err);
+            }
         }
     }
 

diff --git a/src/appBootstrapper.tsx b/src/appBootstrapper.tsx
@@ -22,17 +22,18 @@ import * as superagent from 'superagent';
 import { buildCBioPortalPageUrl } from './shared/api/urls';
 import browser from 'bowser';
 import { setNetworkListener } from './shared/lib/ajaxQuiet';
-import { initializeTracking } from 'shared/lib/tracking';
+import { initializeTracking, sendToLoggly } from 'shared/lib/tracking';
 import superagentCache from 'superagent-cache';
 import { getBrowserWindow } from 'cbioportal-frontend-commons';
-import { AppStore } from './AppStore';
+import { AppStore, SiteError } from './AppStore';
 import { handleLongUrls } from 'shared/lib/handleLongUrls';
 import 'shared/polyfill/canvasToBlob';
 import { setCurrentURLHeader } from 'shared/lib/extraHeader';
 import Container from 'appShell/App/Container';
 import { IServerConfig } from 'config/IAppConfig';
 import { initializeGenericAssayServerConfig } from 'shared/lib/GenericAssayUtils/GenericAssayConfig';
 import { FeatureFlagStore } from 'shared/FeatureFlagStore';
+import eventBus from 'shared/events/eventBus';
 
 export interface ICBioWindow {
     globalStores: {
@@ -163,6 +164,11 @@ const stores = {
 
 browserWindow.globalStores = stores;
 
+eventBus.on('error', (err: SiteError) => {
+    sendToLoggly(err?.errorObj?.message);
+    stores.appStore.addError(err);
+});
+
 //@ts-ignore
 const end = superagent.Request.prototype.end;
 

diff --git a/src/appShell/App/Container.tsx b/src/appShell/App/Container.tsx
@@ -30,6 +30,10 @@ import {
 import makeRoutes from 'routes';
 import { AppContext } from 'cbioportal-frontend-commons';
 import { IAppContext } from 'cbioportal-frontend-commons';
+import { ErrorAlert } from 'shared/components/errorScreen/ErrorAlert';
+import { ErrorInfo } from 'react';
+import { observable } from 'mobx';
+import { sendToLoggly } from 'shared/lib/tracking';
 
 interface IContainerProps {
     location: Location;
@@ -96,57 +100,110 @@ export default class Container extends React.Component<IContainerProps, {}> {
 
         return (
             <AppContext.Provider value={this.appContext}>
-                <div>
-                    <Helmet>
-                        <meta charSet="utf-8" />
-                        <title>{getServerConfig().skin_title}</title>
-                        <meta
-                            name="description"
-                            content={getServerConfig().skin_description}
-                        />
-                    </Helmet>
+                <ErrorBoundary>
+                    <div>
+                        <Helmet>
+                            <meta charSet="utf-8" />
+                            <title>{getServerConfig().skin_title}</title>
+                            <meta
+                                name="description"
+                                content={getServerConfig().skin_description}
+                            />
+                        </Helmet>
 
-                    <div id="pageTopContainer" className="pageTopContainer">
-                        <UserMessager />
+                        <div id="pageTopContainer" className="pageTopContainer">
+                            <UserMessager />
 
-                        {shouldShowStudyViewWarning() && <StudyAgreement />}
+                            {shouldShowStudyViewWarning() && <StudyAgreement />}
 
-                        {shouldShowGenieWarning() && <GenieAgreement />}
+                            {shouldShowGenieWarning() && <GenieAgreement />}
 
-                        <div className="contentWidth">
-                            <PortalHeader appStore={this.appStore} />
+                            <div className="contentWidth">
+                                <PortalHeader appStore={this.appStore} />
+                            </div>
                         </div>
+                        <If condition={this.appStore.isErrorCondition}>
+                            <Then>
+                                <div className="contentWrapper">
+                                    <ErrorScreen
+                                        title={
+                                            formatErrorTitle(
+                                                this.appStore.siteErrors
+                                            ) ||
+                                            'Oops. There was an error retrieving data.'
+                                        }
+                                        body={
+                                            <a
+                                                href={buildCBioPortalPageUrl(
+                                                    '/'
+                                                )}
+                                            >
+                                                Return to homepage
+                                            </a>
+                                        }
+                                        errorLog={formatErrorLog(
+                                            this.appStore.siteErrors
+                                        )}
+                                        errorMessages={formatErrorMessages(
+                                            this.appStore.siteErrors
+                                        )}
+                                    />
+                                </div>
+                            </Then>
+                            <Else>
+                                <div className="contentWrapper">
+                                    <ErrorAlert appStore={this.appStore} />
+
+                                    {makeRoutes()}
+                                </div>
+                            </Else>
+                        </If>
                     </div>
-                    <If condition={this.appStore.isErrorCondition}>
-                        <Then>
-                            <div className="contentWrapper">
-                                <ErrorScreen
-                                    title={
-                                        formatErrorTitle(
-                                            this.appStore.undismissedSiteErrors
-                                        ) ||
-                                        'Oops. There was an error retrieving data.'
-                                    }
-                                    body={
-                                        <a href={buildCBioPortalPageUrl('/')}>
-                                            Return to homepage
-                                        </a>
-                                    }
-                                    errorLog={formatErrorLog(
-                                        this.appStore.undismissedSiteErrors
-                                    )}
-                                    errorMessages={formatErrorMessages(
-                                        this.appStore.undismissedSiteErrors
-                                    )}
-                                />
-                            </div>
-                        </Then>
-                        <Else>
-                            <div className="contentWrapper">{makeRoutes()}</div>
-                        </Else>
-                    </If>
-                </div>
+                </ErrorBoundary>
             </AppContext.Provider>
         );
     }
 }
+
+class ErrorBoundary extends React.Component<
+    any,
+    { hasError: boolean; error?: Error }
+> {
+    @observable hasError = false;
+
+    constructor(props: any) {
+        super(props);
+        this.state = {
+            hasError: false,
+        };
+    }
+
+    static getDerivedStateFromError(error: any) {
+        // Update state so the next render will show the fallback UI.
+        return { hasError: true, error };
+    }
+
+    componentDidCatch(error: Error, errorInfo: ErrorInfo) {
+        // You can also log the error to an error reporting service
+
+        sendToLoggly(error.message, 'ERROR_JS');
+        this.hasError = true;
+    }
+
+    render() {
+        if (this.state.hasError) {
+            // fallback UI
+            return (
+                <ErrorScreen
+                    title={'Oh no! We encountered an error.'}
+                    errorLog={JSON.stringify({
+                        type: 'ErrorBoundary',
+                        log: this.state.error?.toString(),
+                    })}
+                />
+            );
+        } else {
+            return this.props.children;
+        }
+    }
+}
diff --git a/src/pages/groupComparison/GroupComparisonStore.ts b/src/pages/groupComparison/GroupComparisonStore.ts
@@ -10,7 +10,6 @@ import {
     SampleFilter,
     CancerStudy,
     MutationMultipleStudyFilter,
-    SampleMolecularIdentifier,
     GenePanelDataMultipleStudyFilter,
     Mutation,
     Gene,

diff --git a/src/pages/patientView/clinicalInformation/PatientViewPageStore.ts b/src/pages/patientView/clinicalInformation/PatientViewPageStore.ts
@@ -136,7 +136,7 @@ import { groupTrialMatchesById } from '../trialMatch/TrialMatchTableUtils';
 import { GeneFilterOption } from '../mutation/GeneFilterMenu';
 import TumorColumnFormatter from '../mutation/column/TumorColumnFormatter';
 import { getVariantAlleleFrequency } from 'shared/lib/MutationUtils';
-import { AppStore, SiteError } from 'AppStore';
+import { AppStore, SiteError } from '../../../AppStore';
 import { getGeneFilterDefault } from './PatientViewPageStoreUtil';
 import { checkNonProfiledGenesExist } from '../PatientViewPageUtils';
 import autobind from 'autobind-decorator';
@@ -761,11 +761,7 @@ export class PatientViewPageStore {
                     this.sampleId
                 ),
             onError: (err: Error) => {
-                this.appStore.siteErrors.push({
-                    errorObj: err,
-                    dismissed: false,
-                    title: 'Samples / Patients not valid',
-                } as SiteError);
+                this.appStore.siteErrors.push(new SiteError(err));
             },
         },
         []
@@ -1684,6 +1680,7 @@ export class PatientViewPageStore {
                     return Promise.resolve([]);
                 }
             },
+            onError: () => {},
         },
         []
     );
@@ -1719,7 +1716,7 @@ export class PatientViewPageStore {
                                 map: { [entrezGeneId: number]: boolean },
                                 next: CancerGene
                             ) => {
-                                if (next.oncokbAnnotated) {
+                                if (next?.oncokbAnnotated) {
                                     map[next.entrezGeneId] = true;
                                 }
                                 return map;
@@ -1731,6 +1728,7 @@ export class PatientViewPageStore {
                     return Promise.resolve({});
                 }
             },
+            onError: () => {},
         },
         {}
     );
@@ -2453,6 +2451,7 @@ export class PatientViewPageStore {
                     this.oncoKbAnnotatedGenes,
                     this.mutationData
                 ),
+            onError: () => {},
         },
         ONCOKB_DEFAULT
     );
@@ -2714,5 +2713,6 @@ export class PatientViewPageStore {
                 .value();
         },
         default: {},
+        onError: () => {},
     });
 }
diff --git a/src/pages/patientView/copyNumberAlterations/column/AnnotationColumnFormatter.tsx b/src/pages/patientView/copyNumberAlterations/column/AnnotationColumnFormatter.tsx
@@ -53,7 +53,10 @@ export default class AnnotationColumnFormatter {
 
             let oncoKbGeneExist = false;
             let isOncoKbCancerGene = false;
-            if (oncoKbCancerGenes && !(oncoKbCancerGenes instanceof Error)) {
+            if (
+                oncoKbCancerGenes &&
+                !(oncoKbCancerGenes.result instanceof Error)
+            ) {
                 oncoKbGeneExist =
                     _.find(
                         oncoKbCancerGenes.result,