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

chore: add contrib-eme errors #8634

Merged
merged 2 commits into from
Mar 12, 2024
Merged

chore: add contrib-eme errors #8634

merged 2 commits into from
Mar 12, 2024

Conversation

adrums86
Copy link
Contributor

@adrums86 adrums86 commented Mar 8, 2024

Description

Add error constants for contrib-eme specific errors.

Specific Changes proposed

Add constants that are representative of the separate keysystems for error reporting out of videojs-contrib-eme, also add errors specific to each step in the EME specific process.
Two notable omissions:
No errors for remove or load on the mediakeysession as they're not implemented in contrib-eme.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.71%. Comparing base (9977a93) to head (1e6d5e7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8634      +/-   ##
==========================================
- Coverage   83.43%   82.71%   -0.73%     
==========================================
  Files         113      113              
  Lines        7636     7636              
  Branches     1835     1835              
==========================================
- Hits         6371     6316      -55     
- Misses       1265     1320      +55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@wseymour15 wseymour15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@adrums86 adrums86 merged commit 42fe1e6 into main Mar 12, 2024
14 of 15 checks passed
@adrums86 adrums86 deleted the contrib-eme-errors branch March 12, 2024 19:10
// Errors used in contrib-eme:
EMEEncryptedError: 'eme-encrypted-error',
MSKeyError: 'ms-key-error',
WebkitKeyError: 'webkit-key-error',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate more on this 3:

What are they?
Why do we need them?

  EMEEncryptedError: 'eme-encrypted-error',
  MSKeyError: 'ms-key-error',
  WebkitKeyError: 'webkit-key-error',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea behind these 3 was to be a catch-all for the outermost promise to the respective handle events. For MSKeyError and WebkitKeyError they specifically cover errors from legacy workflows ('msneedey' and 'webkitneedkey' events). Since this is not really in-spec EME, I figured they were worthy of their own specific errors. For EMEEncryptedError it might not be necessary and was only added incase any other type of unknown error occurs and bubbles up to the handleEncryptedEvent promise.

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.

3 participants