Skip to content

Commit

Permalink
Clean up uncaught Promise rejections
Browse files Browse the repository at this point in the history
 - Translate uncaught Promise rejections into test failures
   (Chrome only until the event is more widely implemented)

 - Clean up uncaught Promise rejection caused by exceptions thrown
   after destroy() in:
   - CastProxy
   - CastReceiver
   - NetworkingEngine
   - StreamingEngine

 - Clean up uncaught Promise rejection caused by test cases in:
   - CancelableChain unit tests
   - DrmEngine unit tests
   - StreamingEngine unit and integration tests
   - Player unit and integration tests

 - Speed up rejection in NetworkingEngine when we should not retry

 - Add --delay-tests to test.py, to aid in debugging uncaught
   Promise rejections and other types of async test pollution

Closes shaka-project#1323

Change-Id: I5a8f5702a22430929babeb071bf6650c52c5ad17
  • Loading branch information
joeyparrish committed Feb 28, 2018
1 parent e5779be commit f48f7fd
Show file tree
Hide file tree
Showing 15 changed files with 175 additions and 48 deletions.
19 changes: 19 additions & 0 deletions build/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def __init__(self, description):
'Pre-Launch',
'These commands are handled before the tests start running.')


running_commands.add_argument(
'--browsers',
help='Specify which browsers to run tests on as a space-separated or '
Expand Down Expand Up @@ -202,6 +203,19 @@ def __init__(self, description):
'This defaults to one minute.',
type=int,
default=60000)
running_commands.add_argument(
'--delay-tests',
help='Insert an artifical delay between tests [s]. '
'This can be helpful when tracking down asynchronous test '
'pollution, in which an async process belonging to one test may '
'trigger a failure after other tests have begun. '
'If no delay is specified, defaults to 2 seconds.',
type=int,
default=None,
const=2,
nargs='?')


logging_commands.add_argument(
'--colors',
help='Use colors when reporting and printing logs.',
Expand Down Expand Up @@ -244,6 +258,8 @@ def __init__(self, description):
'--report-slower-than',
help='Report tests that are slower than the given time [ms].',
type=int)


networking_commands.add_argument(
'--port',
help='Port where the server is running.',
Expand All @@ -253,6 +269,8 @@ def __init__(self, description):
help='Specify the hostname to be used when capturing browsers. This '
'defaults to localhost.',
default='localhost')


pre_launch_commands.add_argument(
'--force',
help='Force a rebuild of the project before running tests. This will '
Expand Down Expand Up @@ -299,6 +317,7 @@ def ParseArguments(self, args):
'seed',
'single_run',
'uncompiled',
'delay_tests',
]

# Check each value before setting it to avoid passing null values.
Expand Down
6 changes: 5 additions & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@ module.exports = function(config) {
specFilter: settings.filter,

// Set what level of logs for the player to print.
logLevel: SHAKA_LOG_MAP[settings.logging]
logLevel: SHAKA_LOG_MAP[settings.logging],

// Delay tests to aid in debugging async failures that pollute
// subsequent tests.
delayTests: settings.delay_tests,
}],
},

Expand Down
27 changes: 18 additions & 9 deletions lib/cast/cast_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ shaka.cast.CastProxy.prototype.cast = function() {
// TODO: transfer side-loaded text tracks?

return this.sender_.cast(initState).then(function() {
if (!this.localPlayer_) {
// We've already been destroyed.
return;
}

// Unload the local manifest when casting succeeds.
return this.localPlayer_.unload();
}.bind(this));
Expand Down Expand Up @@ -374,13 +379,6 @@ shaka.cast.CastProxy.prototype.onResumeLocal_ = function() {
// Don't autoplay the content until we finish setting up initial state.
this.localVideo_.autoplay = false;
manifestReady = this.localPlayer_.load(manifestUri, startTime);
// Pass any errors through to the app.
manifestReady.catch(function(error) {
goog.asserts.assert(error instanceof shaka.util.Error,
'Wrong error type!');
let event = new shaka.util.FakeEvent('error', {'detail': error});
this.localPlayer_.dispatchEvent(event);
}.bind(this));
}

// Get the video state into a temp variable since we will apply it async.
Expand All @@ -390,7 +388,12 @@ shaka.cast.CastProxy.prototype.onResumeLocal_ = function() {
}.bind(this));

// Finally, take on video state and player's "after load" state.
manifestReady.then(function() {
manifestReady.then(() => {
if (!this.localVideo_) {
// We've already been destroyed.
return;
}

shaka.cast.CastUtils.VideoInitStateAttributes.forEach(function(name) {
this.localVideo_[name] = videoState[name];
}.bind(this));
Expand All @@ -408,7 +411,13 @@ shaka.cast.CastProxy.prototype.onResumeLocal_ = function() {
// Resume playback with transferred state.
this.localVideo_.play();
}
}.bind(this));
}, (error) => {
// Pass any errors through to the app.
goog.asserts.assert(error instanceof shaka.util.Error,
'Wrong error type!');
let event = new shaka.util.FakeEvent('error', {'detail': error});
this.localPlayer_.dispatchEvent(event);
});
};


Expand Down
72 changes: 53 additions & 19 deletions lib/cast/cast_receiver.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ goog.require('goog.asserts');
goog.require('shaka.cast.CastUtils');
goog.require('shaka.log');
goog.require('shaka.util.Error');
goog.require('shaka.util.EventManager');
goog.require('shaka.util.FakeEvent');
goog.require('shaka.util.FakeEventTarget');
goog.require('shaka.util.IDestroyable');
Expand Down Expand Up @@ -54,6 +55,9 @@ shaka.cast.CastReceiver =
/** @private {shaka.Player} */
this.player_ = player;

/** @private {shaka.util.EventManager} */
this.eventManager_ = new shaka.util.EventManager();

/** @private {Object} */
this.targets_ = {
'video': video,
Expand Down Expand Up @@ -124,14 +128,18 @@ shaka.cast.CastReceiver.prototype.isIdle = function() {
* @export
*/
shaka.cast.CastReceiver.prototype.destroy = function() {
let p = this.player_ ? this.player_.destroy() : Promise.resolve();
const async = [
this.eventManager_ ? this.eventManager_.destroy() : null,
this.player_ ? this.player_.destroy() : null,
];

if (this.pollTimerId_ != null) {
window.clearTimeout(this.pollTimerId_);
}

this.video_ = null;
this.player_ = null;
this.eventManager_ = null;
this.targets_ = null;
this.appDataCallback_ = null;
this.isConnected_ = false;
Expand All @@ -140,7 +148,7 @@ shaka.cast.CastReceiver.prototype.destroy = function() {
this.genericBus_ = null;
this.pollTimerId_ = null;

return p.then(function() {
return Promise.all(async).then(function() {
let manager = cast.receiver.CastReceiverManager.getInstance();
manager.stop();
});
Expand Down Expand Up @@ -177,11 +185,13 @@ shaka.cast.CastReceiver.prototype.init_ = function() {
}

shaka.cast.CastUtils.VideoEvents.forEach(function(name) {
this.video_.addEventListener(name, this.proxyEvent_.bind(this, 'video'));
this.eventManager_.listen(
this.video_, name, this.proxyEvent_.bind(this, 'video'));
}.bind(this));

shaka.cast.CastUtils.PlayerEvents.forEach(function(name) {
this.player_.addEventListener(name, this.proxyEvent_.bind(this, 'player'));
this.eventManager_.listen(
this.player_, name, this.proxyEvent_.bind(this, 'player'));
}.bind(this));

// In our tests, the original Chromecast seems to have trouble decoding above
Expand All @@ -200,32 +210,32 @@ shaka.cast.CastReceiver.prototype.init_ = function() {

// Do not start excluding values from update messages until the video is
// fully loaded.
this.video_.addEventListener('loadeddata', function() {
this.eventManager_.listen(this.video_, 'loadeddata', function() {
this.startUpdatingUpdateNumber_ = true;
}.bind(this));

// Maintain idle state.
this.player_.addEventListener('loading', function() {
this.eventManager_.listen(this.player_, 'loading', function() {
// No longer idle once loading. This allows us to show the spinner during
// the initial buffering phase.
this.isIdle_ = false;
this.onCastStatusChanged_();
}.bind(this));
this.video_.addEventListener('playing', function() {
this.eventManager_.listen(this.video_, 'playing', function() {
// No longer idle once playing. This allows us to replay a video without
// reloading.
this.isIdle_ = false;
this.onCastStatusChanged_();
}.bind(this));
this.video_.addEventListener('pause', function() {
this.eventManager_.listen(this.video_, 'pause', function() {
this.onCastStatusChanged_();
}.bind(this));
this.player_.addEventListener('unloading', function() {
this.eventManager_.listen(this.player_, 'unloading', function() {
// Go idle when unloading content.
this.isIdle_ = true;
this.onCastStatusChanged_();
}.bind(this));
this.video_.addEventListener('ended', function() {
this.eventManager_.listen(this.video_, 'ended', function() {
// Go idle 5 seconds after 'ended', assuming we haven't started again or
// been destroyed.
window.setTimeout(function() {
Expand Down Expand Up @@ -264,6 +274,11 @@ shaka.cast.CastReceiver.prototype.onCastStatusChanged_ = function() {
// Player calling unload() as part of load()) are coalesced before the event
// goes out.
Promise.resolve().then(function() {
if (!this.player_) {
// We've already been destroyed.
return;
}

let event = new shaka.util.FakeEvent('caststatuschanged');
this.dispatchEvent(event);
// Send a media status message, with a media info message if appropriate.
Expand Down Expand Up @@ -300,17 +315,15 @@ shaka.cast.CastReceiver.prototype.initState_ = function(initState, appData) {
this.video_.autoplay = false;
manifestReady = this.player_.load(
initState['manifest'], initState['startTime']);
// Pass any errors through to the app.
manifestReady.catch(function(error) {
goog.asserts.assert(error instanceof shaka.util.Error,
'Wrong error type!');
let event = new shaka.util.FakeEvent('error', {'detail': error});
this.player_.dispatchEvent(event);
}.bind(this));
}

// Finally, take on video state and player's "after load" state.
manifestReady.then(function() {
manifestReady.then(() => {
if (!this.player_) {
// We've already been destroyed.
return;
}

for (let k in initState['video']) {
let v = initState['video'][k];
this.video_[k] = v;
Expand All @@ -330,7 +343,13 @@ shaka.cast.CastReceiver.prototype.initState_ = function(initState, appData) {
// Notify generic controllers of the state change.
this.sendMediaStatus_(0);
}
}.bind(this));
}, (error) => {
// Pass any errors through to the app.
goog.asserts.assert(error instanceof shaka.util.Error,
'Wrong error type!');
let event = new shaka.util.FakeEvent('error', {'detail': error});
this.player_.dispatchEvent(event);
});
};


Expand Down Expand Up @@ -616,6 +635,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
}
case 'STOP':
this.player_.unload().then(function() {
if (!this.player_) {
// We've already been destroyed.
return;
}

this.sendMediaStatus_(0);
}.bind(this));
break;
Expand Down Expand Up @@ -661,6 +685,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
this.video_.autoplay = true;
}
this.player_.load(manifestUri, currentTime).then(function() {
if (!this.player_) {
// We've already been destroyed.
return;
}

// Notify generic controllers that the media has changed.
this.sendMediaInfoMessage_();
}.bind(this)).catch(function(error) {
Expand Down Expand Up @@ -702,6 +731,11 @@ shaka.cast.CastReceiver.prototype.onGenericMessage_ = function(event) {
*/
shaka.cast.CastReceiver.prototype.sendAsyncComplete_ =
function(senderId, id, error) {
if (!this.player_) {
// We've already been destroyed.
return;
}

this.sendMessage_({
'type': 'asyncComplete',
'id': id,
Expand Down
2 changes: 1 addition & 1 deletion lib/media/drm_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ shaka.media.DrmEngine.prototype.onKeyStatusesChange_ = function(event) {
if (!this.activeSessions_[i].updatePromise) {
shaka.log.debug('Session has expired', session);
this.activeSessions_.splice(i, 1);
session.close();
session.close().catch(() => {}); // Silence uncaught rejection errors
}
}

Expand Down
28 changes: 27 additions & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ shaka.media.StreamingEngine.prototype.init = function() {
// Setup the initial set of Streams and then begin each update cycle. After
// startup completes onUpdate_() will set up the remaining Periods.
return this.initStreams_(initialStreams).then(function() {
if (this.destroyed_) {
return;
}

shaka.log.debug('init: completed initial Stream setup');

// Subtlety: onInitialStreamsSetup() may call switch() or seeked(), so we
Expand Down Expand Up @@ -514,6 +518,10 @@ shaka.media.StreamingEngine.prototype.loadNewTextStream = function(stream) {
return mediaSourceEngine.init({text: stream}).then(() => {
return this.setupStreams_([stream]);
}).then(() => {
if (this.destroyed_) {
return;
}

if ((this.textStreamSequenceId_ == currentSequenceId) &&
!this.mediaStates_[ContentType.TEXT] && !this.unloadingTextStream_) {
let playheadTime = this.playerInterface_.playhead.getTime();
Expand Down Expand Up @@ -832,13 +840,19 @@ shaka.media.StreamingEngine.prototype.initStreams_ = function(
// Init MediaSourceEngine.
let mediaSourceEngine = this.playerInterface_.mediaSourceEngine;
return mediaSourceEngine.init(streamsByType).then(() => {
if (this.destroyed_) {
return;
}

this.setDuration_();

// Setup the initial set of Streams and then begin each update cycle. After
// startup completes onUpdate_() will set up the remaining Periods.
return this.setupStreams_(streams);
}).then(() => {
if (this.destroyed_) return;
if (this.destroyed_) {
return;
}

for (let type in streamsByType) {
let stream = streamsByType[type];
Expand Down Expand Up @@ -1084,6 +1098,10 @@ shaka.media.StreamingEngine.prototype.onUpdate_ = function(mediaState) {
if (mediaStates.every(function(ms) { return ms.endOfStream; })) {
shaka.log.v1(logPrefix, 'calling endOfStream()...');
this.playerInterface_.mediaSourceEngine.endOfStream().then(function() {
if (this.destroyed_) {
return;
}

// If the media segments don't reach the end, then we need to update the
// timeline duration to match the final media duration to avoid buffering
// forever at the end.
Expand Down Expand Up @@ -1850,6 +1868,10 @@ shaka.media.StreamingEngine.prototype.handleStartup_ = function(
// Period is probably the initial one.
if (!this.canSwitchPeriod_[currentPeriodIndex]) {
this.setupPeriod_(currentPeriodIndex).then(function() {
if (this.destroyed_) {
return;
}

shaka.log.v1(logPrefix, 'calling onCanSwitch()...');
this.playerInterface_.onCanSwitch();
}.bind(this)).catch(Functional.noop);
Expand Down Expand Up @@ -2167,6 +2189,10 @@ shaka.media.StreamingEngine.prototype.handleStreamingError_ = function(error) {
// rapid retry cycle that could be very unkind to the server. Instead,
// use the backoff system to delay and backoff the error handling.
this.failureCallbackBackoff_.attempt().then(function() {
if (this.destroyed_) {
return;
}

// First fire an error event.
this.playerInterface_.onError(error);

Expand Down
Loading

0 comments on commit f48f7fd

Please sign in to comment.