From a718718710e32b31772f16cac5775dd4cd561360 Mon Sep 17 00:00:00 2001 From: Jeremy Press Date: Mon, 11 Sep 2017 13:34:51 -0700 Subject: [PATCH] New: More specific error messages from conversion (#378) * New: More specific error messages from conversion --- src/i18n/en-US.properties | 8 +++- src/lib/Preview.js | 2 +- src/lib/RepStatus.js | 38 +++++++++++++++++- src/lib/__tests__/RepStatus-test.js | 40 ++++++++++++++++++- src/lib/viewers/BaseViewer.js | 12 +++--- src/lib/viewers/__tests__/BaseViewer-test.js | 6 +++ .../viewers/box3d/model3d/Model3DRenderer.js | 2 +- 7 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/i18n/en-US.properties b/src/i18n/en-US.properties index a703d81d0..57a54c9d7 100644 --- a/src/i18n/en-US.properties +++ b/src/i18n/en-US.properties @@ -48,12 +48,16 @@ error_rate_limit=We're sorry, the preview didn't load because your request was r error_reupload=We're sorry, the preview didn't load. Please re-upload the file or contact Box support. # Preview error message shown when the user's browser doesn't support previews of this file type error_browser_unsupported=We're sorry, your browser doesn't support preview for {1}. -# Preview error message shown when the file has invalid formatting (most likely a 3D file) -error_file_type_unsupported=We're sorry, this file format is not supported. # Preview error message shown when an iWork file is previewed error_iwork=We're sorry, iWork files are not currently supported. # Preview error message shown when document loading fails (most likely due to password or watermark) error_document=We're sorry, the preview didn't load. This document may be protected. +# Preview error message shown when the document is password protected +error_password_protected=We're sorry the preview didn't load. This document is protected. +# Preview error message shown when conversion was unable to process the file at the given time. +error_try_again_later=We're sorry the preview didn't load. Please try again later. +# Preview error message shown when conversion failed due to file contents +error_bad_file=We're sorry the preivew didn't load. This file may be corrupted. # Media Preview # Label for changing speed in media player diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 08b5ffae2..8ba71a544 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -787,7 +787,7 @@ class Preview extends EventEmitter { // If no loader then throw an unsupported error // If file type specific error message, throw the generic one if (!loader) { - throw new Error(FILE_EXT_ERROR_MAP[this.file.extension] || __('error_file_type_unsupported')); + throw new Error(FILE_EXT_ERROR_MAP[this.file.extension] || __('error_default')); } // Determine the viewer to use diff --git a/src/lib/RepStatus.js b/src/lib/RepStatus.js index 15f221677..31fe03330 100644 --- a/src/lib/RepStatus.js +++ b/src/lib/RepStatus.js @@ -4,13 +4,17 @@ import { STATUS_SUCCESS, STATUS_VIEWABLE } from './constants'; const STATUS_UPDATE_INTERVAL_MS = 2000; +const ERROR_PASSWORD_PROTECTED = 'error_password_protected'; +const ERROR_TRY_AGAIN_LATER = 'error_try_again_later'; +const ERROR_UNSUPPORTED_FORMAT = 'error_unsupported_format'; + class RepStatus extends EventEmitter { /** * Gets the status out of represenation * * @public * @param {Object} representation - representation object - * @return {string} rep status instance + * @return {string} rep status state */ static getStatus(representation) { let { status } = representation; @@ -18,6 +22,19 @@ class RepStatus extends EventEmitter { return status; } + /** + * Gets the error code out of the representation + * + * @public + * @param {Object} representation - representation object + * @return {string} rep error code + */ + static getErrorCode(representation) { + const { status } = representation; + const code = typeof status === 'object' ? status.code : representation.temp_status.code; + return code; + } + /** * [constructor] * @@ -89,9 +106,26 @@ class RepStatus extends EventEmitter { */ handleResponse() { const status = RepStatus.getStatus(this.representation); + let errorCode; + switch (status) { case 'error': - this.reject(); + switch (RepStatus.getErrorCode(this.representation)) { + case ERROR_PASSWORD_PROTECTED: + errorCode = __('error_password_protected'); + break; + case ERROR_TRY_AGAIN_LATER: + errorCode = __('error_try_again_later'); + break; + case ERROR_UNSUPPORTED_FORMAT: + errorCode = __('error_bad_file'); + break; + default: + errorCode = __('error_refresh'); + break; + } + + this.reject(errorCode); break; case STATUS_SUCCESS: diff --git a/src/lib/__tests__/RepStatus-test.js b/src/lib/__tests__/RepStatus-test.js index 4480d63cb..c6c9e987a 100644 --- a/src/lib/__tests__/RepStatus-test.js +++ b/src/lib/__tests__/RepStatus-test.js @@ -48,6 +48,18 @@ describe('lib/RepStatus', () => { }); }); + describe('getErrorCode()', () => { + it('should return the code from the representation state object', () => { + expect( + RepStatus.getErrorCode({ + status: { + code: 'conversion_failed' + } + }) + ).to.equal('conversion_failed'); + }); + }); + describe('constructor()', () => { const infoUrl = 'someUrl'; @@ -131,9 +143,33 @@ describe('lib/RepStatus', () => { repStatus.updateStatus = () => {}; }); - it('should reject if the rep status is error', () => { - sandbox.mock(repStatus).expects('reject'); + it('should reject with the refresh message if the rep status is error', () => { + sandbox.mock(repStatus).expects('reject').withArgs(__('error_refresh')); + repStatus.representation.status.state = 'error'; + + repStatus.handleResponse(); + }); + + it('should reject with the protected message if the rep status is error due to a password protected PDF', () => { + sandbox.mock(repStatus).expects('reject').withArgs(__('error_password_protected')); + repStatus.representation.status.state = 'error'; + repStatus.representation.status.code = 'error_password_protected'; + + repStatus.handleResponse(); + }); + + it('should reject with the try again message if the rep status is error due to unavailability', () => { + sandbox.mock(repStatus).expects('reject').withArgs(__('error_try_again_later')); + repStatus.representation.status.state = 'error'; + repStatus.representation.status.code = 'error_try_again_later'; + + repStatus.handleResponse(); + }); + + it('should reject with the unsupported format message if the rep status is error due a bad file', () => { + sandbox.mock(repStatus).expects('reject').withArgs(__('error_bad_file')); repStatus.representation.status.state = 'error'; + repStatus.representation.status.code = 'error_unsupported_format'; repStatus.handleResponse(); }); diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 4f14c21db..fe045a195 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -227,13 +227,13 @@ class BaseViewer extends EventEmitter { }; /** - * Emits an error when an asset (static or representation) fails to load. + * Triggers an error when an asset (static or representation) fails to load. * - * @emits error + * @param {string} [err] - Optional error message * @return {void} */ - handleAssetError = () => { - this.triggerError(); + handleAssetError = (err) => { + this.triggerError(err); this.destroyed = true; }; @@ -242,11 +242,11 @@ class BaseViewer extends EventEmitter { * * @protected * @emits error - * @param {Error} [err] - Optional error with message + * @param {Error|string} [err] - Optional error or string with message * @return {void} */ triggerError(err) { - this.emit('error', err instanceof Error ? err : new Error(__('error_refresh'))); + this.emit('error', err instanceof Error ? err : new Error(err || __('error_refresh'))); } /** diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index cebeb674a..2908f51f5 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -157,6 +157,12 @@ describe('lib/viewers/BaseViewer', () => { expect(base.triggerError).to.be.called; expect(base.destroyed).to.be.true; }); + + it('should pass along the error if provided', () => { + sandbox.stub(base, 'triggerError'); + base.handleAssetError('error'); + expect(base.triggerError).to.be.calledWith('error'); + }); }); describe('triggerError()', () => { diff --git a/src/lib/viewers/box3d/model3d/Model3DRenderer.js b/src/lib/viewers/box3d/model3d/Model3DRenderer.js index 77f2e613c..b15ed4ad8 100644 --- a/src/lib/viewers/box3d/model3d/Model3DRenderer.js +++ b/src/lib/viewers/box3d/model3d/Model3DRenderer.js @@ -346,7 +346,7 @@ class Model3DRenderer extends Box3DRenderer { * @return {void} */ onUnsupportedRepresentation() { - this.emit('error', new Error(__('error_file_type_unsupported'))); + this.emit('error', new Error(__('error_default'))); } /**