Fix EME controller async errors #2552
Conversation
robwalch
force-pushed
the
bugfix/eme-async
branch
from
42ccde4
to
9ab1544
Compare
robwalch
force-pushed
the
bugfix/eme-async
branch
from
9ab1544
to
39f02b0
Compare
|
Related to #1918 |
| this._onMediaKeySystemAccessObtained(keySystem, mediaKeySystemAccess)); | ||
|
|
||
| keySystemAccessPromise.catch((err) => { | ||
| logger.error(`Failed to obtain key-system "${keySystem}" access:`, err); |
There was a problem hiding this comment.
Why don't we actually trigger errors in catch blocks?
There was a problem hiding this comment.
We definitely can throw an error. Not sure off the top of my head what happens if you rethrow in the catch block, it might actually hit window and stop the current stack from running, rather than just reject the promise.
There was a problem hiding this comment.
I'm just talking about triggering the no-key/access/session error events in catch blocks.
Throwing in the catch block throws the stack, and is caught in a wrapping promise if there is one.
We could throw in methods wrapped in promises like _attemptSetMediaKeys and _generateRequestWithPreferredKeySession and in a catch block, trigger the error events based on error object. These are just thoughts for how to improve the controller in v1. For this PR the goal is to just fix the failing functional test which it does.
| }); | ||
|
|
||
| mediaKeysPromise.catch((err) => { | ||
| logger.error('Failed to create media-keys:', err); |
There was a problem hiding this comment.
If we triggered errors in catch blocks we wouldn't need keysListItem checks in _attemptSetMediaKeys and _generateRequestWithPreferredKeySession.
My thought is to leave as-is for 0.14, but refactor in feature/v1.0.0.
| type: ErrorTypes.KEY_SYSTEM_ERROR, | ||
| details: ErrorDetails.KEY_SYSTEM_NO_KEYS, | ||
| fatal: true | ||
| }); |
There was a problem hiding this comment.
This would really be more of an async error.
If for whatever reason onManifestParsed was not called or did not set mediaKeysPromise, we could always call _attemptKeySystemAccess here to get the ball rolling. That way this redundant error handling could be removed.
This PR will...
Fix
"Fatal: Media is encrypted but no CDM access or no keys have been obtained yet"error by using a mediaKeys promise in the"encrypted"event callback.Why is this Pull Request needed?
EME API is async, but our handling of the 'encrypted' media event is not which causes functional tests to fail.
Checklist