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

Wasm driver throws string errors #25

Closed
adomasven opened this issue Dec 3, 2019 · 3 comments · Fixed by #75
Closed

Wasm driver throws string errors #25

adomasven opened this issue Dec 3, 2019 · 3 comments · Fixed by #75
Labels
A-wasm Area: wasm package on npm I-packaging Issue: problem with a built package

Comments

@adomasven
Copy link
Member

Wasm driver throws string errors from rust level code (instead of Error object). E.g.

could not parse cluster from host: data did not match any variant of untagged enum Cluster2

String errors do not contain a stack trace, which makes them hard to debug.

@cormacrelf
Copy link
Collaborator

cormacrelf commented Dec 3, 2019

There was a reason I didn’t create Error objects, but I don’t recall why now. I think it was something to do with wasm-bindgen, but I don’t see any barrier today. Although at the end of the day, you will never get any deeper trace into Rust than the single stack frame showing a Driver function, see why below. You would get your own code, which would be helpful.

(Rust errors can be any type. In general, they do not include backtraces as that wouldn’t be “zero-cost” as Rust likes to put it. Storing a backtrace is an explicit operation. None of the errors in citeproc-rs do it. Although panicking creates a backtrace. If they happen, there is usually a problem with the library, but you’ll get a backtrace printed out for debug builds. There are a few panics (unwrap, expect, unreachable, unimplemented, panic) in the codebase still. General rule is that unwrap means “Cormac didn’t bubble up this client usage error and he should do that”, but the others are more likely logic errors.)

FWIW the error you refer to is from generated code to deserialise Cluster2 from JSON. The deserialjzer gets a bit fragile with untagged enums (here detecting note vs inText and choosing an enum variant based on that) because unrecognised fields have to be treated as possibly matching a different variant. But that will be improved if we get rid of those fields and make it a struct instead, and you’ll get good errors like “unrecognised field” instead.

@adomasven
Copy link
Member Author

Although at the end of the day, you will never get any deeper trace into Rust than the single stack frame showing a Driver function

That's fine. At least you get an idea which calling code resulted in the error and what's the potential problem.

As for the reason for this error, it went away once I added an inText property. I assume you didn't make them optional in #24 yet?

@cormacrelf
Copy link
Collaborator

Correct.

cormacrelf added a commit that referenced this issue Sep 21, 2020
…fix #25)

This is for the sake of throwing errors that have stack traces, rather
than strings which strip them for JS hosts.

*Slight* API change in that setStyle now throws an error with a message,
rather than returning a perfectly serialized JSON csl::StyleError. But
that's an API for elsewhere; it would be called parseStyle, and it needs
more features than setStyle does.
@cormacrelf cormacrelf added A-wasm Area: wasm package on npm I-packaging Issue: problem with a built package labels Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-wasm Area: wasm package on npm I-packaging Issue: problem with a built package
Development

Successfully merging a pull request may close this issue.

2 participants