-
Notifications
You must be signed in to change notification settings - Fork 10k
Changelog for workers-rs panic recovery #25285
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
Changelog for workers-rs panic recovery #25285
Conversation
irvinebroque
left a comment
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.
This is awesome and think it is good to get into this kind of detail here
JohnDaWalka
left a comment
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.
Thx
danlapid
left a comment
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.
LOVE IT
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.
@guybedford this feels more like part of a blog post or docs than a Changelog.
Instead of going into the details (at least initially), I'd make it much more to the point:
- Previously, when in workers-rs when a Rust panic occured, X would happen. This was bad because the Worker would then do Y.
- Now you can add the new
setPanicHookfunctionality to handle these errors.
I thin we can and should go a little into the details after this, but I'd probably try to trim it down a bit from where its at.
Also, I see setPanicHook being imported from index.js. It's not clear to me where it actually comes from and how to implement. Is there a way to add some more color there?
EDIT: Brendan and Dan seemed more into including the details here than I was. 😄 - I'm okay keeping what we have as long as they don't interfere with immediately defining the problem and how we now solve it. So I'd push the wasm-bindgen bits down below, and explain "Wasm is a virtual machine" later too, and then I think its good.
Summary
Provides a technical changelog for the new workers-rs panic recovery functionality.
It's quite nice to go into the details here, but we could cut back on the content too if this is the wrong place for it.
Screenshots (optional)
Documentation checklist