Skip to content

Commit

Permalink
Chore: Adding download reachability checks to other viewers (#705)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press authored Mar 13, 2018
1 parent 1102077 commit ac73b92
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 36 deletions.
1 change: 1 addition & 0 deletions src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const ERROR_CODE = {
BAD_INPUT: 'error_bad_input',
LOAD_ANNOTATIONS: 'error_load_annotations',
LOAD_ASSET: 'error_load_asset',
LOAD_CSV: 'error_load_csv',
LOAD_DOCUMENT: 'error_load_document',
LOAD_MEDIA: 'error_load_media',
LOAD_VIEWER: 'error_load_viewer',
Expand Down
8 changes: 4 additions & 4 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,16 @@ class BaseViewer extends EventEmitter {
return;
}

this.hasRetriedContentDownload = true;
this.load();

if (isCustomDownloadHost(downloadURL)) {
setDownloadReachability(downloadURL).then((isBlocked) => {
if (isBlocked) {
this.emitMetric(DOWNLOAD_REACHABILITY_METRICS.DOWNLOAD_BLOCKED, getHostnameFromUrl(downloadURL));
}
});
}

this.hasRetriedContentDownload = true;
this.load();
}

/**
Expand Down Expand Up @@ -463,7 +463,7 @@ class BaseViewer extends EventEmitter {
* @return {void}
*/
viewerLoadHandler(event) {
const contentTemplate = getProp(this.options, 'representation.content.url_template', null);
const contentTemplate = getProp(this.options, 'representation.content.url_template', '');
const downloadHostToNotify = downloadNotificationToShow(contentTemplate);
if (downloadHostToNotify) {
this.previewUI.notification.show(
Expand Down
21 changes: 15 additions & 6 deletions src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const MANIFEST = 'manifest.mpd';
const DEFAULT_VIDEO_WIDTH_PX = 854;
const DEFAULT_VIDEO_HEIGHT_PX = 480;

const SHAKA_CODE_ERROR_RECOVERABLE = 1;

class DashViewer extends VideoBaseViewer {
/**
* @inheritdoc
Expand Down Expand Up @@ -169,7 +171,7 @@ class DashViewer extends VideoBaseViewer {
this.player.getNetworkingEngine().registerRequestFilter(this.requestFilter);

this.startLoadTimer();
this.player.load(this.mediaUrl, this.startTimeInSeconds);
this.player.load(this.mediaUrl, this.startTimeInSeconds).catch(this.shakaErrorHandler);
}

/**
Expand Down Expand Up @@ -363,18 +365,25 @@ class DashViewer extends VideoBaseViewer {
* @return {void}
*/
shakaErrorHandler(shakaError) {
const normalizedShakaError = shakaError.detail ? shakaError.detail : shakaError;
const error = new PreviewError(
ERROR_CODE.SHAKA,
__('error_refresh'),
{},
`Shaka error. Code = ${shakaError.detail.code}, Category = ${shakaError.detail.category}, Severity = ${
shakaError.detail.severity
}, Data = ${shakaError.detail.data.toString()}`
`Shaka error. Code = ${normalizedShakaError.code}, Category = ${
normalizedShakaError.category
}, Severity = ${normalizedShakaError.severity}, Data = ${normalizedShakaError.data.toString()}`
);

if (shakaError.detail.severity > 1) {
if (normalizedShakaError.severity > SHAKA_CODE_ERROR_RECOVERABLE) {
// Anything greater than a recoverable error should be critical
if (normalizedShakaError.code === shaka.util.Error.Code.HTTP_ERROR) {
const downloadURL = normalizedShakaError.data[0];
this.handleDownloadError(error, downloadURL);
return;
}
// critical error
this.emit('error', error);
this.triggerError(error);
}
}

Expand Down
13 changes: 10 additions & 3 deletions src/lib/viewers/media/MediaBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,16 @@ class MediaBaseViewer extends BaseViewer {
// eslint-disable-next-line
console.error(err);

// Display a generic error message but log the real one
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA, __('error_refresh'), {}, err.message);
this.emit('error', error);
const errorCode = getProp(err, 'target.error.code');
const errorDetails = errorCode ? { error_code: errorCode } : {};

const error = new PreviewError(ERROR_CODE.LOAD_MEDIA, __('error_refresh'), errorDetails);

if (!this.isLoaded()) {
this.handleDownloadError(error, this.mediaUrl);
} else {
this.triggerError(error);
}
}

/**
Expand Down
34 changes: 32 additions & 2 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ describe('lib/viewers/media/DashViewer', () => {
stubs.mockPlayer.expects('addEventListener').withArgs('adaptation', sinon.match.func);
stubs.mockPlayer.expects('addEventListener').withArgs('error', sinon.match.func);
stubs.mockPlayer.expects('configure');
stubs.mockPlayer.expects('load').withArgs('url');
stubs.mockPlayer.expects('load').withArgs('url').returns(Promise.resolve());;

dash.loadDashPlayer();

Expand All @@ -228,6 +228,8 @@ describe('lib/viewers/media/DashViewer', () => {
it('should invoke startLoadTimer()', () => {
sandbox.stub(dash, 'startLoadTimer');
sandbox.stub(shaka, 'Player').returns(dash.player);
stubs.mockPlayer.expects('load').returns(Promise.resolve());

dash.loadDashPlayer();

expect(dash.startLoadTimer).to.be.called;
Expand All @@ -238,7 +240,7 @@ describe('lib/viewers/media/DashViewer', () => {
dash.mediaUrl = 'url';
dash.startTimeInSeconds = START_TIME_IN_SECONDS;
sandbox.stub(shaka, 'Player').returns(dash.player);
stubs.mockPlayer.expects('load').withArgs('url', START_TIME_IN_SECONDS);
stubs.mockPlayer.expects('load').withArgs('url', START_TIME_IN_SECONDS).returns(Promise.resolve());

dash.loadDashPlayer();
});
Expand Down Expand Up @@ -503,6 +505,34 @@ describe('lib/viewers/media/DashViewer', () => {

expect(dash.emit).to.not.be.called;
});

it('should work when the error does not contain a details object', () => {
const shakaError = {
severity: 2, // critical severity
category: 1,
code: 1100, // HTTP Error code
data: ['foobar']
};
dash.shakaErrorHandler(shakaError);

const [ event, error ] = dash.emit.getCall(0).args;
expect(event).to.equal('error');
expect(error).to.be.instanceof(PreviewError);
expect(error.code).to.equal('error_shaka');
});

it('should handle the download error if an HTTP shaka error is thrown', () => {
const shakaError = {
severity: 2, // critical severity
category: 1,
code: 1002, // hTTP Error code
data: ['foobar']
};
sandbox.stub(dash, 'handleDownloadError')
dash.shakaErrorHandler(shakaError);

expect(dash.handleDownloadError).to.be.called;
});
});

describe('addEventListenersForMediaControls()', () => {
Expand Down
27 changes: 16 additions & 11 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,20 +188,25 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
});

describe('errorHandler()', () => {
it('should emit the error and set a display message', () => {
sandbox.stub(media, 'emit');
const err = new Error('blah');
sandbox
.mock(window.console)
.expects('error')
.withArgs(err);
it('should handle download error if the viewer was not yet loaded', () => {
media.mediaUrl = 'foo'
sandbox.stub(media, 'isLoaded').returns(false);
sandbox.stub(media, 'handleDownloadError');
const err = new Error();

media.errorHandler(err);

expect(media.handleDownloadError).to.be.calledWith(sinon.match.has('code'), 'foo');
});

it('should trigger an error if Preview is already loaded', () => {
sandbox.stub(media, 'isLoaded').returns(true);
sandbox.stub(media, 'triggerError');
const err = new Error();

media.errorHandler(err);

const [event, error] = media.emit.getCall(0).args;
expect(event).to.equal('error');
expect(error).to.be.instanceof(PreviewError);
expect(error.code).to.equal('error_load_media');
expect(media.triggerError).to.be.calledWith(sinon.match.has('code'));
});
});

Expand Down
6 changes: 4 additions & 2 deletions src/lib/viewers/text/CSVViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import TextBaseViewer from './TextBaseViewer';
import { createAssetUrlCreator, get } from '../../util';
import { TEXT_STATIC_ASSETS_VERSION } from '../../constants';
import './CSV.scss';
import { VIEWER_EVENT } from '../../events';
import { ERROR_CODE, VIEWER_EVENT } from '../../events';
import PreviewError from '../../PreviewError';

const JS = [`third-party/text/${TEXT_STATIC_ASSETS_VERSION}/papaparse.min.js`, 'csv.js'];

Expand Down Expand Up @@ -56,7 +57,8 @@ class CSVViewer extends TextBaseViewer {
Papa.parse(urlWithAuth, {
download: true,
error: (err, file, inputElem, reason) => {
this.emit('error', reason);
const error = new PreviewError(ERROR_CODE.LOAD_CSV, __('error_refresh'), { reason });
this.handleDownloadError(error, urlWithAuth);
},
complete: (results) => {
if (this.isDestroyed() || !results) {
Expand Down
8 changes: 6 additions & 2 deletions src/lib/viewers/text/PlainTextViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class PlainTextViewer extends TextBaseViewer {
* Loads a text file.
*
* @private
* @return {void}
* @return {Promise} promise to get text content
*/
postLoad = () => {
const { representation, file } = this.options;
Expand All @@ -192,8 +192,12 @@ class PlainTextViewer extends TextBaseViewer {
this.truncated = size > SIZE_LIMIT_BYTES;
const headers = this.truncated ? { Range: `bytes=0-${SIZE_LIMIT_BYTES}` } : {};

const contentUrl = this.createContentUrlWithAuthParams(template);
this.startLoadTimer();
get(this.createContentUrlWithAuthParams(template), headers, 'text')
return get(contentUrl, headers, 'text')
.catch((error) => {
this.handleDownloadError(error, contentUrl);
})
.then((text) => {
if (this.isDestroyed()) {
return;
Expand Down
24 changes: 18 additions & 6 deletions src/lib/viewers/text/__tests__/PlainTextViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ describe('lib/viewers/text/PlainTextViewer', () => {
const getPromise = Promise.resolve(someText);
sandbox.stub(util, 'get').returns(getPromise);
sandbox.stub(text, 'finishLoading');
text.options.file.size = 196608 + 1; // 192KB + 1
text.options.file.size = 196608 + 1; // 192KB + 1;

text.postLoad();
const promise = text.postLoad();

return getPromise.then(() => {
expect(text.finishLoading).to.be.calledWith(`${someText}...`, false);
return promise.then(() => {
expect(text.finishLoading).to.be.called;
});
});

Expand All @@ -226,9 +226,9 @@ describe('lib/viewers/text/PlainTextViewer', () => {
text.options.file.size = 196608 + 1; // 192KB + 1
text.options.file.extension = 'js'; // code extension

text.postLoad();
const promise = text.postLoad();

return getPromise.then(() => {
return promise.then(() => {
expect(text.initHighlightJs).to.be.calledWith(`${someText}...`);
});
});
Expand All @@ -246,6 +246,18 @@ describe('lib/viewers/text/PlainTextViewer', () => {
expect(text.startLoadTimer).to.be.called;
});
});

it('should handle a download error', () => {
const getPromise = Promise.reject();
sandbox.stub(util, 'get').returns(getPromise);
sandbox.stub(text, 'handleDownloadError');

const promise = text.postLoad();

return promise.catch(() => {
expect(text.handleDownloadError).to.be.called;
});
});
});

describe('initHighlightJs()', () => {
Expand Down

0 comments on commit ac73b92

Please sign in to comment.