Skip to content

Commit

Permalink
fix: check tech error before pause loaders (#969)
Browse files Browse the repository at this point in the history
A failed poster image may trigger an error event from the tech. This shouldn't cause us to pause our loaders, though.
Instead, we should verify that there's an actual error object on the tech before proceeding with pausing the loaders.
  • Loading branch information
brandonocasey authored and gkatsev committed Oct 2, 2020
1 parent a05d032 commit 0c7b2cb
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/videojs-http-streaming.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,9 @@ class VhsHandler extends Component {
});

this.on(this.tech_, 'error', function() {
if (this.masterPlaylistController_) {
// verify that the error was real and we are loaded
// enough to have mpc loaded.
if (this.tech_.error() && this.masterPlaylistController_) {
this.masterPlaylistController_.pauseLoading();
}
});
Expand Down
4 changes: 4 additions & 0 deletions test/master-playlist-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3228,9 +3228,13 @@ QUnit.test('pauses subtitle segment loader on tech errors', function(assert) {

masterPlaylistController.subtitleSegmentLoader_.pause = () => pauseCount++;

this.player.tech_.error = () => 'foo';
this.player.tech_.trigger('error');

assert.equal(pauseCount, 1, 'paused subtitle segment loader');

assert.equal(this.env.log.error.calls, 1, '1 media error logged');
this.env.log.error.reset();
});

QUnit.test('disposes subtitle loaders on dispose', function(assert) {
Expand Down
30 changes: 30 additions & 0 deletions test/videojs-http-streaming.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,36 @@ QUnit.test('the VhsHandler instance is referenced by player.vhs', function(asser
assert.equal(this.env.log.warn.calls, 1, 'warning logged');
});

QUnit.test('tech error may pause loading', function(assert) {
this.player.src({
src: 'manifest/playlist.m3u8',
type: 'application/vnd.apple.mpegurl'
});
this.clock.tick(1);

const vhs = this.player.tech_.vhs;
const mpc = vhs.masterPlaylistController_;
let pauseCalled = false;

mpc.pauseLoading = () => {
pauseCalled = true;
};

this.player.tech_.error = () => null;
this.player.tech_.trigger('error');

assert.notOk(pauseCalled, 'no video el error attribute, no pause loading');

this.player.tech_.error = () => 'foo';
this.player.tech_.trigger('error');

assert.ok(pauseCalled, 'video el error and trigger pauses loading');

assert.equal(this.env.log.error.calls, 1, '1 media error logged');
this.env.log.error.reset();

});

QUnit.test('a deprecation notice is shown when using player.dash', function(assert) {
this.player.src({
src: 'manifest/playlist.m3u8',
Expand Down

0 comments on commit 0c7b2cb

Please sign in to comment.