Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PlaybackErrorEvent error TS typing #4363

Conversation

kris-youview
Copy link
Contributor

@kris-youview kris-youview commented Jan 17, 2024

PlaybackErrorEvent when triggered in PlaybackController, attaches the original MediaError as error property. This commit corrects the TS typing to reflect that.

Context

See here for reference: https://github.com/Dash-Industry-Forum/dash.js/blob/development/src/streaming/controllers/PlaybackController.js#L714

We've encountered this as we pass errors using MessageBus and we failed to serialise this one as it carries an object property (thus becomes an object itself) which was not correctly reflected in PlaybackErrorEvent TS type.

Also see here: https://html.spec.whatwg.org/multipage/media.html#dom-media-error

@dsilhavy dsilhavy added this to the 4.7.4 milestone Jan 17, 2024
@dsilhavy dsilhavy self-requested a review January 17, 2024 13:48
`PlaybackErrorEvent` when triggered in `PlaybackController`, attaches
the original `MediaError` as `error` property.  This commit corrects the
TS typing to reflect that.
@kris-youview
Copy link
Contributor Author

Updated the unit test for this.

@dsilhavy dsilhavy merged commit 49257d9 into Dash-Industry-Forum:development Jan 18, 2024
3 checks passed
eirikbjornr pushed a commit to bbc/dash.js that referenced this pull request Feb 14, 2024
`PlaybackErrorEvent` when triggered in `PlaybackController`, attaches
the original `MediaError` as `error` property.  This commit corrects the
TS typing to reflect that.
@kris-youview kris-youview deleted the kris/playback-error-event-fix branch March 27, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants