From 3762eadaab613989e065d5c01145d1ed0cf03a50 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Tue, 12 Sep 2017 17:03:03 -0700 Subject: [PATCH] Chore: Adding console errors for annotation errors (#388) --- src/lib/annotations/AnnotationService.js | 33 +++++++++------ src/lib/annotations/AnnotationThread.js | 40 +++++++++++++++++-- src/lib/annotations/Annotator.js | 9 +++++ .../__tests__/AnnotationService-test.js | 12 ++++-- .../__tests__/AnnotationThread-test.js | 2 +- src/lib/annotations/doc/DocAnnotator.js | 6 +-- src/lib/util.js | 4 +- src/lib/viewers/BaseViewer.js | 6 ++- 8 files changed, 86 insertions(+), 26 deletions(-) diff --git a/src/lib/annotations/AnnotationService.js b/src/lib/annotations/AnnotationService.js index 74091bf04..f3be99709 100644 --- a/src/lib/annotations/AnnotationService.js +++ b/src/lib/annotations/AnnotationService.js @@ -110,17 +110,20 @@ class AnnotationService extends EventEmitter { resolve(createdAnnotation); } else { - reject(new Error('Could not create annotation')); + const error = new Error('Could not create annotation'); + reject(error); this.emit('annotationerror', { - reason: 'create' + reason: 'create', + error: error.toString() }); } }) /* istanbul ignore next */ - .catch(() => { + .catch((error) => { reject(new Error('Could not create annotation due to invalid or expired token')); this.emit('annotationerror', { - reason: 'authorization' + reason: 'authorization', + error: error.toString() }); }); }); @@ -161,17 +164,20 @@ class AnnotationService extends EventEmitter { if (response.status === 204) { resolve(); } else { - reject(new Error(`Could not delete annotation with ID ${annotationID}`)); + const error = new Error(`Could not delete annotation with ID ${annotationID}`); + reject(error); this.emit('annotationerror', { - reason: 'delete' + reason: 'delete', + error: error.toString() }); } }) /* istanbul ignore next */ - .catch(() => { + .catch((error) => { reject(new Error('Could not delete annotation due to invalid or expired token')); this.emit('annotationerror', { - reason: 'authorization' + reason: 'authorization', + error: error.toString() }); }); }); @@ -287,9 +293,11 @@ class AnnotationService extends EventEmitter { .then((response) => response.json()) .then((data) => { if (data.type === 'error' || !Array.isArray(data.entries)) { - reject(new Error(`Could not read annotations from file version with ID ${fileVersionId}`)); + const error = new Error(`Could not read annotations from file version with ID ${fileVersionId}`); + reject(error); this.emit('annotationerror', { - reason: 'read' + reason: 'read', + error: error.toString() }); } else { data.entries.forEach((annotationData) => { @@ -303,10 +311,11 @@ class AnnotationService extends EventEmitter { } } }) - .catch(() => { + .catch((error) => { reject(new Error('Could not read annotations from file due to invalid or expired token')); this.emit('annotationerror', { - reason: 'authorization' + reason: 'authorization', + error: error.toString() }); }); } diff --git a/src/lib/annotations/AnnotationThread.js b/src/lib/annotations/AnnotationThread.js index 497a674d4..83a4b33d2 100644 --- a/src/lib/annotations/AnnotationThread.js +++ b/src/lib/annotations/AnnotationThread.js @@ -158,7 +158,7 @@ class AnnotationThread extends EventEmitter { return this.annotationService .create(annotationData) .then((savedAnnotation) => this.updateTemporaryAnnotation(tempAnnotation, savedAnnotation)) - .catch(() => this.handleThreadSaveError(tempAnnotationID)); + .catch((error) => this.handleThreadSaveError(error, tempAnnotationID)); } /** @@ -171,9 +171,27 @@ class AnnotationThread extends EventEmitter { deleteAnnotation(annotationID, useServer = true) { // Ignore if no corresponding annotation exists in thread or user doesn't have permissions const annotation = this.annotations.find((annot) => annot.annotationID === annotationID); - if (!annotation || (annotation.permissions && !annotation.permissions.can_delete)) { + if (!annotation) { // Broadcast error this.emit(THREAD_EVENT.deleteError); + /* eslint-disable no-console */ + console.error( + THREAD_EVENT.deleteError, + `Annotation with ID ${annotation.threadNumber} could not be found.` + ); + /* eslint-enable no-console */ + return Promise.reject(); + } + + if (annotation.permissions && !annotation.permissions.can_delete) { + // Broadcast error + this.emit(THREAD_EVENT.deleteError); + /* eslint-disable no-console */ + console.error( + THREAD_EVENT.deleteError, + `User does not have the correct permissions to delete annotation with ID ${annotation.threadNumber}.` + ); + /* eslint-enable no-console */ return Promise.reject(); } @@ -203,6 +221,12 @@ class AnnotationThread extends EventEmitter { } if (!useServer) { + /* eslint-disable no-console */ + console.error( + THREAD_EVENT.deleteError, + `Annotation with ID ${annotation.threadNumber} not deleted from server` + ); + /* eslint-enable no-console */ return Promise.resolve(); } @@ -228,9 +252,12 @@ class AnnotationThread extends EventEmitter { // Broadcast annotation deletion event this.emit(THREAD_EVENT.delete); }) - .catch(() => { + .catch((error) => { // Broadcast error this.emit(THREAD_EVENT.deleteError); + /* eslint-disable no-console */ + console.error(THREAD_EVENT.deleteError, error.toString()); + /* eslint-enable no-console */ }); } @@ -588,15 +615,20 @@ class AnnotationThread extends EventEmitter { * Deletes the temporary annotation if the annotation failed to save on the server * * @private + * @param {error} error - error thrown while saving the annotation * @param {string} tempAnnotationID - ID of temporary annotation to be updated with annotation from server * @return {void} */ - handleThreadSaveError(tempAnnotationID) { + handleThreadSaveError(error, tempAnnotationID) { // Remove temporary annotation this.deleteAnnotation(tempAnnotationID, /* useServer */ false); // Broadcast error this.emit(THREAD_EVENT.createError); + + /* eslint-disable no-console */ + console.error(THREAD_EVENT.createError, error.toString()); + /* eslint-enable no-console */ } /** diff --git a/src/lib/annotations/Annotator.js b/src/lib/annotations/Annotator.js index 4e2408dfe..19adada8c 100644 --- a/src/lib/annotations/Annotator.js +++ b/src/lib/annotations/Annotator.js @@ -652,6 +652,12 @@ class Annotator extends EventEmitter { default: } + if (data.error) { + /* eslint-disable no-console */ + console.error(ANNOTATOR_EVENT.error, data.error); + /* eslint-enable no-console */ + } + if (errorMessage) { this.emit(ANNOTATOR_EVENT.error, errorMessage); } @@ -935,6 +941,9 @@ class Annotator extends EventEmitter { } this.emit(ANNOTATOR_EVENT.error, __('annotations_load_error')); + /* eslint-disable no-console */ + console.error('Annotation could not be created due to invalid params'); + /* eslint-enable no-console */ this.validationErrorEmitted = true; } diff --git a/src/lib/annotations/__tests__/AnnotationService-test.js b/src/lib/annotations/__tests__/AnnotationService-test.js index 4bab59f92..e7e7199fa 100644 --- a/src/lib/annotations/__tests__/AnnotationService-test.js +++ b/src/lib/annotations/__tests__/AnnotationService-test.js @@ -94,7 +94,8 @@ describe('lib/annotations/AnnotationService', () => { (error) => { expect(error.message).to.equal('Could not create annotation'); expect(emitStub).to.be.calledWith('annotationerror', { - reason: 'create' + reason: 'create', + error: sinon.match.string }); } ); @@ -214,7 +215,8 @@ describe('lib/annotations/AnnotationService', () => { (error) => { expect(error.message).to.equal('Could not delete annotation with ID 3'); expect(emitStub).to.be.calledWith('annotationerror', { - reason: 'delete' + reason: 'delete', + error: sinon.match.string }); } ); @@ -415,7 +417,8 @@ describe('lib/annotations/AnnotationService', () => { (error) => { expect(error.message).to.equal('Could not read annotations from file version with ID 2'); expect(emitStub).to.be.calledWith('annotationerror', { - reason: 'read' + reason: 'read', + error: sinon.match.string }); } ); @@ -439,7 +442,8 @@ describe('lib/annotations/AnnotationService', () => { return promise.catch((error) => { expect(error.message).to.equal('Could not read annotations from file due to invalid or expired token'); expect(emitStub).to.be.calledWith('annotationerror', { - reason: 'authorization' + reason: 'authorization', + error: sinon.match.string }); }); }); diff --git a/src/lib/annotations/__tests__/AnnotationThread-test.js b/src/lib/annotations/__tests__/AnnotationThread-test.js index 805ae171b..86f4c5098 100644 --- a/src/lib/annotations/__tests__/AnnotationThread-test.js +++ b/src/lib/annotations/__tests__/AnnotationThread-test.js @@ -773,7 +773,7 @@ describe('lib/annotations/AnnotationThread', () => { describe('handleThreadSaveError()', () => { it('should delete temp annotation and emit event', () => { sandbox.stub(thread, 'deleteAnnotation'); - thread.handleThreadSaveError(1); + thread.handleThreadSaveError(new Error(), 1); expect(thread.deleteAnnotation).to.be.calledWith(1, false); expect(thread.emit).to.be.calledWith(THREAD_EVENT.createError); }); diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js index d631e7798..f5cf659de 100644 --- a/src/lib/annotations/doc/DocAnnotator.js +++ b/src/lib/annotations/doc/DocAnnotator.js @@ -463,6 +463,9 @@ class DocAnnotator extends Annotator { const highlightThreads = this.getHighlightThreadsOnPage(pageNum); highlightThreads.forEach((thread) => { if (annotatorUtil.isPending(thread.state)) { + /* eslint-disable no-console */ + console.error('Pending annotation thread destroyed', thread.threadNumber); + /* eslint-enable no-console */ thread.destroy(); } }); @@ -861,20 +864,17 @@ class DocAnnotator extends Annotator { // Select page of first node selected const { pageEl } = annotatorUtil.getPageInfo(selection.anchorNode); - if (!pageEl) { return; } const lastRange = selection.getRangeAt(selection.rangeCount - 1); const rects = lastRange.getClientRects(); - if (rects.length === 0) { return; } const { right, bottom } = rects[rects.length - 1]; - const pageDimensions = pageEl.getBoundingClientRect(); const pageLeft = pageDimensions.left; const pageTop = pageDimensions.top + PAGE_PADDING_TOP; diff --git a/src/lib/util.js b/src/lib/util.js index 64dc3e492..4e3207702 100644 --- a/src/lib/util.js +++ b/src/lib/util.js @@ -155,7 +155,9 @@ export function get(url, ...rest) { break; } - return fetch(url, { headers }).then(checkStatus).then(parser); + return fetch(url, { headers }) + .then(checkStatus) + .then(parser); } /** diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 8a565b740..76b5d4f12 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -346,7 +346,11 @@ class BaseViewer extends EventEmitter { } if (this.annotationsPromise) { - this.annotationsPromise.then(this.loadAnnotator); + this.annotationsPromise.then(this.loadAnnotator).catch((error) => { + /* eslint-disable no-console */ + console.error('Annotation assets failed to load', error.toString()); + /* eslint-enable no-console */ + }); } }); }