Add error logs for API calls

Author: willdurand

src/api/index.tsx

@@ -20,7 +20,7 @@ type CallApiParams = {
   query?: { [key: string]: string };
 };
 
-type ErrorResponseType = { error: Error };
+export type ErrorResponseType = { error: Error };
 
 type CallApiResponse<SuccessResponseType> =
   | SuccessResponseType

src/components/App/index.spec.tsx

@@ -8,23 +8,28 @@ import configureStore from '../../configureStore';
 import { actions as apiActions } from '../../reducers/api';
 import { actions as userActions } from '../../reducers/users';
 import {
+  updateWithProps,
   createContextWithFakeRouter,
   fakeUser,
+  getFakeLogger,
   shallowUntilTarget,
 } from '../../test-helpers';
 import Navbar from '../Navbar';
+import * as api from '../../api';
 
-import App, { AppBase } from '.';
+import App, { AppBase, DefaultProps } from '.';
 
 describe(__filename, () => {
   type RenderParams = {
-    store?: Store;
+    _log?: DefaultProps['_log'];
     authToken?: string | null;
+    store?: Store;
   };
 
-  const render = ({
-    store = configureStore(),
+  const render = async ({
+    _log,
     authToken = 'some-token',
+    store = configureStore(),
   }: RenderParams = {}) => {
     const contextWithRouter = createContextWithFakeRouter();
     const context = {
@@ -35,71 +40,125 @@ describe(__filename, () => {
       },
     };
 
-    const root = shallowUntilTarget(<App authToken={authToken} />, AppBase, {
+    const props = {
+      _log,
+      authToken,
+    };
+
+    return shallowUntilTarget(<App {...props} />, AppBase, {
       shallowOptions: { ...context },
     });
-
-    return root;
   };
 
-  it('renders without an authentication token', () => {
-    const root = render({ authToken: null });
+  it('renders without an authentication token', async () => {
+    const root = await render({ authToken: null });
 
     expect(root.find(Container)).toHaveClassName(styles.container);
     expect(root.find(`.${styles.content}`)).toHaveLength(1);
     expect(root.find(Navbar)).toHaveLength(1);
   });
 
-  it('renders with an empty authentication token', () => {
-    const root = render({ authToken: '' });
+  it('renders with an empty authentication token', async () => {
+    const root = await render({ authToken: '' });
 
     expect(root.find(Container)).toHaveClassName(styles.container);
     expect(root.find(`.${styles.content}`)).toHaveLength(1);
     expect(root.find(Navbar)).toHaveLength(1);
   });
 
-  it('displays a loading message until the user profile gets loaded', () => {
-    const root = render();
+  it('displays a loading message until the user profile gets loaded', async () => {
+    const root = await render();
 
     expect(root).toIncludeText('Getting your workspace ready');
     expect(root.find(Navbar)).toHaveLength(0);
   });
 
-  it('dispatches setAuthToken on mount when authToken is valid', () => {
+  it('dispatches setAuthToken on mount when authToken is valid', async () => {
     const authToken = 'my-token';
     const store = configureStore();
     const dispatch = jest.spyOn(store, 'dispatch');
 
-    render({ authToken, store });
+    await render({ authToken, store });
 
     expect(dispatch).toHaveBeenCalledWith(
       apiActions.setAuthToken({ authToken }),
     );
   });
 
-  it('does not dispatch setAuthToken on mount when authToken is null', () => {
+  it('does not dispatch setAuthToken on mount when authToken is null', async () => {
     const store = configureStore();
     const dispatch = jest.spyOn(store, 'dispatch');
 
-    render({ authToken: null, store });
+    await render({ authToken: null, store });
 
     expect(dispatch).not.toHaveBeenCalled();
   });
 
-  it('configures no route when the user is not logged in', () => {
-    const root = render({ authToken: null });
+  it('configures no route when the user is not logged in', async () => {
+    const root = await render({ authToken: null });
 
     expect(root.find(Route)).toHaveLength(0);
     expect(root.find(`.${styles.loginMessage}`)).toIncludeText('Please log in');
   });
 
-  it('exposes more routes when the user is logged in', () => {
+  it('exposes more routes when the user is logged in', async () => {
     const store = configureStore();
     store.dispatch(userActions.loadCurrentUser({ user: fakeUser }));
 
-    const root = render({ store });
+    const root = await render({ store });
 
     expect(root.find(Route)).toHaveLength(3);
     expect(root.find(`.${styles.loginMessage}`)).toHaveLength(0);
   });
+
+  it('dispatches loadCurrentUser() on update', async () => {
+    const _log = getFakeLogger();
+    const authToken = 'my-token';
+    const user = fakeUser;
+
+    const store = configureStore();
+    const dispatch = jest.spyOn(store, 'dispatch');
+
+    const mockApi = jest.spyOn(api, 'getCurrentUserProfile');
+    mockApi.mockReturnValue(Promise.resolve(user));
+
+    const root = await render({ _log, authToken: null, store });
+
+    // Set the authentication token into the state.
+    store.dispatch(apiActions.setAuthToken({ authToken }));
+    const { api: apiState } = store.getState();
+    dispatch.mockReset();
+
+    // Wait until componentDidUpdate() logic has been executed with the new
+    // props.
+    await updateWithProps(root, { apiState });
+
+    expect(dispatch).toHaveBeenCalledWith(
+      userActions.loadCurrentUser({
+        user,
+      }),
+    );
+  });
+
+  it('logs an error when the API request to retrieve the current user profile has failed', async () => {
+    const _log = getFakeLogger();
+    const authToken = 'my-token';
+    const store = configureStore();
+
+    const error = new Error('server error');
+    const mockApi = jest.spyOn(api, 'getCurrentUserProfile');
+    mockApi.mockReturnValue(Promise.resolve({ error }));
+
+    const root = await render({ _log, store });
+
+    // Set the authentication token into the state.
+    store.dispatch(apiActions.setAuthToken({ authToken }));
+    const { api: apiState } = store.getState();
+
+    // Wait until componentDidUpdate() logic has been executed with the new
+    // props.
+    await updateWithProps(root, { apiState });
+
+    expect(_log.error).toHaveBeenCalled();
+  });
 });

src/components/App/index.tsx

@@ -8,6 +8,7 @@ import {
   withRouter,
 } from 'react-router-dom';
 import makeClassName from 'classnames';
+import log from 'loglevel';
 
 import { ApplicationState, ConnectedReduxProps } from '../../configureStore';
 import styles from './styles.module.scss';
@@ -34,14 +35,23 @@ type PropsFromState = {
   profile: User | null;
 };
 
+export type DefaultProps = {
+  _log: typeof log;
+};
+
 /* eslint-disable @typescript-eslint/indent */
 type Props = PublicProps &
   PropsFromState &
+  DefaultProps &
   ConnectedReduxProps &
   RouteComponentProps<{}>;
 /* eslint-enable @typescript-eslint/indent */
 
 export class AppBase extends React.Component<Props> {
+  static defaultProps = {
+    _log: log,
+  };
+
   componentDidMount() {
     const { authToken, dispatch } = this.props;
 
@@ -51,12 +61,14 @@ export class AppBase extends React.Component<Props> {
   }
 
   async componentDidUpdate(prevProps: Props) {
-    const { apiState, dispatch, profile } = this.props;
+    const { _log, apiState, dispatch, profile } = this.props;
 
     if (!profile && prevProps.apiState.authToken !== apiState.authToken) {
       const response = await getCurrentUserProfile(apiState);
 
-      if (!isErrorResponse(response)) {
+      if (isErrorResponse(response)) {
+        _log.error(`TODO: handle this error response: ${response.error}`);
+      } else {
         dispatch(userActions.loadCurrentUser({ user: response }));
       }
     }

src/pages/Browse/index.spec.tsx

@@ -1,8 +1,10 @@
 import * as React from 'react';
+import { Store } from 'redux';
 
 import {
   createFakeHistory,
   fakeVersion,
+  getFakeLogger,
   shallowUntilTarget,
 } from '../../test-helpers';
 import * as api from '../../api';
@@ -13,7 +15,7 @@ import {
 } from '../../reducers/versions';
 import FileTree from '../../components/FileTree';
 
-import Browse, { BrowseBase } from '.';
+import Browse, { BrowseBase, DefaultProps } from '.';
 
 describe(__filename, () => {
   const createFakeRouteComponentProps = ({
@@ -35,13 +37,22 @@ describe(__filename, () => {
     };
   };
 
+  type RenderParams = {
+    _log?: DefaultProps['_log'];
+    addonId?: string;
+    versionId?: string;
+    store?: Store;
+  };
+
   const render = async ({
+    _log,
     addonId = '999',
     versionId = '123',
     store = configureStore(),
-  } = {}) => {
+  }: RenderParams = {}) => {
     const props = {
       ...createFakeRouteComponentProps({ params: { addonId, versionId } }),
+      _log,
     };
 
     return shallowUntilTarget(<Browse {...props} />, BrowseBase, {
@@ -102,4 +113,16 @@ describe(__filename, () => {
       versionActions.loadVersionInfo({ version }),
     );
   });
+
+  it('logs an error when the API request to retrieve the current user profile has failed', async () => {
+    const _log = getFakeLogger();
+    const error = new Error('server error');
+
+    const mockApi = jest.spyOn(api, 'getVersionFile');
+    mockApi.mockReturnValue(Promise.resolve({ error }));
+
+    await render({ _log });
+
+    expect(_log.error).toHaveBeenCalled();
+  });
 });

src/pages/Browse/index.tsx

@@ -2,6 +2,7 @@ import * as React from 'react';
 import { RouteComponentProps } from 'react-router-dom';
 import { connect } from 'react-redux';
 import { Col } from 'react-bootstrap';
+import log from 'loglevel';
 
 import { ApplicationState, ConnectedReduxProps } from '../../configureStore';
 import { ApiState } from '../../reducers/api';
@@ -23,15 +24,24 @@ type PropsFromState = {
   version: Version;
 };
 
+export type DefaultProps = {
+  _log: typeof log;
+};
+
 /* eslint-disable @typescript-eslint/indent */
 type Props = RouteComponentProps<PropsFromRouter> &
   PropsFromState &
+  DefaultProps &
   ConnectedReduxProps;
 /* eslint-enable @typescript-eslint/indent */
 
 export class BrowseBase extends React.Component<Props> {
+  static defaultProps = {
+    _log: log,
+  };
+
   async componentDidMount() {
-    const { apiState, dispatch, match } = this.props;
+    const { _log, apiState, dispatch, match } = this.props;
     const { addonId, versionId } = match.params;
 
     const response = await getVersionFile({
@@ -40,7 +50,9 @@ export class BrowseBase extends React.Component<Props> {
       versionId: parseInt(versionId, 10),
     });
 
-    if (!isErrorResponse(response)) {
+    if (isErrorResponse(response)) {
+      _log.error(`TODO: handle this error response: ${response.error}`);
+    } else {
       dispatch(versionActions.loadVersionInfo({ version: response }));
     }
   }

src/test-helpers.tsx

@@ -1,6 +1,7 @@
 import PropTypes from 'prop-types';
 import { History, Location } from 'history';
-import { shallow } from 'enzyme';
+import { shallow, ShallowWrapper } from 'enzyme';
+import log from 'loglevel';
 
 import { ExternalUser } from './reducers/users';
 import {
@@ -197,3 +198,30 @@ export const shallowUntilTarget = (
      gave up after ${maxTries} tries`,
   );
 };
+
+export const getFakeLogger = () => {
+  return {
+    ...log,
+    debug: jest.fn(),
+    error: jest.fn(),
+    info: jest.fn(),
+  };
+};
+
+export const updateWithProps = (root: ShallowWrapper, props: object) => {
+  return new Promise((resolve) => {
+    const instance = root.instance();
+    const oldComponentDidUpdate = instance.componentDidUpdate;
+
+    instance.componentDidUpdate = (...args) => {
+      if (oldComponentDidUpdate) {
+        instance.componentDidUpdate = oldComponentDidUpdate;
+        oldComponentDidUpdate.apply(instance, args);
+      }
+
+      resolve();
+    };
+
+    root.setProps(props);
+  });
+};