Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Provide readyPromiseReject to instantiateWasm #23038

Open
FelixNumworks opened this issue Nov 29, 2024 · 7 comments · May be fixed by #23780
Open

Provide readyPromiseReject to instantiateWasm #23038

FelixNumworks opened this issue Nov 29, 2024 · 7 comments · May be fixed by #23780

Comments

@FelixNumworks
Copy link

FelixNumworks commented Nov 29, 2024 •
edited
Loading

When providing an instantiateWasm method to the Module, only a successCallback is passed to the method (see docs here)

The problem is that no failureCallback is provided.
In the case of a sync instantiation of the WASM, I can return false if the instantiation fails.
But in the case of an async instantiation, if the instantiation fails, the error isn't properly handled and the page freezes.

I'd like instantiateWasm to take 3 args instead of 2: imports, successCallback and failureCallback.

This means changing this line:

emscripten/src/preamble.js

Line 1067 in 06cebfc

return Module['instantiateWasm'](info, receiveInstance);

to

return Module['instantiateWasm'](info, receiveInstance, readyPromiseReject);

Current workaround

For others having the same problem, I'm currently using a dirty hack through pre-js.js by doing:

// pre-js.js

if (Module['readyPromiseRejectWrapper']) {
  Module['readyPromiseRejectWrapper']['value'] = readyPromiseReject;
}

And when I instantiate my module:

const readyPromiseRejectWrapper = { value: (e) => {} };
// Pass the wrapper to Module
const module = Module(
  readyPromiseRejectWrapper,
  // You can use the wrapper in 'instantiateWasm' when instantiation fails
  instantiateWasm: (imports, successCallback) => {
    (async () => {
      try {
        const instance = await foo(); // instantiate the wasm
        successCallback(instance, module);
      } catch (e) {
        readyPromiseRejectWrapper.value(e);
      }
    })();
    return {};
  }
);

Version of emscripten/emsdk:
emcc 3.1.61
@edmond-chow
Copy link

Do you mean when I call the .catch or .then using Module({}) after still cannot catch up the things from quit_? Also where can I find the pre-js.js using a dirty hack?

@FelixNumworks
Copy link
Author

Do you mean when I call the .catch or .then using Module({}) after still cannot catch up the things from quit_?

Sorry I don't understand the sentence :/ What I mean is that in instantiateWasm, there is no way to properly reject the promise after an instanciation failure.

Also where can I find the pre-js.js using a dirty hack?

You should create the pre-j.js file yourself, and add it as an emcc argument.
See https://emscripten.org/docs/tools_reference/emcc.html (search -pre-js in the page)

@hoodmane
Copy link
Collaborator

I've definitely run into this problem that if an error is thrown in instantiateWasm Emscripten does not handle it cleanly so +1 for this issue.

I think we ought to deprecate instantiateWasm and replace it with a new setting that we can pass an async function to. Emscripten can use normal async/await now and that will make both user code and the Emscripten source simpler.

@FelixNumworks FelixNumworks linked a pull request Feb 27, 2025 that will close this issue
Open
@sbc100
Copy link
Collaborator

sbc100 commented Feb 27, 2025

@FelixNumworks, can I ask what you use case is for overriding instantiateWasm? I'm trying to understand how/why certain options are used.

@FelixNumworks
Copy link
Author

@sbc100

I need to fetch my .wasm. The canonical method to do this would be to pass a locateFile method to the module. The problem is that I need to fetch it with the option { credentials: 'include' }, which is not done by default (default is { credentials: 'same-origin' }

I am now realizing that the parameter fetchSettings can be passed to the Module (found here), but it needs to be explicitely declared in INCOMING_MODULE_JS_API

Since this works (locateFile + fetchSettings) and results in a clearer implementation, I will probably use this method rather than instantiateWasm

I must say though that having to explicitly set all my module parameters in INCOMING_MODULE_JS_API is a bit burdensome.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 28, 2025

I must say though that having to explicitly set all my module parameters in INCOMING_MODULE_JS_API is a bit burdensome.

Yes, unfortunately is code size and complexity thing. The problem is that each element of INCOMING_MODULE_JS_API adds code size and complexity to the generated program. We would like to keep the default output simple and small and have folks opt into extra complexity.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 28, 2025

Great that fetchSettings works for you. We will still obviously work to fix/improve instantiateWasm in case there are other use cases out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Provide a way to reject Promise in instantiateWasm FelixNumworks/emscripten
4 participants
@sbc100 @hoodmane @edmond-chow @FelixNumworks