Add global error bubble

Summary: Improved the display of error messages. Where previously only one error message could be displayed (quite obtrusively), with this change multiple errors can now be displayed and stack traces and further info can be hidden. Reviewed By: passy Differential Revision: D18036569 fbshipit-source-id: 2bc3dfa7a5196f931370a6e6dbf27c55b6cfb2bf
2019-10-22 08:45:11 -07:00
parent d350e1b339
commit 3b1429b8b0
6 changed files with 303 additions and 46 deletions
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -8,7 +8,7 @@
 */

 import React from 'react';
-import {FlexColumn, FlexRow, Client} from 'flipper';
+import {FlexColumn, FlexRow} from 'flipper';
 import {connect} from 'react-redux';
 import TitleBar from './chrome/TitleBar';
 import MainSidebar from './chrome/MainSidebar';
@@ -36,7 +36,7 @@ import {
 import {Logger} from './fb-interfaces/Logger';
 import BugReporter from './fb-stubs/BugReporter';
 import {State as Store} from './reducers/index';
-import {StaticView} from './reducers/connections';
+import {StaticView, FlipperError} from './reducers/connections';
 import PluginManager from './chrome/PluginManager';
 import StatusBar from './chrome/StatusBar';
 import SettingsSheet from './chrome/SettingsSheet';
@@ -49,7 +49,7 @@ type OwnProps = {

 type StateFromProps = {
  leftSidebarVisible: boolean;
-  error: string | null;
+  errors: FlipperError[];
  activeSheet: ActiveSheet;
  share: ShareType | null;
  staticView: StaticView;
@@ -126,6 +126,7 @@ export class App extends React.Component<Props> {
    return (
      <FlexColumn grow={true}>
        <TitleBar version={version} />
+        <ErrorBar />
        <Sheet>{this.getSheet}</Sheet>
        <FlexRow grow={true}>
          {this.props.leftSidebarVisible && <MainSidebar />}
@@ -136,7 +137,6 @@ export class App extends React.Component<Props> {
          )}
        </FlexRow>
        <StatusBar />
-        <ErrorBar text={this.props.error} />
      </FlexColumn>
    );
  }
@@ -145,12 +145,12 @@ export class App extends React.Component<Props> {
 export default connect<StateFromProps, {}, OwnProps, Store>(
  ({
    application: {leftSidebarVisible, activeSheet, share},
-    connections: {error, staticView},
+    connections: {errors, staticView},
  }) => ({
    leftSidebarVisible,
    activeSheet,
    share: share,
-    error,
+    errors,
    staticView,
  }),
 )(App);
--- a/src/chrome/ErrorBar.tsx
+++ b/src/chrome/ErrorBar.tsx
@@ -8,28 +8,158 @@
 */

 import {styled, colors} from 'flipper';
-import React from 'react';
+import React, {useState, memo} from 'react';
+import {connect} from 'react-redux';
+import {FlipperError, dismissError} from '../reducers/connections';
+import {State as Store} from '../reducers/index';
+import {ErrorBlock, ButtonGroup, Button} from 'flipper';
+import {FlexColumn, FlexRow} from 'flipper';

-const ErrorBarContainer = styled('div')({
-  backgroundColor: colors.cherry,
-  bottom: 0,
-  color: '#fff',
-  left: 0,
-  lineHeight: '26px',
-  position: 'absolute',
-  right: 0,
-  textAlign: 'center',
-  zIndex: 2,
-});
-
-type Props = {
-  text: string | null | undefined;
+type StateFromProps = {
+  errors: FlipperError[];
 };

-export default function ErrorBar(props: Props) {
-  if (props.text == null) {
+type DispatchFromProps = {
+  dismissError: typeof dismissError;
+};
+
+type Props = DispatchFromProps & StateFromProps;
+
+const ErrorBar = memo(function ErrorBar(props: Props) {
+  const [collapsed, setCollapsed] = useState(false);
+
+  if (!props.errors.length) {
    return null;
-  } else {
-    return <ErrorBarContainer>{props.text}</ErrorBarContainer>;
  }
+
+  const errorCount = props.errors.reduce(
+    (sum, error) => sum + (error.occurrences || 1),
+    0,
+  );
+
+  return (
+    <ErrorBarContainer>
+      <ErrorRows className={collapsed ? 'collapsed' : ''}>
+        {props.errors.map((error, index) => (
+          <ErrorTile
+            onDismiss={() => props.dismissError(index)}
+            key={index}
+            error={error}
+          />
+        ))}
+      </ErrorRows>
+      <DismissAllErrors
+        onClick={() => setCollapsed(c => !c)}
+        title="Show / hide errors">
+        {collapsed ? `▼ ${errorCount}` : '▲'}
+      </DismissAllErrors>
+    </ErrorBarContainer>
+  );
+});
+
+export default connect<StateFromProps, DispatchFromProps, {}, Store>(
+  ({connections: {errors}}) => ({
+    errors,
+  }),
+  {
+    dismissError,
+  },
+)(ErrorBar);
+
+function ErrorTile({
+  onDismiss,
+  error,
+}: {
+  onDismiss: () => void;
+  error: FlipperError;
+}) {
+  const [collapsed, setCollapsed] = useState(true);
+  return (
+    <ErrorRow>
+      <FlexRow>
+        <FlexColumn style={{flexGrow: 1}}>{error.message}</FlexColumn>
+        {error.occurrences! > 1 && (
+          <ErrorCounter title="Nr of times this error occurred">
+            {error.occurrences}
+          </ErrorCounter>
+        )}
+        <FlexColumn style={{marginLeft: '8px'}}>
+          <ButtonGroup>
+            {(error.details || error.error) && (
+              <Button onClick={() => setCollapsed(s => !s)}>
+                {collapsed ? `▼ ` : '▲ '} Details
+              </Button>
+            )}
+            <Button onClick={onDismiss}>Dismiss</Button>
+          </ButtonGroup>
+        </FlexColumn>
+      </FlexRow>
+      {!collapsed && (
+        <FlexRow>
+          <ErrorDetails>
+            {error.details}
+            {error.error && <ErrorBlock error={error.error} />}
+          </ErrorDetails>
+        </FlexRow>
+      )}
+    </ErrorRow>
+  );
 }
+
+const ErrorBarContainer = styled('div')({
+  boxShadow: '2px 2px 2px #ccc',
+  userSelect: 'text',
+});
+
+const DismissAllErrors = styled('div')({
+  boxShadow: '2px 2px 2px #ccc',
+  backgroundColor: colors.cherryDark3,
+  color: '#fff',
+  textAlign: 'center',
+  borderBottomLeftRadius: '4px',
+  borderBottomRightRadius: '4px',
+  position: 'absolute',
+  width: '48px',
+  height: '16px',
+  zIndex: 2,
+  right: '20px',
+  fontSize: '6pt',
+  lineHeight: '16px',
+  cursor: 'pointer',
+  alignItems: 'center',
+});
+
+const ErrorDetails = styled('div')({
+  width: '100%',
+  marginTop: 4,
+});
+
+const ErrorRows = styled('div')({
+  backgroundColor: colors.cherry,
+  color: '#fff',
+  maxHeight: '600px',
+  overflowY: 'auto',
+  overflowX: 'hidden',
+  transition: 'max-height 0.3s ease',
+  '&.collapsed': {
+    maxHeight: '0px',
+  },
+});
+
+const ErrorRow = styled('div')({
+  padding: '4px 12px',
+  borderBottom: '1px solid ' + colors.cherryDark3,
+  verticalAlign: 'middle',
+  lineHeight: '28px',
+});
+
+const ErrorCounter = styled(FlexColumn)({
+  backgroundColor: colors.cherryDark3,
+  color: colors.cherry,
+  width: 24,
+  height: 24,
+  borderRadius: 24,
+  marginTop: 2,
+  lineHeight: '24px',
+  textAlign: 'center',
+});
--- a/src/dispatcher/server.tsx
+++ b/src/dispatcher/server.tsx
@@ -49,14 +49,14 @@ export default (store: Store, logger: Logger) => {
  });

  server.addListener('error', err => {
-    const payload: string =
+    const message: string =
      err.code === 'EADDRINUSE'
        ? "Couldn't start websocket server. Looks like you have multiple copies of Flipper running."
        : err.message || 'Unknown error';

    store.dispatch({
      type: 'SERVER_ERROR',
-      payload,
+      payload: {message},
    });
  });

--- a/src/reducers/tests/connections.node.tsx
+++ b/src/reducers/tests/connections.node.tsx
@@ -14,16 +14,85 @@ import BaseDevice from '../../devices/BaseDevice';
 test('REGISTER_DEVICE doesnt remove error', () => {
  const initialState: State = reducer(undefined, {
    type: 'SERVER_ERROR',
-    payload: 'something went wrong',
+    payload: {message: 'something went wrong'},
  });

  // Precondition
-  expect(initialState.error).toEqual('something went wrong');
+  expect(initialState.errors).toEqual([
+    {message: 'something went wrong', occurrences: 1},
+  ]);

  const endState = reducer(initialState, {
    type: 'REGISTER_DEVICE',
    payload: new BaseDevice('serial', 'physical', 'title', 'Android'),
  });

-  expect(endState.error).toEqual('something went wrong');
+  expect(endState.errors).toEqual([
+    {message: 'something went wrong', occurrences: 1},
+  ]);
+});
+
+test('errors are collected on a by name basis', () => {
+  const initialState: State = reducer(undefined, {
+    type: 'SERVER_ERROR',
+    payload: {
+      message: 'error1',
+      error: 'stack1',
+    },
+  });
+
+  expect(initialState.errors).toMatchInlineSnapshot(`
+        Array [
+          Object {
+            "error": "stack1",
+            "message": "error1",
+            "occurrences": 1,
+          },
+        ]
+    `);
+
+  const state2: State = reducer(initialState, {
+    type: 'SERVER_ERROR',
+    payload: {
+      message: 'error2',
+      error: 'stack2',
+    },
+  });
+  // There are now two errors
+  expect(state2.errors).toMatchInlineSnapshot(`
+        Array [
+          Object {
+            "error": "stack1",
+            "message": "error1",
+            "occurrences": 1,
+          },
+          Object {
+            "error": "stack2",
+            "message": "error2",
+            "occurrences": 1,
+          },
+        ]
+    `);
+  const state3: State = reducer(state2, {
+    type: 'SERVER_ERROR',
+    payload: {
+      message: 'error1',
+      error: 'stack3',
+    },
+  });
+  // Still two errors, but error1 has been updated and occurrences increased
+  expect(state3.errors).toMatchInlineSnapshot(`
+    Array [
+      Object {
+        "error": "stack3",
+        "message": "error1",
+        "occurrences": 2,
+      },
+      Object {
+        "error": "stack2",
+        "message": "error2",
+        "occurrences": 1,
+      },
+    ]
+  `);
 });
--- a/src/reducers/connections.tsx
+++ b/src/reducers/connections.tsx
@@ -27,6 +27,13 @@ export type StaticView =
  | typeof WelcomeScreen
  | typeof SupportRequestForm;

+export type FlipperError = {
+  occurrences?: number;
+  message: string;
+  details?: string;
+  error?: Error | string;
+};
+
 export type State = {
  devices: Array<BaseDevice>;
  androidEmulators: Array<string>;
@@ -37,7 +44,7 @@ export type State = {
  userPreferredPlugin: null | string;
  userPreferredApp: null | string;
  userLRUPlugins: {[key: string]: Array<string>};
-  error: null | string;
+  errors: FlipperError[];
  clients: Array<Client>;
  uninitializedClients: Array<{
    client: UninitializedClient;
@@ -79,7 +86,7 @@ export type Action =
    }
  | {
      type: 'SERVER_ERROR';
-      payload: null | string;
+      payload: null | FlipperError;
    }
  | {
      type: 'NEW_CLIENT';
@@ -107,7 +114,7 @@ export type Action =
    }
  | {
      type: 'CLIENT_SETUP_ERROR';
-      payload: {client: UninitializedClient; error: Error};
+      payload: {client: UninitializedClient; error: FlipperError};
    }
  | {
      type: 'CLIENT_SHOW_MORE_OR_LESS';
@@ -117,6 +124,10 @@ export type Action =
  | {
      type: 'SET_STATIC_VIEW';
      payload: StaticView;
+    }
+  | {
+      type: 'DISMISS_ERROR';
+      payload: number;
    };

 const DEFAULT_PLUGIN = 'DeviceLogs';
@@ -131,7 +142,7 @@ const INITAL_STATE: State = {
  userPreferredPlugin: null,
  userPreferredApp: null,
  userLRUPlugins: {},
-  error: null,
+  errors: [],
  clients: [],
  uninitializedClients: [],
  deepLinkPayload: null,
@@ -356,7 +367,10 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => {
    }
    case 'SERVER_ERROR': {
      const {payload} = action;
-      return {...state, error: payload};
+      if (!payload) {
+        return state;
+      }
+      return {...state, errors: mergeError(state.errors, payload)};
    }
    case 'START_CLIENT_SETUP': {
      const {payload} = action;
@@ -385,10 +399,11 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => {
      const {payload} = action;

      const errorMessage =
-        payload.error instanceof Error ? payload.error.message : payload.error;
-      console.error(
-        `Client setup error: ${errorMessage} while setting up client: ${payload.client.os}:${payload.client.deviceName}:${payload.client.appName}`,
-      );
+        payload.error instanceof Error
+          ? payload.error.message
+          : '' + payload.error;
+      const details = `Client setup error: ${errorMessage} while setting up client: ${payload.client.os}:${payload.client.deviceName}:${payload.client.appName}`;
+      console.error(details);
      return {
        ...state,
        uninitializedClients: state.uninitializedClients
@@ -398,7 +413,11 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => {
              : c,
          )
          .sort((a, b) => a.client.appName.localeCompare(b.client.appName)),
-        error: `Client setup error: ${errorMessage}`,
+        errors: mergeError(state.errors, {
+          message: `Client setup error: ${errorMessage}`,
+          details,
+          error: payload.error instanceof Error ? payload.error : undefined,
+        }),
      };
    }
    case 'CLIENT_SHOW_MORE_OR_LESS': {
@@ -428,6 +447,14 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => {
        userLRUPlugins: clearLRUPlugins,
      };
    }
+    case 'DISMISS_ERROR': {
+      const errors = state.errors.slice();
+      errors.splice(action.payload, 1);
+      return {
+        ...state,
+        errors,
+      };
+    }
    default:
      return state;
  }
@@ -438,23 +465,48 @@ export default (state: State = INITAL_STATE, action: Actions): State => {

  if (nextState.selectedDevice) {
    const {selectedDevice} = nextState;
-    const deviceNotSupportedError = 'iOS Devices are not yet supported';
+    const deviceNotSupportedErrorMessage = 'iOS Devices are not yet supported';
    const error =
      selectedDevice.os === 'iOS' &&
      selectedDevice.deviceType === 'physical' &&
      !iosUtil.isAvailable()
-        ? deviceNotSupportedError
+        ? deviceNotSupportedErrorMessage
        : null;

-    if (nextState.error === deviceNotSupportedError) {
-      nextState.error = error;
-    } else {
-      nextState.error = error || nextState.error;
+    if (error) {
+      const deviceNotSupportedError = nextState.errors.find(
+        error => error.message === deviceNotSupportedErrorMessage,
+      );
+      if (deviceNotSupportedError) {
+        deviceNotSupportedError.message = error;
+      } else {
+        nextState.errors.push({message: error});
+      }
    }
  }
  return nextState;
 };

+function mergeError(
+  errors: FlipperError[],
+  newError: FlipperError,
+): FlipperError[] {
+  const idx = errors.findIndex(error => error.message === newError.message);
+  const results = errors.slice();
+  if (idx !== -1) {
+    results[idx] = {
+      ...newError,
+      occurrences: (errors[idx].occurrences || 0) + 1,
+    };
+  } else {
+    results.push({
+      ...newError,
+      occurrences: 1,
+    });
+  }
+  return results;
+}
+
 export const selectDevice = (payload: BaseDevice): Action => ({
  type: 'SELECT_DEVICE',
  payload,
@@ -495,3 +547,8 @@ function extractAppNameFromAppId(appId: string | null): string | null {
  // Expect the name of the app to be on the first matching
  return matchedRegex && matchedRegex[1];
 }
+
+export const dismissError = (index: number): Action => ({
+  type: 'DISMISS_ERROR',
+  payload: index,
+});
--- a/src/ui/components/ErrorBlock.tsx
+++ b/src/ui/components/ErrorBlock.tsx
@@ -18,6 +18,7 @@ export const ErrorBlockContainer = styled(CodeBlock)({
  color: '#a94442',
  overflow: 'auto',
  padding: 10,
+  whiteSpace: 'pre',
 });

 /**