rive-wasm icon indicating copy to clipboard operation
rive-wasm copied to clipboard

Uncaught ReferenceError error(s) when WebAssembly is not available

Open localpcguy opened this issue 4 years ago • 3 comments

Despite checks for WebAssembly support, the abort method then tries to use WebAssembly to create a RuntimeError and crashes with an Uncaught ReferenceError. This is in an Angular app using ng-rive, but the errors appears to be in the code here.

Expected: if lack of WebAssembly support can cause abort to be called, it should recheck its availability before using it internally.

(note: sorry, the samples are from the rive.mjs file I see in my browser and published versions here as I couldn't find where they were compiled from in a quick search in this repo without a deeper dive)

The check to see if WebAssembly exists: https://github.com/rive-app/rive-wasm/blob/9a57351dad86f01dcbadd88c05ec67aad2d166bb/wasm/publish/rive.lean.mjs#L215-L217

however, the abort function tries to use WebAssembly: https://github.com/rive-app/rive-wasm/blob/9a57351dad86f01dcbadd88c05ec67aad2d166bb/wasm/publish/rive.js#L555-L567

Specifically: https://github.com/rive-app/rive-wasm/blob/9a57351dad86f01dcbadd88c05ec67aad2d166bb/wasm/publish/rive.js#L564

Error in console:

image

localpcguy avatar Dec 09 '21 16:12 localpcguy

I might be interested in trying to resolve this if someone would be able to point me in the right direction for the source code that compiles into these checks.

localpcguy avatar Aug 25 '22 17:08 localpcguy

@localpcguy Sorry for not getting back here! The file you referenced publish/rive.js is from an older version of the web/wasm runtime to support a low-level usage of the API. I believe we can delete this to prevent confusion.

The code you are referencing comes from emscripten itself and compiles in JS binding code that loads in our web assembly. In that link, you'll see the FAQ they provide around this. I do find it kind of strange that at that level it is calling WebAssembly to throw a runtime error. You can find that error here: https://github.com/emscripten-core/emscripten/blob/main/src/preamble.js#L505

We might be able to add the check at the high-level JS before loading it in potentially (in our js/rive.ts, for usage in @rive-app/canvas or @rive-app/webgl) if that helps and throw a error you can catch in a callback perhaps. Maybe onLoadError but potentially a new callback for WASM failure.

zplata avatar Aug 29 '22 15:08 zplata

Thanks, I'll try to take a look and see if 1) it's still an issue with the the newer JS files (I've been using this through ngRive and so may need to push for or create a PR to upgrade that if it's been resolved there. If not, I'll see what I can figure out and report back, although may not be immediately.

localpcguy avatar Aug 29 '22 17:08 localpcguy

Hi @localpcguy - do you still have this issue today?

zplata avatar Feb 03 '23 17:02 zplata

I can try to test it later on, in our app, we use a wrapper component / widget that tests for WASM support prior to trying to instantiate Rive, and if it doesn't exist, doesn't load Rive (and provides a fallback). So I'll need to disable that check and see if it still errors then. I do still see that same code lines in my original post above. I have not had a chance to investigate whether ng-rive needs updates.

localpcguy avatar Feb 04 '23 00:02 localpcguy

I was able to check and verify that I am still seeing this error. However, I'm guessing it is as you noted, that it might be using an older version of your code. The ng-rive package (for using Rive in Angular apps via an Angular component) is using "rive-canvas": "^0.7.6", (as opposed to of the @rive-app/canvas variant(s)) and it looks like that was last updated in June '21. I think there have been some pretty significant updates since then. I can't say for certain though whether this bug would then go away, given that it's maybe not directly related to the Rive code specifically but rather Emscripten compiling. I'll open an issue on the ng-rive package and see whether there are intentions to update or if they are accepting PRs. I'll reference this issue so it should get a cross-link when I do that.

localpcguy avatar Feb 06 '23 14:02 localpcguy

Just wanted to update this, still seeing the same error, now using a more up to date version of rive-canvas ("@rive-app/canvas-advanced": "^2.4.0").

console showing wasm support not detected and then WebAssembly error console showing some of the Rive files the error originates in

I took a look into the Emscripten code in the preamble where this error gets thrown, and it seems to be intentional as per this comment recommended adding that change and the PR implementing it.

At that time, it was still wrapped in an if block that looked for WASM, however, the removal of fastcomp meant that would always be true, so a subsequent PR removed the check altogether.

That's where I left off. My understanding is that throwing a WebAssembly.RuntimeError was intentional. I'm not positive from what I saw that it should break when WebAssembly itself is undefined/not supported/disabled though. I could not find a specific issue open or closed on the emscripten repo discussing this. I may open one to get see if it's something that can be fixed or not - I don't know what kind of side-effects a change to check for WebAssembly before calling RuntimeError on it would have. My guess is it'd just go from throwing Uncaught Reference: WebAssembly is not defined to manually defined error effectively saying the same thing.

For this issue, I'll likely just continue to check for WebAssembly support before trying to instantiate a Rive animation or use any of the Rive WASM functionality. At this point, up to y'all if you want to keep this issue open or not.

localpcguy avatar Mar 07 '24 00:03 localpcguy

Closing - the fix in Emscripten was to remove the WebAssembly check from release builds altogether and only check for WebAssembly in debug builds. Closing this issue as it is intentional that the generated code throw the error that WebAssembly is not defined when used on an unsupported browser. It may be slightly more likely once the implemented fix propagates to the version being used.

localpcguy avatar Mar 18 '24 23:03 localpcguy