JS InstantiateStreaming fallback does not work

[yoshuawuyts]

Describe the Bug

In the generated JS file there's a fallback for when InstantiateStreaming doesn't work. This code will never succeed because the fetch body has already been consumed:

wasm-bindgen/crates/cli-support/src/js/mod.rs

Lines 538 to 546 in 732b691

	.catch(e => {{
	console.warn(\"`WebAssembly.instantiateStreaming` failed. Assuming this is \
	because your server does not serve wasm with \
	`application/wasm` MIME type. Falling back to \
	`WebAssembly.instantiate` which is slower. Original \
	error:\\n\", e);
	return response
	.then(r => r.arrayBuffer())
	.then(bytes => WebAssembly.instantiate(bytes, imports));

Steps to Reproduce

I've got a bad wasm setup right now, and not sure why it's bad (I do have the mime set to application/wasm, but yeah, not sure what's going on).

Expected Behavior

The fallback should be working.

Actual Behavior

The fallback breaks because the request body is already consumed. Not working on both Chrome and Firefox.

chrome

firefox

Additional Context

I think the way to fix it is by sending out another fetch request. It's wasteful, but it's probably better than straight up throwing.

Also I should figure out how to make streaming work from my server, but that's on me, haha. Thanks!

[alexcrichton]

Thanks for the report! The bug here is actually unrelated to appliation/wasm mime types but there's a real instantiation error which happens regardless.

I do think though that there's a bug in our instantiation fallback in the sense that this is a pretty confusing error message and we should probably get the original error to get displayed.

@fitzgen would you be able to take a look at this?

[fitzgen]

I've definitely had the fallback work for me (eg with python -m SimpleHTTPServer) so either we broke that support (possible) or it is hiding the real error, as Alex points out (likely). Will take a look.

[fitzgen]

Well, we do get the original error displayed, in this case it is

TypeError: "import object field './playground_wasm_async.js' is not an Object"

So it looks like we are generating an import, but then the isntantiation code is buggy?

[alexcrichton]

Oh sorry yeah what I mean is that the instantiation fallback definitely works for the mime type, but it's aggressively retrying all errors as opposed to just errors related to the mime type. We should ideally only handle errors related to mime type and otherwise propagate all other errors.

(this is all sort of independent of whether there's a wasm-bindgen bug as well)

[fitzgen]

This is kind of a philosophical question. We don't have a dedicated Error subtype that we can instanceof check for these mime type failures. Therefore, our options are:

1. Catch and retry for every error (our current behavior).

2. We could also never catch and retry (our old behavior).

3. Or we can try and do some reflection on the error type and message and see if we think it is likely a mime type error, and only catch and retry then. If that imprecise reflection check isn't very good we end up with false positives (like in the OP; annoying and potentially confusing) or false negatives (doesn't retry when it actually did get a mime type error; frustrating because the wasm won't load with server S and browser B). Additionally, this is a little bit more JS code size for every project using wasm-bindgen.

Do we want to play the (3) game? Do the benefits outweigh the costs in this situation?

[alexcrichton]

I do sort of think we want to play the (3) game a little bit because I've been pretty confused by this before. Our fallback reports the wrong error in the JS console since the body is already consumed if the instantiation fails for any reason other than headers not working well.

Could we perhaps do something like:

• call fetch
• call instantiateStreaming immediately
• if instantiateStreaming fails, use the return value of fetch and use .then to get the real response
• Given the response, look at the headers and see if the mime type was correct
• If it's the wrong mime type, do our fallback, otherwise reject the promise with the original error

That way we're not really doing error detection per se, but we are only whitelisting one case for continuing with the fallback. Similarly we could try to call r.arrayBuffer() but if that fails just return the original error instead.