update(ErrorMessage,ErrorBoundary): Show error info and error/trace I…

…Ds (#363) * update(ErrorMessage): show error and trace ids in plaintext * update(ErrorBoundary): Use ErrorMessage internally * Fix type Co-authored-by: Tyler Geonetta <tyler.geonetta@airbnb.com>
airbnb · Apr 24, 2020 · bf0965a · bf0965a
1 parent d9e6734
commit bf0965a
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 133 deletions.
diff --git a/packages/core/src/components/ErrorBoundary/index.tsx b/packages/core/src/components/ErrorBoundary/index.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import componentName from '../../prop-types/componentName';
-import Tripped from './private/Tripped';
 import { Logger } from '../../types';
+import ErrorMessage from '../ErrorMessage';
 
 export type ErrorBoundaryProps = {
   /** Content to wrap. */
@@ -26,7 +26,7 @@ export default class ErrorBoundary extends React.Component<ErrorBoundaryProps, E
     name: 'UnknownBoundary',
   };
 
-  state = {
+  state: ErrorBoundaryState = {
     error: null,
   };
 
@@ -49,6 +49,6 @@ export default class ErrorBoundary extends React.Component<ErrorBoundaryProps, E
       return this.props.children;
     }
 
-    return <Tripped />;
+    return <ErrorMessage error={error} />;
   }
 }
diff --git a/packages/core/src/components/ErrorBoundary/private/Tripped.tsx b/packages/core/src/components/ErrorBoundary/private/Tripped.tsx
diff --git a/packages/core/src/components/ErrorBoundary/story.tsx b/packages/core/src/components/ErrorBoundary/story.tsx
@@ -1,8 +1,11 @@
 import React from 'react';
 import ErrorBoundary from '.';
+import { ErrorObject } from '../..';
 
 function TestComponent() {
-  throw new Error('This was thrown from a child.');
+  const error = new Error('This was thrown from a child.');
+  (error as ErrorObject).error_id = '6db2d1a0275c97134535b4681914f8b6d';
+  throw error;
 
   // eslint-disable-next-line
   return null;

diff --git a/packages/core/src/components/ErrorMessage/index.tsx b/packages/core/src/components/ErrorMessage/index.tsx
@@ -1,12 +1,11 @@
 import React from 'react';
 import T from '../Translate';
 import Alert from '../Alert';
-import MutedButton from '../MutedButton';
 import Spacing from '../Spacing';
 import StatusText from '../StatusText';
 import { ErrorType } from '../../types';
-import Core from '../..';
-import ButtonGroup from '../ButtonGroup';
+import Copy from '../Copy';
+import StatusLabel from '../StatusLabel';
 
 export function getErrorMessage(error: string | ErrorType, includeCode: boolean = false): string {
   if (typeof error === 'string') {
@@ -63,36 +62,37 @@ export default function ErrorMessage({
   return (
     <Alert
       danger
-      title={title || code || <T k="lunar.error.unknown" phrase="Unknown error" />}
+      title={
+        title ||
+        code || (
+          <T k="lunar.error.featureCrashed" phrase="This feature has crashed or failed to load." />
+        )
+      }
       onClose={onClose}
     >
       {message}
 
-      {(errorID || traceID) && (
+      {errorID && (
         <Spacing top={1}>
-          <ButtonGroup>
-            {errorID && (
-              <MutedButton
-                inverted
-                small
-                openInNewWindow
-                href={error.error_url || Core.settings.errorURL.replace('{{id}}', errorID)}
-              >
-                <T k="lunar.error.viewDetails" phrase="View error details" />
-              </MutedButton>
-            )}
+          <StatusLabel compact>
+            <T k="lunar.error.id" phrase="Error ID" />
+          </StatusLabel>
+          <StatusText inline bold muted micro>
+            {errorID}
+          </StatusText>
+          <Copy text={errorID} />
+        </Spacing>
+      )}
 
-            {traceID && (
-              <MutedButton
-                inverted
-                small
-                openInNewWindow
-                href={Core.settings.traceURL.replace('{{id}}', traceID)}
-              >
-                <T k="lunar.trace.viewDetails" phrase="View trace details" />
-              </MutedButton>
-            )}
-          </ButtonGroup>
+      {traceID && (
+        <Spacing top={1}>
+          <StatusLabel compact>
+            <T k="lunar.trace.id" phrase="Trace ID" />
+          </StatusLabel>
+          <StatusText inline bold muted micro>
+            {traceID}
+          </StatusText>
+          <Copy text={traceID} />
         </Spacing>
       )}
     </Alert>

diff --git a/packages/core/src/components/ErrorMessage/story.tsx b/packages/core/src/components/ErrorMessage/story.tsx
@@ -35,8 +35,8 @@ fromAnApiEndpointError.story = {
 
 export function fromAnApiEndpointErrorWithTraceID() {
   // Would be shown for an APIError from airbnb-api-resource.
-  const error = new Error('Oh noes') as ErrorObject;
-  error.trace_id = 'tRaCeId==';
+  const error = new Error('This is an error message.') as ErrorObject;
+  error.trace_id = 'tRaDDDDS34534534qqqqqCeId==';
 
   return <ErrorMessage error={error} />;
 }

diff --git a/packages/core/test/components/ErrorBoundary.test.tsx b/packages/core/test/components/ErrorBoundary.test.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import ErrorBoundary from '../../src/components/ErrorBoundary';
-import Tripped from '../../src/components/ErrorBoundary/private/Tripped';
+import ErrorMessage from '../../src/components/ErrorMessage';
 
 describe('<ErrorBoundary />', () => {
   const props = {
@@ -19,15 +19,17 @@ describe('<ErrorBoundary />', () => {
     const wrapper = shallow(<ErrorBoundary {...props}>Foo</ErrorBoundary>);
 
     expect(wrapper.contains('Foo')).toBe(true);
-    expect(wrapper.find(Tripped)).toHaveLength(0);
+    expect(wrapper.find(ErrorMessage)).toHaveLength(0);
   });
 
   it('will render a simple alert', () => {
     const wrapper = shallow(<ErrorBoundary {...props}>Foo</ErrorBoundary>);
 
-    wrapper.setState({ error: new Error() });
+    const error = new Error();
+    wrapper.setState({ error });
 
-    expect(wrapper.find(Tripped)).toHaveLength(1);
+    expect(wrapper.find(ErrorMessage)).toHaveLength(1);
+    expect(wrapper.find(ErrorMessage).prop('error')).toBe(error);
   });
 
   it('will catch error and set state', () => {

diff --git a/packages/core/test/components/ErrorBoundary/Tripped.test.tsx b/packages/core/test/components/ErrorBoundary/Tripped.test.tsx
diff --git a/packages/core/test/components/ErrorBoundary/__snapshots__/Tripped.test.tsx.snap b/packages/core/test/components/ErrorBoundary/__snapshots__/Tripped.test.tsx.snap
diff --git a/packages/core/test/components/ErrorMessage.test.tsx b/packages/core/test/components/ErrorMessage.test.tsx
@@ -2,7 +2,6 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import T from '../../src/components/Translate';
 import Alert from '../../src/components/Alert';
-import MutedButton from '../../src/components/MutedButton';
 import ErrorMessage, { getErrorMessage } from '../../src/components/ErrorMessage';
 import StatusText from '../../src/components/StatusText';
 import { ErrorObject } from '../../src/types';
@@ -110,7 +109,9 @@ describe('<ErrorMessage />', () => {
     const alert = wrapper.find(Alert);
 
     expect(alert).toHaveLength(1);
-    expect(alert.prop('title')).toEqual(<T k="lunar.error.unknown" phrase="Unknown error" />);
+    expect(alert.prop('title')).toEqual(
+      <T k="lunar.error.featureCrashed" phrase="This feature has crashed or failed to load." />,
+    );
     expect(alert.contains('Failure')).toBe(true);
   });
 
@@ -160,7 +161,7 @@ describe('<ErrorMessage />', () => {
     expect(alert.contains('Failure')).toBe(true);
   });
 
-  it('renders a button if an error ID is passed', () => {
+  it('renders an error ID if error object passed', () => {
     const wrapper = shallow(
       <ErrorMessage
         error={{
@@ -169,12 +170,7 @@ describe('<ErrorMessage />', () => {
       />,
     );
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.error.viewDetails" phrase="View error details" />),
-    ).toBe(false);
+    expect(wrapper.find(Alert).find(StatusText)).toHaveLength(0);
 
     wrapper.setProps({
       error: {
@@ -183,28 +179,15 @@ describe('<ErrorMessage />', () => {
       },
     });
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.error.viewDetails" phrase="View error details" />),
-    ).toBe(true);
-    expect(wrapper.find(Alert).find(MutedButton).prop('href')).toBe(
-      'http://error-url-test.com/ABC',
-    );
+    expect(wrapper.find(Alert).find(StatusText).prop('children')).toMatch('ABC');
   });
 
-  it('renders a button if an error instance containing an error ID is passed', () => {
+  it('renders an error ID if error instance passed', () => {
     const error = new Error('Whatever') as ErrorObject; // Satisfy TS.
 
     const wrapper = shallow(<ErrorMessage error={error} />);
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.error.viewDetails" phrase="View error details" />),
-    ).toBe(false);
+    expect(wrapper.find(Alert).find(StatusText)).toHaveLength(0);
 
     error.error_message = 'Failure';
     error.error_id = 'ABC';
@@ -213,18 +196,10 @@ describe('<ErrorMessage />', () => {
       error,
     });
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.error.viewDetails" phrase="View error details" />),
-    ).toBe(true);
-    expect(wrapper.find(Alert).find(MutedButton).prop('href')).toBe(
-      'http://error-url-test.com/ABC',
-    );
+    expect(wrapper.find(Alert).find(StatusText).prop('children')).toMatch('ABC');
   });
 
-  it('renders a button if an trace ID is passed', () => {
+  it('renders a trace ID if passed', () => {
     const error = new Error('Whatever') as ErrorObject; // Satisfy TS.
 
     const wrapper = shallow(
@@ -235,27 +210,14 @@ describe('<ErrorMessage />', () => {
       />,
     );
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.trace.viewDetails" phrase="View trace details" />),
-    ).toBe(false);
+    expect(wrapper.find(Alert).find(StatusText)).toHaveLength(0);
 
     error.trace_id = 'tRaCiD1337==';
 
     wrapper.setProps({
       error,
     });
 
-    expect(
-      wrapper
-        .find(Alert)
-        .find(MutedButton)
-        .contains(<T k="lunar.trace.viewDetails" phrase="View trace details" />),
-    ).toBe(true);
-    expect(wrapper.find(Alert).find(MutedButton).prop('href')).toBe(
-      'http://trace-url-test.com/tRaCiD1337==',
-    );
+    expect(wrapper.find(Alert).find(StatusText).prop('children')).toMatch('tRaCiD1337==');
   });
 });