Skip to content

Commit

Permalink
fix(mediaviewer): address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Mingze Xiao committed Oct 28, 2019
1 parent c92b700 commit 547da31
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 63 deletions.
1 change: 1 addition & 0 deletions src/lib/events.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const ERROR_CODE = {
PREFETCH_FILE: 'error_prefetch_file',
RATE_LIMIT: 'error_rate_limit',
SHAKA: 'error_shaka',
TOKEN_NOT_VALID: 'error_token_function_not_valid',
UNSUPPORTED_FILE_TYPE: 'error_unsupported_file_type',
VIEWER_LOAD_TIMEOUT: 'error_viewer_load_timeout',
};
Expand Down
35 changes: 3 additions & 32 deletions src/lib/viewers/media/DashViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const MAX_BUFFER = SEGMENT_SIZE * 12; // 60 sec
const MANIFEST = 'manifest.mpd';
const DEFAULT_VIDEO_WIDTH_PX = 854;
const DEFAULT_VIDEO_HEIGHT_PX = 480;
const MAX_RETRY_TOKEN = 3; // number of times to retry refreshing token for unauthorized error

const SHAKA_CODE_ERROR_RECOVERABLE = 1;

Expand Down Expand Up @@ -505,32 +504,6 @@ class DashViewer extends VideoBaseViewer {
}
};

/**
* Handle expired token error
*
* @private
* @param {PreviewError} error
* @return {?PreviewError}
*/
handleExpiredTokenError = error => {
if (this.isExpiredTokenError(error)) {
if (this.retryTokenCount >= MAX_RETRY_TOKEN) {
error.details.error_message = 'Reach refreshing token limit for unauthorized error.';
} else {
this.options
.refreshToken()
.then(this.restartPlayback)
.catch(tokenError => {
error.details.error_message = tokenError.message;
this.triggerError(error);
});
this.retryTokenCount += 1;
return undefined;
}
}
return error;
};

/**
* Handles errors thrown by shaka player. See https://shaka-player-demo.appspot.com/docs/api/shaka.util.Error.html
*
Expand All @@ -540,7 +513,7 @@ class DashViewer extends VideoBaseViewer {
*/
shakaErrorHandler(shakaError) {
const normalizedShakaError = shakaError.detail ? shakaError.detail : shakaError;
let error = new PreviewError(
const error = new PreviewError(
ERROR_CODE.SHAKA,
__('error_refresh'),
{
Expand All @@ -561,12 +534,10 @@ class DashViewer extends VideoBaseViewer {
return;
}

error = this.handleExpiredTokenError(error);
this.handleExpiredTokenError(error);

// critical error
if (error) {
this.triggerError(error);
}
this.triggerError(error);
}
}

Expand Down
40 changes: 24 additions & 16 deletions src/lib/viewers/media/MediaBaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,25 +310,34 @@ class MediaBaseViewer extends BaseViewer {
*
* @protected
* @param {PreviewError} error
* @return {?PreviewError}
* @return {void}
*/
handleExpiredTokenError = error => {
if (this.isExpiredTokenError(error)) {
if (this.retryTokenCount >= MAX_RETRY_TOKEN) {
error.details.error_message += ' Reach refreshing token limit for unauthorized error.';
const tokenError = new PreviewError(
ERROR_CODE.TOKEN_NOT_VALID,
null,
{ silent: true },
'Reach refreshing token limit for unauthorized error.',
);
this.triggerError(tokenError);
} else {
this.options
.refreshToken()
.then(this.restartPlayback)
.catch(tokenError => {
error.details.error_message += ` ${tokenError.message}`;
this.triggerError(error);
.catch(e => {
const tokenError = new PreviewError(
ERROR_CODE.TOKEN_NOT_VALID,
null,
{ silent: true },
e.message,
);
this.triggerError(tokenError);
});
this.retryTokenCount += 1;
return undefined;
}
}
return error;
};

/**
Expand All @@ -346,15 +355,14 @@ class MediaBaseViewer extends BaseViewer {
const errorCode = getProp(err, 'target.error.code');
const errorMessage = getProp(err, 'target.error.message');
const errorDetails = errorCode ? { error_code: errorCode, error_message: errorMessage } : {};
const error = this.handleExpiredTokenError(
new PreviewError(ERROR_CODE.LOAD_MEDIA, __('error_refresh'), errorDetails),
);
if (error) {
if (!this.isLoaded()) {
this.handleDownloadError(error, this.mediaUrl);
} else {
this.triggerError(error);
}
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA, __('error_refresh'), errorDetails);

this.handleExpiredTokenError(error);

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

Expand Down
24 changes: 9 additions & 15 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,23 +189,21 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
});

describe('handleExpiredTokenError()', () => {
it('should return the same error if is not an ExpiredTokenError', () => {
it('should not trigger error if is not an ExpiredTokenError', () => {
sandbox.stub(media, 'isExpiredTokenError').returns(false);
sandbox.stub(media, 'triggerError');
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA);
expect(media.handleExpiredTokenError(error)).to.equal(error);
media.handleExpiredTokenError(error);
expect(media.triggerError).to.not.be.called;
});

it('should add error message if retry token count reaches max retry limit', () => {
it('should trigger error if retry token count reaches max retry limit', () => {
media.retryTokenCount = MAX_RETRY_TOKEN + 1;
sandbox.stub(media, 'isExpiredTokenError').returns(true);
const originalError = 'Original error.';
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA, 'error_refresh', {
error_message: originalError,
});
const newError = media.handleExpiredTokenError(error);
expect(newError.details.error_message).to.equal(
`${originalError} Reach refreshing token limit for unauthorized error.`,
);
sandbox.stub(media, 'triggerError');
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA);
media.handleExpiredTokenError(error);
expect(media.triggerError).to.be.calledWith(sinon.match.has('code', ERROR_CODE.TOKEN_NOT_VALID));
});

it('should call refreshToken if retry token count did not reach max retry limit', () => {
Expand All @@ -223,11 +221,9 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
describe('errorHandler()', () => {
it('should handle download error if the viewer was not yet loaded', () => {
const err = new Error();
const error = new PreviewError(ERROR_CODE.LOAD_MEDIA);
media.mediaUrl = 'foo';
sandbox.stub(media, 'isLoaded').returns(false);
sandbox.stub(media, 'handleDownloadError');
sandbox.stub(media, 'handleExpiredTokenError').returns(error);

media.errorHandler(err);

Expand All @@ -236,10 +232,8 @@ describe('lib/viewers/media/MediaBaseViewer', () => {

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

media.errorHandler(err);

Expand Down

0 comments on commit 547da31

Please sign in to comment.