Skip to content

Commit

Permalink
New: More specific error messages from conversion (#378)
Browse files Browse the repository at this point in the history
* New: More specific error messages from conversion
  • Loading branch information
Jeremy Press authored Sep 11, 2017
1 parent bfa50e5 commit a718718
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 14 deletions.
8 changes: 6 additions & 2 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 36 additions & 2 deletions src/lib/RepStatus.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,37 @@ 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;
status = typeof status === 'object' ? status.state : representation.temp_status.state;
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]
*
Expand Down Expand Up @@ -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:
Expand Down
40 changes: 38 additions & 2 deletions src/lib/__tests__/RepStatus-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
});
Expand Down
12 changes: 6 additions & 6 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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')));
}

/**
Expand Down
6 changes: 6 additions & 0 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/box3d/model3d/Model3DRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')));
}

/**
Expand Down

0 comments on commit a718718

Please sign in to comment.