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

Absorb exceptions thrown in the JSON-RPC callback #2527

Merged
merged 3 commits into from
Jul 19, 2022

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Jul 19, 2022

Close #2521

This was an overlook. If the user uses throw in a callback (or, more likely, something accidentally throws), the exception will propagate through the smoldot VM and act as if smoldot had crashed.
We don't want that to happen.

Copy link
Contributor

@mergify mergify bot left a comment

Choose a reason for hiding this comment

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

Automatically approving tomaka's pull requests. This auto-approval will be removed once more maintainers are active.

@tomaka
Copy link
Contributor Author

tomaka commented Jul 19, 2022

Note that I'm absorbing exceptions at this level rather than in the callback called by the Wasm VM, because if a callback called by the Wasm VM throws an exception it indicates a bug in the smoldot JS code, in which case it isn't wrong to crash smoldot.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 19, 2022

twiggy diff report

Difference in .wasm size before and after this pull request.


 Delta Bytes │ Item
─────────────┼──────────────────
          +0 ┊ Σ [0 Total Rows]

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

bin/wasm-node/javascript/src/client.ts Show resolved Hide resolved
@tomaka tomaka added the automerge Automatically merge pull request as soon as possible label Jul 19, 2022
@mergify mergify bot merged commit 93d848b into paritytech:main Jul 19, 2022
@tomaka tomaka deleted the no-panic-cb branch July 19, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge pull request as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic in Deno demo when closing JSON-RPC client
2 participants