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.

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

Ensure error is actually visible if wasm fails to load #147

Closed
lukaslueg opened this issue Oct 20, 2018 · 10 comments
Closed

Ensure error is actually visible if wasm fails to load #147

lukaslueg opened this issue Oct 20, 2018 · 10 comments

Comments

@lukaslueg
Copy link

console.log( "Error loading Rust wasm module '{{{module_name}}}':", error );

This should be error.toString(), not just error to ensure that the error message is actually printed. If the object can't be cloned, the message will be "<unavailable>", which is not helpful at all.

@Pauan
Copy link
Contributor

Pauan commented Oct 20, 2018

If the object can't be cloned, the message will be "", which is not helpful at all.

Is that true with Firefox extensions? I know Firefox in general supports printing error objects without any problem (they're not cloned, they're just printed).

@lukaslueg
Copy link
Author

At least in the context of loading an extension, the error is always <unavailable>, unless it's manually turned into a string first.

@koute
Copy link
Owner

koute commented Dec 3, 2018

At least in the context of loading an extension, the error is always <unavailable>, unless it's manually turned into a string first.

That sounds to me like a bug of console.log. Are you sure it isn't a bug?

@Pauan
Copy link
Contributor

Pauan commented Dec 6, 2018

@koute Yeah, it's a bug with Firefox Extensions: using console.log with an Error works fine in Firefox, Chrome, and Chrome Extensions (just not in Firefox Extensions):

https://bugzilla.mozilla.org/show_bug.cgi?id=1327280
https://bugzilla.mozilla.org/show_bug.cgi?id=1462501
https://bugzilla.mozilla.org/show_bug.cgi?id=1393026

Technically it's an intentional feature omission (not a bug), but it sounds like they have plans to fix it regardless.

@lukaslueg
Copy link
Author

We could have the shim try to avoid the situation, yet as far as I'm concerned this can be closed.

@Pauan
Copy link
Contributor

Pauan commented Dec 6, 2018

@lukaslueg Personally, I think it's reasonable for the web-extension runtime to use .toString(), since this is an issue only with Firefox extensions. Other runtimes would be unchanged.

It would only affect the wasm loading logic, other errors might still show up as <unavailable>. So the impact is minimized.

Do you want to make a PR for that, or should I?

@lukaslueg
Copy link
Author

lukaslueg commented Dec 6, 2018 via email

@koute
Copy link
Owner

koute commented Dec 6, 2018

Well, I'm fine with adding the .toString() if it's only in the context of the extension-specific runtime. We can always remove it later when the issue is fixed in Firefox.

Pauan added a commit to Pauan/cargo-web that referenced this issue Dec 6, 2018
@Pauan
Copy link
Contributor

Pauan commented Dec 6, 2018

This was fixed by #162

@koute
Copy link
Owner

koute commented Dec 12, 2018

Published as 0.6.22

@koute koute closed this as completed Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants