diff options
author | Andreu Botella <andreu@andreubotella.com> | 2022-03-29 14:44:33 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-29 14:44:33 +0200 |
commit | d983b577bc903f18028a99d0a40a17322ac80ffe (patch) | |
tree | 177395212614bc600cb8c073ecccec1469d9eec0 /ext/fetch | |
parent | f7ce96ea6e7a720d578b6f7f719fba90c8afdded (diff) |
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.
Diffstat (limited to 'ext/fetch')
-rw-r--r-- | ext/fetch/26_fetch.js | 88 |
1 files changed, 46 insertions, 42 deletions
diff --git a/ext/fetch/26_fetch.js b/ext/fetch/26_fetch.js index b13552b02..be7c48dda 100644 --- a/ext/fetch/26_fetch.js +++ b/ext/fetch/26_fetch.js @@ -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", - }); - - // 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."); - } - } + try { + const res = webidl.converters["Response"](source, { + prefix: "Failed to call 'WebAssembly.compileStreaming'", + context: "Argument 1", + }); - // 2.5. - if (!res.ok) { - throw new TypeError(`HTTP status code ${res.status}`); + // 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."); } + } - // Pass the resolved URL to v8. - core.opSync("op_wasm_streaming_set_url", rid, res.url); + // 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); + 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 ??= {}; |