Skip to content

Commit

Permalink
Chore: log actual error message (#343)
Browse files Browse the repository at this point in the history
* Chore: log actual error message

* Chore: Fix style

* Chore: Responding to comments
  • Loading branch information
Jeremy Press authored Aug 29, 2017
1 parent f9f511e commit f1bc217
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 22 deletions.
6 changes: 1 addition & 5 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -1056,18 +1056,14 @@ class Preview extends EventEmitter {
// Destroy anything still showing
this.destroy();

// Figure out what error message to log and what error message to display
const logMessage = err instanceof Error ? err.message : __('error_default');
const displayMessage = err && err.displayMessage ? err.displayMessage : logMessage;

// Instantiate the error viewer
this.viewer = this.getErrorViewer();

// Add listeners for viewer events
this.attachViewerListeners();

// Load the error viewer
this.viewer.load(displayMessage);
this.viewer.load(err);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1696,11 +1696,12 @@ describe('lib/Preview', () => {
});

it('should get the error viewer, attach viewer listeners, and load the error viewer', () => {
preview.triggerError();
const err = new Error();
preview.triggerError(err);

expect(stubs.getErrorViewer).to.be.called;
expect(stubs.attachViewerListeners).to.be.called;
expect(ErrorViewer.load).to.be.called;
expect(ErrorViewer.load).to.be.calledWith(err);
});
});

Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ class DocBaseViewer extends BaseViewer {
if (err instanceof Error) {
error.displayMessage = __('error_document');
}

this.triggerError(err);
});
}
Expand Down
19 changes: 14 additions & 5 deletions src/lib/viewers/error/PreviewErrorViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,14 @@ class PreviewErrorViewer extends BaseViewer {
/**
* Shows an error message to the user.
*
* @param {string} reason - Error reason
* @param {Error} err - Error
* @return {void}
*/
load(reason) {
load(err) {
this.setup();

const { file, showDownload } = this.options;
let icon = ICON_FILE_DEFAULT;
const message = reason || __('error_default');

// Generic errors will not have the file object
if (file) {
Expand All @@ -77,17 +76,27 @@ class PreviewErrorViewer extends BaseViewer {
}
}

/* eslint-disable no-param-reassign */
err = err instanceof Error ? err : new Error(__('error_default'));
/* eslint-enable no-param-reassign */

// If there is no display message fallback to the message from above
const displayMessage = err.displayMessage || err.message;

this.iconEl.innerHTML = icon;
this.messageEl.textContent = message;
this.messageEl.textContent = displayMessage;

// Add optional download button
if (checkPermission(file, PERMISSION_DOWNLOAD) && showDownload && Browser.canDownload()) {
this.addDownloadButton();
}

this.loaded = true;

// The error will either be the message from the original error, the displayMessage from the orignal error,
// or the default message from the locally created error
this.emit('load', {
error: message
error: err.message || displayMessage
});
}

Expand Down
40 changes: 30 additions & 10 deletions src/lib/viewers/error/__tests__/PreviewErrorViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,13 @@ describe('lib/viewers/error/PreviewErrorViewer', () => {
const extension = testCase[0];
const expectedIcon = testCase[1];

const message = 'reason';
const err = new Error();
err.displayMessage = 'reason';
error.options.file.extension = extension;
error.load(message);
error.load(err);

expect(error.iconEl).to.contain.html(expectedIcon);
expect(error.messageEl.textContent).to.equal(message);
expect(error.messageEl.textContent).to.equal(err.displayMessage);
});
});

Expand Down Expand Up @@ -112,17 +113,36 @@ describe('lib/viewers/error/PreviewErrorViewer', () => {
expect(error.addDownloadButton).to.not.be.called;
});

it('should broadcast load', () => {
it('should broadcast the log message', () => {
sandbox.stub(error, 'emit');

const message = 'reason';
error.load(message);
const err = new Error();
err.displayMessage = 'reason';
err.message = 'this is bad';

error.load(err);

expect(error.emit).to.be.calledWith(
'load', {
error: 'this is bad'
}
);
});

it('should broadcast the display message if there is no log message', () => {
sandbox.stub(error, 'emit');

const err = new Error();
err.displayMessage = 'reason';
err.message = undefined;

error.load(err);


expect(error.emit).to.be.calledWith(
'load',
sinon.match({
error: message
})
'load', {
error: 'reason'
}
);
});
});
Expand Down

0 comments on commit f1bc217

Please sign in to comment.