diff --git a/src/api/API.js b/src/api/API.js index 17a210612..fcdc7ccdc 100644 --- a/src/api/API.js +++ b/src/api/API.js @@ -3,7 +3,7 @@ import EventEmitter from 'events'; import axios from 'axios'; import { getHeaders } from '../util'; -import { PLACEHOLDER_USER, ANNOTATOR_EVENT, ERROR_TYPE } from '../constants'; +import { PLACEHOLDER_USER, ANNOTATOR_EVENT } from '../constants'; class API extends EventEmitter { /** @property {string} */ @@ -95,7 +95,7 @@ class API extends EventEmitter { */ errorHandler = (error: $AxiosError): void => { this.emit(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.auth, + reason: error.name, error: error.toString() }); }; diff --git a/src/api/AnnotationAPI.js b/src/api/AnnotationAPI.js index 536754b43..2402e55dd 100644 --- a/src/api/AnnotationAPI.js +++ b/src/api/AnnotationAPI.js @@ -1,6 +1,6 @@ // @flow import API from './API'; -import { ANNOTATOR_EVENT, ERROR_TYPE } from '../constants'; +import { ERROR_TYPE } from '../constants'; const HTTP_POST = 'POST'; const HTTP_DELETE = 'DELETE'; @@ -57,10 +57,8 @@ class AnnotationAPI extends API { createSuccessHandler = (data: Object): CommentProps => { if (data.type === 'error') { const error = new Error('Could not create annotation'); - this.emit(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.create, - error: error.toString() - }); + error.name = ERROR_TYPE.create; + this.errorHandler(error); return data; } @@ -94,10 +92,8 @@ class AnnotationAPI extends API { deleteSuccessHandler = (data: Object, id: string) => { if (data.type === 'error' || !data.id) { const error = new Error(`Could not delete annotation with ID ${id}`); - this.emit(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.delete, - error: error.toString() - }); + error.name = ERROR_TYPE.delete; + this.errorHandler(error); } return data; diff --git a/src/api/FileVersionAPI.js b/src/api/FileVersionAPI.js index 86ebf9862..1f2084e42 100644 --- a/src/api/FileVersionAPI.js +++ b/src/api/FileVersionAPI.js @@ -1,6 +1,6 @@ // @flow import API from './API'; -import { ANNOTATOR_EVENT, ERROR_TYPE, PLACEHOLDER_USER, TYPES } from '../constants'; +import { ERROR_TYPE, PLACEHOLDER_USER, TYPES } from '../constants'; const FIELDS = 'item,thread,details,message,created_by,created_at,modified_at,permissions'; @@ -46,7 +46,7 @@ class FileVersionAPI extends API { */ fetchVersionAnnotations(version: string): Promise { this.fileVersionId = version; - + // $FlowFixMe return this.fetchFromMarker({ version, fields: FIELDS }).then(this.createAnnotationMap); } @@ -77,11 +77,17 @@ class FileVersionAPI extends API { * @param {Params} queryParams - Key-value map of querystring params * @return {void} */ - successHandler = (data: Data, queryParams: Params): void => { + successHandler = (data: Data, queryParams: Params): Promise => { + if (data.type === 'error' || !Array.isArray(data.entries)) { + const error = new Error(`Could not read annotations from file version with ID ${this.fileVersionId}`); + error.name = ERROR_TYPE.read; + return Promise.reject(error); + } + const entries = this.data ? this.data.entries : []; this.data = { ...data, - entries: entries.concat(data.entries) + entries: [...entries, ...data.entries] }; const { next_marker } = data; @@ -91,16 +97,10 @@ class FileVersionAPI extends API { marker: next_marker }; - this.fetchFromMarker(params); + return this.fetchFromMarker(params); } - if (data.type === 'error' || !Array.isArray(data.entries)) { - const error = new Error(`Could not read annotations from file version with ID ${this.fileVersionId}`); - this.emit(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.read, - error: error.toString() - }); - } + return Promise.resolve(this.data); }; /** diff --git a/src/api/__tests__/API-test.js b/src/api/__tests__/API-test.js index acba6fff7..671b6f030 100644 --- a/src/api/__tests__/API-test.js +++ b/src/api/__tests__/API-test.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-expressions */ import API from '../API'; -import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants'; +import { ANNOTATOR_EVENT } from '../../constants'; const API_HOST = 'https://app.box.com/api'; let api; @@ -49,7 +49,7 @@ describe('api/API', () => { api.errorHandler(error); expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.auth, + reason: error.name, error: error.toString() }); }); diff --git a/src/api/__tests__/AnnotationAPI-test.js b/src/api/__tests__/AnnotationAPI-test.js index 6116fc5c1..ff3e543df 100644 --- a/src/api/__tests__/AnnotationAPI-test.js +++ b/src/api/__tests__/AnnotationAPI-test.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-expressions */ import AnnotationAPI from '../AnnotationAPI'; -import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants'; +import { ERROR_TYPE } from '../../constants'; const API_HOST = 'https://app.box.com/api'; const HTTP_POST = 'POST'; @@ -24,6 +24,7 @@ describe('api/AnnotationAPI', () => { token: 'someToken' }); api.axios = jest.fn().mockReturnValue(promise); + api.errorHandler = jest.fn(); api.axiosSource = { token: '123abc' }; api.headers = {}; }); @@ -67,14 +68,11 @@ describe('api/AnnotationAPI', () => { describe('createSuccessHandler()', () => { it('should emit an error event', () => { - api.emit = jest.fn(); const error = new Error('Could not create annotation'); + error.name = ERROR_TYPE.create; api.createSuccessHandler({ type: 'error' }); - expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.create, - error: error.toString() - }); + expect(api.errorHandler).toBeCalledWith(error); }); it('should return the created annotation', () => { @@ -87,14 +85,11 @@ describe('api/AnnotationAPI', () => { describe('deleteSuccessHandler()', () => { it('should emit an error event', () => { - api.emit = jest.fn(); const error = new Error('Could not delete annotation with ID 123'); + error.name === ERROR_TYPE.delete; api.deleteSuccessHandler({ type: 'error' }, '123'); - expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.delete, - error: error.toString() - }); + expect(api.errorHandler).toBeCalledWith(error); }); it('should return the deleted annotation', () => { diff --git a/src/api/__tests__/FileVersionAPI-test.js b/src/api/__tests__/FileVersionAPI-test.js index 2100d82d6..1a834dfd5 100644 --- a/src/api/__tests__/FileVersionAPI-test.js +++ b/src/api/__tests__/FileVersionAPI-test.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-expressions */ import FileVersionAPI from '../FileVersionAPI'; -import { ANNOTATOR_EVENT, ERROR_TYPE } from '../../constants'; +import { ERROR_TYPE } from '../../constants'; const API_HOST = 'https://app.box.com/api'; @@ -55,34 +55,34 @@ describe('api/FileVersionAPI', () => { describe('successHandler()', () => { beforeEach(() => { - api.fetchFromMarker = jest.fn(); + api.fetchFromMarker = jest.fn().mockReturnValue(Promise.resolve()); api.emit = jest.fn(); }); it('should call fetch the remaining annotations if the version has more to fetch', () => { api.data = { entries: [{}] }; - api.successHandler({ entries: [{}, {}], next_marker: 'marker', limit: 1 }); - expect(api.data.entries.length).toEqual(3); - expect(api.fetchFromMarker).toBeCalled(); + api.successHandler({ entries: [{}, {}], next_marker: 'marker', limit: 1 }).then(() => { + expect(api.data.entries.length).toEqual(3); + expect(api.fetchFromMarker).toBeCalled(); + }); }); it('should not call fetch if no more annotations need to be fetched', () => { api.data = { entries: [{}] }; - api.successHandler({ entries: [{}, {}], limit: 1 }); - expect(api.data.entries.length).toEqual(3); - expect(api.fetchFromMarker).not.toBeCalled(); + api.successHandler({ entries: [{}, {}], limit: 1 }).then(() => { + expect(api.data.entries.length).toEqual(3); + expect(api.fetchFromMarker).not.toBeCalled(); + }); }); it('should emit an error if the success response is of type error', () => { api.fileVersionId = 123; - api.successHandler({ type: 'error' }); - const error = new Error(`Could not read annotations from file version with ID ${api.fileVersionId}`); + api.successHandler({ type: 'error' }).catch(() => { + const error = new Error(`Could not read annotations from file version with ID ${api.fileVersionId}`); + error.name = ERROR_TYPE.read; - expect(api.data.entries.length).toEqual(1); - expect(api.fetchFromMarker).not.toBeCalled(); - expect(api.emit).toBeCalledWith(ANNOTATOR_EVENT.error, { - reason: ERROR_TYPE.read, - error: error.toString() + expect(api.data.entries.length).toEqual(1); + expect(api.fetchFromMarker).not.toBeCalled(); }); }); }); diff --git a/src/constants.js b/src/constants.js index a99a9bd3a..f92d0a918 100644 --- a/src/constants.js +++ b/src/constants.js @@ -178,10 +178,10 @@ export const CREATE_EVENT = { }; export const ERROR_TYPE = { - auth: 'authorization', - read: 'read', - create: 'create', - delete: 'delete' + auth: 'AuthorizationError', + read: 'ReadError', + create: 'CreateError', + delete: 'DeleteError' }; export const POINT_ANNOTATION_ICON_HEIGHT = 31;