Skip to content

Commit

Permalink
Merge pull request #2597 from cloudflare/jsnell/queuemicrotask-report…
Browse files Browse the repository at this point in the history
…error
  • Loading branch information
jasnell authored Aug 24, 2024
2 parents 91fa83b + a33c3a1 commit 537c806
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 8 deletions.
28 changes: 20 additions & 8 deletions src/workerd/api/global-scope.c++
Original file line number Diff line number Diff line change
Expand Up @@ -685,14 +685,26 @@ jsg::JsString ServiceWorkerGlobalScope::atob(jsg::Lock& js, kj::String data) {
}

void ServiceWorkerGlobalScope::queueMicrotask(jsg::Lock& js, v8::Local<v8::Function> task) {
// TODO(later): It currently does not appear as if v8 attaches the continuation embedder data
// to microtasks scheduled using EnqueueMicrotask, so we have to wrap in order to propagate
// the context to those. Once V8 is fixed to correctly associate continuation data with
// microtasks automatically, we can remove this workaround.
KJ_IF_SOME(context, jsg::AsyncContextFrame::current(js)) {
task = context.wrap(js, task);
}
js.v8Isolate->EnqueueMicrotask(task);
auto fn = js.wrapReturningFunction(js.v8Context(),
JSG_VISITABLE_LAMBDA((this, fn = js.v8Ref(task)), (fn),
(jsg::Lock & js, const v8::FunctionCallbackInfo<v8::Value>& args)->v8::Local<v8::Value> {
return js.tryCatch([&]() -> v8::Local<v8::Value> {
auto function = fn.getHandle(js);
auto context = js.v8Context();

kj::Vector<v8::Local<v8::Value>> argv(args.Length());
for (int n = 0; n < args.Length(); n++) {
argv.add(args[n]);
}

return jsg::check(function->Call(context, js.v8Null(), args.Length(), argv.begin()));
}, [&](jsg::Value exception) {
reportError(js, jsg::JsValue(exception.getHandle(js)));
return js.v8Undefined();
});
}));

js.v8Isolate->EnqueueMicrotask(fn);
}

jsg::JsValue ServiceWorkerGlobalScope::structuredClone(
Expand Down
36 changes: 36 additions & 0 deletions src/workerd/api/tests/global-scope-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
ok,
} from 'node:assert';

import { AsyncLocalStorage } from 'node:async_hooks';

export const navigatorUserAgent = {
async test() {
strictEqual(navigator.userAgent, 'Cloudflare-Workers');
Expand Down Expand Up @@ -687,3 +689,37 @@ export const webSocketPairIterable = {
ok(b instanceof WebSocket);
},
};

export const queueMicrotaskError = {
async test() {
const als = new AsyncLocalStorage();
const { promise, resolve } = Promise.withResolvers();
const err = new Error('boom');
err.resolve = resolve;

addEventListener(
'error',
(event) => {
// Verify that async context propagates correctly
// in the error event.
strictEqual(als.getStore(), 123);
// We got the correct expected error.
strictEqual(event.error, err);
event.error.resolve();
// Returning true suppresses the default logging.
return true;
},
{ once: true }
);

als.run(123, () =>
globalThis.queueMicrotask(() => {
// Throwing inside a queueMicrotask should trigger the error event.
strictEqual(als.getStore(), 123);
throw err;
})
);

await promise;
},
};

0 comments on commit 537c806

Please sign in to comment.