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

Rethrow UncaughtThrowable thrown by callbacks/error handler without wrapping #61

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Rethrow UncaughtThrowable thrown by callbacks/error handler without wrapping #61

merged 1 commit into from
Oct 24, 2022

Conversation

Nevay
Copy link
Contributor

@Nevay Nevay commented Aug 15, 2022

Useful for driver decorators that wrap provided callbacks in their own callbacks; allows reporting the original callback as error cause.

Allows driver decorators to report the original callable as error cause.
Copy link
Member

@kelunik kelunik left a comment

Choose a reason for hiding this comment

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

LGTM. Strictly speaking, this might be considered a breaking change, so we should probably wait with merging until we decide to release a new major, i.e. 0.3.0 or 1.0.0.

@Nevay
Copy link
Contributor Author

Nevay commented Sep 9, 2022

If we wait until the next major: should we skip the loop errorhandler if a callback throws an UncaughtThrowable?
This would allow decorating drivers to either check if an errorhandler is set and rethrow the original exception instead of an UncaughtThrowable to use the default error handling or implement their own error handling and always throw an UncaughtThrowable to skip the default error handling.

@kelunik
Copy link
Member

kelunik commented Sep 9, 2022

I plan to release 1.0 soon anyway. I want to rework getInfo for 1.0, other than that we're ready for it, IMO. I'll try to get that done in the coming days.

@kelunik kelunik added this to the 1.0.0 milestone Oct 13, 2022
@kelunik
Copy link
Member

kelunik commented Oct 13, 2022

I've created the 1.0 milestone now and added this PR. I've also opened #62 with the change I had in mind that I mentioned earlier.

@kelunik
Copy link
Member

kelunik commented Oct 13, 2022

if a callback throws an UncaughtThrowable

In which case does that happen?

@Nevay
Copy link
Contributor Author

Nevay commented Oct 13, 2022

if a callback throws an UncaughtThrowable

Should only happen when using a driver decorator that implements custom error handling.
My use case: I'm calling the errorhandler within registered callbacks to propagate tracing information from user provided callbacks to the errorhandler (ref).

@kelunik kelunik merged commit a0c1f51 into revoltphp:main Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants