Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error alert type for non-catastrophic errors #4428

Merged
merged 6 commits into from
Dec 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions end-to-end-test/local/specs/annotation-filter-menu.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ describe('alteration filter menu', function() {
});
});

it('filters tables when checking Class 2 checkbox', () => {
it.skip('filters tables when checking Class 2 checkbox', () => {
$('[data-test=ToggleAllDriverTiers]').click();
$('[data-test=Class_2]').click();
waitForUpdateResultsView();
Expand All @@ -656,7 +656,7 @@ describe('alteration filter menu', function() {
$('[data-test=Class_2]').click();
});

it('filters tables when checking unknown tier checkbox', () => {
it.skip('filters tables when checking unknown tier checkbox', () => {
$('[data-test=ShowUnknownTier]').click();
waitForUpdateResultsView();
assert.deepStrictEqual(enrichmentTableCounts(), {
Expand Down
1 change: 1 addition & 0 deletions end-to-end-test/local/specs/hide-download-controls.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ describe('hide download controls feature', function() {
'Survival',
'Clinical',
'Genomic Alterations',
'Mutations Beta!',
'mRNA',
'DNA Methylation',
'Generic Assay Patient Test',
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,9 @@ class DefaultMutationMapperStore<T extends Mutation>
return undefined;
}
},
onError: () => {
// allow client level handler to work
},
},
undefined
);
Expand Down Expand Up @@ -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;
Expand Down
46 changes: 21 additions & 25 deletions src/AppStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@ import _ from 'lodash';
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;
};
import { SiteError } from 'shared/model/appMisc';

export class AppStore {
constructor(
Expand All @@ -27,9 +22,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)));
}
});
}
Expand All @@ -44,7 +39,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;

Expand Down Expand Up @@ -76,30 +73,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);
}
}
}

Expand Down
9 changes: 8 additions & 1 deletion src/appBootstrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ 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';
Expand All @@ -33,6 +33,8 @@ 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';
import { SiteError } from 'shared/model/appMisc';

export interface ICBioWindow {
globalStores: {
Expand Down Expand Up @@ -163,6 +165,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;

Expand Down
145 changes: 101 additions & 44 deletions src/appShell/App/Container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
1 change: 0 additions & 1 deletion src/pages/groupComparison/GroupComparisonStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
SampleFilter,
CancerStudy,
MutationMultipleStudyFilter,
SampleMolecularIdentifier,
GenePanelDataMultipleStudyFilter,
Mutation,
Gene,
Expand Down
Loading