mirror of
https://github.com/denoland/deno.git
synced 2025-01-12 00:54:02 -05:00
chore(wasm): Don't await on the argument to handleWasmStreaming
(#14000)
`handleWasmStreaming` is the function that provides the binding with the `fetch` API needed for `WebAssembly.instantiateStreaming()` and `WebAssembly.compileStreaming()`. When I implemented it in #11200, I thought V8 was calling these functions with the argument of the `WebAssembly` streaming functions, without doing any resolving, and so `handleWasmStreaming` awaits for the parameter to resolve. However, as discovered in https://github.com/denoland/deno/issues/13917#issuecomment-1065805565, V8 does in fact resolve the parameter if it's a promise (and handles rejections arising from that). This change removes the `async` IIFE inside `handleWasmStreaming`, letting initial errors be handled synchronously (which will however not throw synchronously from the `WebAssembly` namespace functions). Awaiting is still necessary for reading the bytes of the response, though, and so there is an `async` IIFE for that.
This commit is contained in:
parent
f7ce96ea6e
commit
d983b577bc
3 changed files with 50 additions and 46 deletions
|
@ -1,2 +1,2 @@
|
|||
error: Uncaught (in promise) TypeError: Invalid WebAssembly content type.
|
||||
at deno:ext/fetch/26_fetch.js:[WILDCARD]
|
||||
at handleWasmStreaming (deno:ext/fetch/26_fetch.js:[WILDCARD])
|
||||
|
|
|
@ -45,11 +45,11 @@ Deno.test(
|
|||
|
||||
Deno.test(
|
||||
async function wasmInstantiateStreamingNoContentType() {
|
||||
const response = new Response(simpleWasm);
|
||||
// Rejects, not throws.
|
||||
const wasmPromise = WebAssembly.instantiateStreaming(response);
|
||||
await assertRejects(
|
||||
async () => {
|
||||
const response = Promise.resolve(new Response(simpleWasm));
|
||||
await WebAssembly.instantiateStreaming(response);
|
||||
},
|
||||
() => wasmPromise,
|
||||
TypeError,
|
||||
"Invalid WebAssembly content type.",
|
||||
);
|
||||
|
|
|
@ -510,68 +510,72 @@
|
|||
}
|
||||
|
||||
/**
|
||||
* Handle the Promise<Response> argument to the WebAssembly streaming
|
||||
* APIs. This function should be registered through
|
||||
* `Deno.core.setWasmStreamingCallback`.
|
||||
* Handle the Response argument to the WebAssembly streaming APIs, after
|
||||
* resolving if it was passed as a promise. This function should be registered
|
||||
* through `Deno.core.setWasmStreamingCallback`.
|
||||
*
|
||||
* @param {any} source The source parameter that the WebAssembly
|
||||
* streaming API was called with.
|
||||
* @param {number} rid An rid that represents the wasm streaming
|
||||
* resource.
|
||||
* @param {any} source The source parameter that the WebAssembly streaming API
|
||||
* was called with. If it was called with a Promise, `source` is the resolved
|
||||
* value of that promise.
|
||||
* @param {number} rid An rid that represents the wasm streaming resource.
|
||||
*/
|
||||
function handleWasmStreaming(source, rid) {
|
||||
// This implements part of
|
||||
// https://webassembly.github.io/spec/web-api/#compile-a-potential-webassembly-response
|
||||
(async () => {
|
||||
try {
|
||||
const res = webidl.converters["Response"](await source, {
|
||||
prefix: "Failed to call 'WebAssembly.compileStreaming'",
|
||||
context: "Argument 1",
|
||||
});
|
||||
try {
|
||||
const res = webidl.converters["Response"](source, {
|
||||
prefix: "Failed to call 'WebAssembly.compileStreaming'",
|
||||
context: "Argument 1",
|
||||
});
|
||||
|
||||
// 2.3.
|
||||
// The spec is ambiguous here, see
|
||||
// https://github.com/WebAssembly/spec/issues/1138. The WPT tests
|
||||
// expect the raw value of the Content-Type attribute lowercased.
|
||||
// We ignore this for file:// because file fetches don't have a
|
||||
// Content-Type.
|
||||
if (!StringPrototypeStartsWith(res.url, "file://")) {
|
||||
const contentType = res.headers.get("Content-Type");
|
||||
if (
|
||||
typeof contentType !== "string" ||
|
||||
StringPrototypeToLowerCase(contentType) !== "application/wasm"
|
||||
) {
|
||||
throw new TypeError("Invalid WebAssembly content type.");
|
||||
}
|
||||
// 2.3.
|
||||
// The spec is ambiguous here, see
|
||||
// https://github.com/WebAssembly/spec/issues/1138. The WPT tests expect
|
||||
// the raw value of the Content-Type attribute lowercased. We ignore this
|
||||
// for file:// because file fetches don't have a Content-Type.
|
||||
if (!StringPrototypeStartsWith(res.url, "file://")) {
|
||||
const contentType = res.headers.get("Content-Type");
|
||||
if (
|
||||
typeof contentType !== "string" ||
|
||||
StringPrototypeToLowerCase(contentType) !== "application/wasm"
|
||||
) {
|
||||
throw new TypeError("Invalid WebAssembly content type.");
|
||||
}
|
||||
}
|
||||
|
||||
// 2.5.
|
||||
if (!res.ok) {
|
||||
throw new TypeError(`HTTP status code ${res.status}`);
|
||||
}
|
||||
// 2.5.
|
||||
if (!res.ok) {
|
||||
throw new TypeError(`HTTP status code ${res.status}`);
|
||||
}
|
||||
|
||||
// Pass the resolved URL to v8.
|
||||
core.opSync("op_wasm_streaming_set_url", rid, res.url);
|
||||
// Pass the resolved URL to v8.
|
||||
core.opSync("op_wasm_streaming_set_url", rid, res.url);
|
||||
|
||||
if (res.body !== null) {
|
||||
// 2.6.
|
||||
// Rather than consuming the body as an ArrayBuffer, this passes each
|
||||
// chunk to the feed as soon as it's available.
|
||||
if (res.body !== null) {
|
||||
(async () => {
|
||||
const reader = res.body.getReader();
|
||||
while (true) {
|
||||
const { value: chunk, done } = await reader.read();
|
||||
if (done) break;
|
||||
core.opSync("op_wasm_streaming_feed", rid, chunk);
|
||||
}
|
||||
}
|
||||
|
||||
// 2.7.
|
||||
})().then(
|
||||
// 2.7
|
||||
() => core.close(rid),
|
||||
// 2.8
|
||||
(err) => core.abortWasmStreaming(rid, err),
|
||||
);
|
||||
} else {
|
||||
// 2.7
|
||||
core.close(rid);
|
||||
} catch (err) {
|
||||
// 2.8 and 3
|
||||
core.abortWasmStreaming(rid, err);
|
||||
}
|
||||
})();
|
||||
} catch (err) {
|
||||
// 2.8
|
||||
core.abortWasmStreaming(rid, err);
|
||||
}
|
||||
}
|
||||
|
||||
window.__bootstrap.fetch ??= {};
|
||||
|
|
Loading…
Reference in a new issue