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

better handling of callbacks that might throw in CallbackStack #3989

Open
samspills opened this issue Feb 3, 2024 · 1 comment
Open

better handling of callbacks that might throw in CallbackStack #3989

samspills opened this issue Feb 3, 2024 · 1 comment

Comments

@samspills
Copy link
Contributor

Capturing @durban's question here: #3973 (comment)
And also @armanbilge: #3943 (comment)

I think we're all agreed that the remaining callbacks shouldn't just be abandoned, and when talking it through with Arman he had a proposed refactor:

if CallbackStack encounters a throwing callback, it can install the current/next node as the head in the finally and then let the throw continue
this leaves it up to the caller to decide what they want to do
if they're cool with the exception and want to invoke the rest of the callbacks, they can just call apply again
if they want to tear down, maybe we can bring back clear for this

@samspills samspills changed the title invoke callbacks in CallbackStack better handling of callbacks that might throw in CallbackStack Feb 3, 2024
@djspiewak
Copy link
Member

We should probably be pretty careful here. Any performance hit at all as a consequence of this is not worth it, since we know a priori that none of our internal callbacks will throw, and CallbackStack is strictly internal. If we can do it without any performance hit though, it would be nice to not make unnecessary assumptions.

@armanbilge armanbilge added this to the v3.5.next milestone Feb 19, 2024
@djspiewak djspiewak removed this from the v3.5.next milestone Mar 5, 2024
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

No branches or pull requests

3 participants