diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index d6b97ecd6c..18b1b1b99a 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -225,6 +225,18 @@ Details--close-sidebar-button = Details--error-boundary-message = .message = Uh oh, some unknown error happened in this panel. +## ErrorBoundary +## This component is shown when an unexpected error is encountered in the application. +## Note that the localization won't be always applied in this component. + +# This message will always be displayed after another context-specific message. +ErrorBoundary--report-error-to-developers-description = + Please report this issue to the developers, including the full + error as displayed in the Developer Tools’ Web Console. + +# This is used in a call to action button, displayed inside the error box. +ErrorBoundary--report-error-on-github = Report the error on GitHub + ## Footer Links FooterLinks--legal = Legal @@ -668,6 +680,11 @@ ProfileLoaderAnimation--loading-view-not-found = View not found ProfileRootMessage--title = { -profiler-brand-name } ProfileRootMessage--additional = Back to home +## Root + +Root--error-boundary-message = + .message = Uh oh, some unknown error happened in profiler.firefox.com. + ## ServiceWorkerManager ## This is the component responsible for handling the service worker installation ## and update. It appears at the top of the UI. diff --git a/res/css/photon/button.css b/res/css/photon/button.css index 9a37ddf7d2..e335b97b9b 100644 --- a/res/css/photon/button.css +++ b/res/css/photon/button.css @@ -4,6 +4,9 @@ /* See https://design.firefox.com/photon/components/buttons.html for the spec */ .photon-button { + /* These two flex options aren't necessary when a real - -
- {errorString ?
{errorString}
: null} - {componentStack ?
{componentStack}
: null} + {this.props.buttonContent} +
@@ -99,3 +97,38 @@ export class ErrorBoundary extends React.Component { return this.props.children; } } + +/** + * Use this error boundary when outside of the AppLocalizationProvider hierarchy + */ +export function NonLocalizedErrorBoundary(props: ExternalProps) { + return ( + + ); +} + +/** + * Use this error boundary when inside of the AppLocalizationProvider hierarchy + */ +export function LocalizedErrorBoundary(props: ExternalProps) { + return ( + + Report the error on GitHub + + } + reportExplanationMessage={ + + Please report this issue to the developers, including the full error + as displayed in the Developer Tools’ Web Console. + + } + /> + ); +} diff --git a/src/components/app/Root.js b/src/components/app/Root.js index ec9c820875..0acc165abb 100644 --- a/src/components/app/Root.js +++ b/src/components/app/Root.js @@ -4,10 +4,14 @@ // @flow import React, { PureComponent } from 'react'; +import { Localized } from '@fluent/react'; import { Provider } from 'react-redux'; import { UrlManager } from './UrlManager'; import { FooterLinks } from './FooterLinks'; -import { ErrorBoundary } from './ErrorBoundary'; +import { + NonLocalizedErrorBoundary, + LocalizedErrorBoundary, +} from './ErrorBoundary'; import { AppViewRouter } from './AppViewRouter'; import { ProfileLoader } from './ProfileLoader'; import { ServiceWorkerManager } from './ServiceWorkerManager'; @@ -28,21 +32,28 @@ export class Root extends PureComponent { render() { const { store } = this.props; return ( - + - - - - - - - - - + + + + + + + + + + + + + - + ); } } diff --git a/src/test/components/ErrorBoundary.test.js b/src/test/components/ErrorBoundary.test.js index 10962907fc..4f86fde978 100644 --- a/src/test/components/ErrorBoundary.test.js +++ b/src/test/components/ErrorBoundary.test.js @@ -4,15 +4,16 @@ // @flow import * as React from 'react'; -import { screen } from '@testing-library/react'; import { stripIndent } from 'common-tags'; import { render } from 'firefox-profiler/test/fixtures/testing-library'; -import { ErrorBoundary } from '../../components/app/ErrorBoundary'; +import { + NonLocalizedErrorBoundary, + LocalizedErrorBoundary, +} from '../../components/app/ErrorBoundary'; import { withAnalyticsMock } from '../fixtures/mocks/analytics'; -import { fireFullClick } from '../fixtures/utils'; -describe('app/ErrorBoundary', function () { +describe('app/NonLocalizedErrorBoundary', function () { const childComponentText = 'This is a child component'; const friendlyErrorMessage = 'Oops, there was an error'; const technicalErrorMessage = 'This is an error.'; @@ -23,28 +24,15 @@ describe('app/ErrorBoundary', function () { function setupComponent(childComponent) { const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); const results = render( - + {childComponent} - + ); return { spy, ...results }; } it('matches the snapshot', () => { const { container } = setupComponent(); - - // We need to change the textContent of the stack, so that path information - // isn't tied to a specific environment. - const stack = screen.getByText(/at ThrowingComponent/); - stack.textContent = stack.textContent - .replace(/\\/g, '/') // normalizes Windows paths - .replace(/\(.*\/src/g, '(REDACTED)/src') // Removes the home directory - // Removes the home directory and line / column numbers of vendored files. - .replace( - /\(.*\/node_modules\/([^:]*):.+/g, - '(REDACTED)/node_modules/$1:(REDACTED)' - ); - expect(container.firstChild).toMatchSnapshot(); }); @@ -55,7 +43,7 @@ describe('app/ErrorBoundary', function () { it('shows the error message children when the component throws error', () => { const { getByText } = setupComponent(); - expect(getByText(friendlyErrorMessage)).toBeInTheDocument(); + expect(getByText(new RegExp(friendlyErrorMessage))).toBeInTheDocument(); }); it('surfaces the error via console.error', () => { @@ -63,19 +51,6 @@ describe('app/ErrorBoundary', function () { expect(console.error).toHaveBeenCalled(); }); - it('can click the component to view the full details', () => { - const { getByText, getByTestId } = setupComponent(); - - // The technical error isn't visible yet. - expect(getByTestId('error-technical-details')).toHaveClass('hide'); - - // Click the button to expand the details. - fireFullClick(getByText('View full error details')); - - // The technical error now exists. - expect(getByTestId('error-technical-details')).not.toHaveClass('hide'); - }); - it('reports errors to the analytics', () => { withAnalyticsMock(() => { setupComponent(); @@ -87,8 +62,9 @@ describe('app/ErrorBoundary', function () { new RegExp(stripIndent` Error: This is an error\\. - at ThrowingComponent \\(.*[/\\\\]ErrorBoundary.test.js:20:11\\) - at ErrorBoundary \\(.*[/\\\\]ErrorBoundary.js:28:66\\) + at ThrowingComponent \\(.*[/\\\\]ErrorBoundary.test.js:.*\\) + at ErrorBoundaryInternal \\(.*[/\\\\]ErrorBoundary.js:.*\\) + at NonLocalizedErrorBoundary at LocalizationProvider \\(.*[/\\\\]@fluent[/\\\\]react[/\\\\]index.js:.*\\) `) ), @@ -97,3 +73,26 @@ describe('app/ErrorBoundary', function () { }); }); }); + +describe('app/LocalizedErrorBoundary', function () { + const friendlyErrorMessage = 'Oops, there was an error'; + const technicalErrorMessage = 'This is an error.'; + const ThrowingComponent = () => { + throw new Error(technicalErrorMessage); + }; + + function setupComponent(childComponent) { + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const results = render( + + {childComponent} + + ); + return { spy, ...results }; + } + + it('matches the snapshot', () => { + setupComponent(); + expect(document.body).toMatchSnapshot(); + }); +}); diff --git a/src/test/components/__snapshots__/ErrorBoundary.test.js.snap b/src/test/components/__snapshots__/ErrorBoundary.test.js.snap index 7d227db6c4..cdd9bfdbbf 100644 --- a/src/test/components/__snapshots__/ErrorBoundary.test.js.snap +++ b/src/test/components/__snapshots__/ErrorBoundary.test.js.snap @@ -1,6 +1,48 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`app/ErrorBoundary matches the snapshot 1`] = ` +exports[`app/LocalizedErrorBoundary matches the snapshot 1`] = ` + +
+
+
+
+
+

+ Oops, there was an error +

+

+ Error: This is an error. + . +

+

+ Please report this issue to the developers, including the full +error as displayed in the Developer Tools’ Web Console. +

+
+ + Report the error on GitHub + +
+
+
+
+ +`; + +exports[`app/NonLocalizedErrorBoundary matches the snapshot 1`] = `
@@ -8,34 +50,30 @@ exports[`app/ErrorBoundary matches the snapshot 1`] = ` class="appErrorBoundaryContents" >
- Oops, there was an error +

+ Oops, there was an error +

+

+ Error: This is an error. + . +

+

+ Please report this error to the developers, including the full error as displayed in the Developer Tools’ Web Console. +

- -
-
-
- Error: This is an error. -
-
- - at ThrowingComponent (REDACTED)/src/test/components/ErrorBoundary.test.js:20:11) - at ErrorBoundary (REDACTED)/src/components/app/ErrorBoundary.js:28:66) - at LocalizationProvider (REDACTED)/node_modules/@fluent/react/index.js:(REDACTED) -
+ Report the error on GitHub +