-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
feat(hooks): Error hooks #7349
feat(hooks): Error hooks #7349
Conversation
Codecov Report
@@ Coverage Diff @@
## main #7349 +/- ##
==========================================
+ Coverage 79.62% 79.66% +0.04%
==========================================
Files 115 116 +1
Lines 7263 7279 +16
Branches 1746 1751 +5
==========================================
+ Hits 5783 5799 +16
Misses 1480 1480
Continue to review full report at Codecov.
|
docs/guides/hooks.md
Outdated
### beforeerror | ||
|
||
`beforeerror` occurs just as we get an error on the player. This allows plugins or other custom code to intercept the error and modify it to be something else. | ||
`error` is can be an object such as `{code: 4}` or something with a message. It potentially includes a `cause` property too. Alternatively, it could be `null` which means that the current error will be getting cleared. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`error` is can be an object such as `{code: 4}` or something with a message. It potentially includes a `cause` property too. Alternatively, it could be `null` which means that the current error will be getting cleared. | |
`error` is an object that has a `code` and `message` property and potentially a `cause` property too. Alternatively, it could be `null` which means that the current error should be cleared. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like error can be a string or just a number too, so, I'll update this and the code based on expectations.
@@ -3991,6 +4005,9 @@ class Player extends Component { | |||
*/ | |||
this.trigger('error'); | |||
|
|||
// notify hooks of the per player error | |||
hooks('error').forEach((hookFunction) => hookFunction(this, this.error_)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use the err
here so that a hook function does not modify player.error_ and cause weird things to happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
player.error()
returns player.error_
directly, so, I don't see an issue with returning it here too.
Adding beforeerror and error hooks that make it easier to know when errors occurred on all players and allows intercepting and modifying errors.
Adding
beforeerror
anderror
hooks that make it easier to know when errors occurred on all players and allows intercepting and modifying errors.