Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Adding console errors for annotation errors #388

Merged
merged 5 commits into from
Sep 13, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/lib/annotations/AnnotationService.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,11 @@ class AnnotationService extends EventEmitter {
}
})
/* 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
});
});
});
Expand Down Expand Up @@ -168,10 +169,11 @@ class AnnotationService extends EventEmitter {
}
})
/* 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
});
});
});
Expand Down Expand Up @@ -303,10 +305,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
});
});
}
Expand Down
29 changes: 25 additions & 4 deletions src/lib/annotations/AnnotationThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand All @@ -171,9 +171,23 @@ 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);
console.error(
THREAD_EVENT.deleteError,
`Annotation with ID ${annotation.threadNumber} could not be found.`
);
return Promise.reject();
}

if (annotation.permissions && !annotation.permissions.can_delete) {
// Broadcast error
this.emit(THREAD_EVENT.deleteError);
console.error(
THREAD_EVENT.deleteError,
`User does not have the correct permissions to delete annotation with ID ${annotation.threadNumber}.`
);
return Promise.reject();
}

Expand Down Expand Up @@ -203,6 +217,10 @@ class AnnotationThread extends EventEmitter {
}

if (!useServer) {
console.error(
THREAD_EVENT.deleteError,
`Annotation with ID ${annotation.threadNumber} not deleted from server`
);
return Promise.resolve();
}

Expand All @@ -228,9 +246,10 @@ class AnnotationThread extends EventEmitter {
// Broadcast annotation deletion event
this.emit(THREAD_EVENT.delete);
})
.catch(() => {
.catch((error) => {
// Broadcast error
this.emit(THREAD_EVENT.deleteError);
console.error(THREAD_EVENT.deleteError, error.toString());
});
}

Expand Down Expand Up @@ -588,15 +607,17 @@ 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);
console.error(THREAD_EVENT.createError, error.toString());
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ class Annotator extends EventEmitter {
default:
}

if (data.error) {
console.error(ANNOTATOR_EVENT.error, data.error.toString());
}

if (errorMessage) {
this.emit(ANNOTATOR_EVENT.error, errorMessage);
}
Expand Down Expand Up @@ -935,6 +939,7 @@ class Annotator extends EventEmitter {
}

this.emit(ANNOTATOR_EVENT.error, __('annotations_load_error'));
console.error('Annotation could not be created due to invalid params');
this.validationErrorEmitted = true;
}

Expand Down
4 changes: 1 addition & 3 deletions src/lib/annotations/doc/DocAnnotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ class DocAnnotator extends Annotator {
const highlightThreads = this.getHighlightThreadsOnPage(pageNum);
highlightThreads.forEach((thread) => {
if (annotatorUtil.isPending(thread.state)) {
console.error('Pending annotation thread destroyed', thread.threadNumber);
thread.destroy();
}
});
Expand Down Expand Up @@ -861,20 +862,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;
Expand Down
4 changes: 3 additions & 1 deletion src/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,9 @@ class BaseViewer extends EventEmitter {
}

if (this.annotationsPromise) {
this.annotationsPromise.then(this.loadAnnotator);
this.annotationsPromise.then(this.loadAnnotator).catch((error) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline the catch()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our linter puts it back in one line 😛

console.error('Annotation assets failed to load', error.toString());
});
}
});
}
Expand Down