perf(ext/ffi): switch from middleware to tasks #21239
Conversation
mmastrac
force-pushed
the
promise_test
branch
from
bb3f951
to
a922941
Compare
mmastrac
changed the title
mmastrac
force-pushed
the
promise_test
branch
2 times, most recently
from
2d0c89e
to
a47ce60
Compare
mmastrac
changed the title
mmastrac
force-pushed
the
promise_test
branch
from
9ab4a7d
to
fe543d3
Compare
mmastrac
changed the title
mmastrac
force-pushed
the
promise_test
branch
from
e5452ed
to
5589e77
Compare
ext/ffi/callback.rs
Outdated
| }; | ||
|
|
||
| async_work_sender.spawn_blocking(move |scope| { | ||
| args.run(scope); |
There was a problem hiding this comment.
question: What happens with errors thrown from here?
There was a problem hiding this comment.
Because these are tasks and we're technically violating the contract by allowing them to bubble into the scope, they end up as "uncaught errors". It doesn't kill the runtime but it isn't a great situation.
It should not be the same as our current state of affairs, however, but I suppose we could add a tc_scope here and print a last-ditch message.
There was a problem hiding this comment.
Note that we will need to figure out what to do with this test as it's failing right now -- either we could punt with a TODO for later, or try to add the last-ditch handler.
There was a problem hiding this comment.
Hm. I did add the error and unhandled rejection listeners in the test but they were not getting fired. Are you sure the errors end up as properly handled uncaught errors/rejections?
There was a problem hiding this comment.
Let me dig into this a bit and see what is going on before we land it...
| ); | ||
|
|
||
| globalThis.addEventListener("error", (data) => { | ||
| console.log("Unhandled error"); |
| console.log("Unhandled error"); | ||
| data.preventDefault(); | ||
| }); | ||
| globalThis.onerror = (data) => { |
There was a problem hiding this comment.
Nor does this get called; this also should be redundant with the one above.
Deno-side changes for denoland/deno_core#350