wasm-bindgen
wasm-bindgen copied to clipboard
rustwasm
Allow for returning custom error type in Result
Currently only Result<T, JsValue> is allowed, would it be reasonable to change this to Result<T, I> so long as I: Into<JsValue> to allow custom error types to be returned?
Happy to open a PR if this makes sense
Indeed allowing something other than just a JsValue itself is something we'd like to enable! I think though we may not want a blanket implementation for I: Into<JsValue> as that's roughly already covered by the ? operator perhaps?
One idea @fitzgen and I had awhile back was to add some sort of error type to wasm-bindgen that you can ? any std::error::Error-implementing error into. That way you could use that error type and use ? for all application errors, and then our wasm-bindgen error would know how to convert itself to a JS error (using the contents of the actual error)
I think though if we did I: Into<JsValue> it may prohibit this implementation maybe? (coherence overlap and whatnot)
I'm curious though, would such an error cover your use case?
My use case is laziness :) It would be ideal for me if I can just sprinkle my existing library with a few well placed #[wasm_bindgen]s, implement Into<JsValue> on my custom error type and call it a day. Having to change my function signatures to return either a JsValue or some new wasm-bindgen specific error type is extra code that I'd prefer to avoid - even if ? helps with that.
Things are amazing close to this now, thanks to the awesome work here by you and others! It really was day and night compared to hand rolling an FFI crate. I would love to see a similar approach to auto-generating FFI functions and C++ / otherlang bindings in the future.
I guess I'm not quite sure what the best practices as far as using wasm-bindgen are. Is it expected that one would re-wrap their existing library in a sort of LIBRARY-wasm crate? I would prefer to avoid that if possible. Other than this issue - the only other thing that got in my way of my laziness is not being able to automatically set a library type (cdylib) based on the target.
Ah ok, thanks for the info! I think this is definitely a use case we haven't really explored all that well, reusing existing crates as-is and exporting them to JS.
I don't think I'm opposed to doing this, and it may even be possible with a custom error strategy in wasm-bindgen itself (as we could just implement Into<JsValue> for that type).
If you' curious to give this a shot, want to test this out in a local patch and see how hard it would be to implement in wasm-bindgen? I suspect the implementation might actually be somewhat localized and easy to do test!
My one concern with allowing arbitrary : Into<JsValue> is that it ends up easily encouraging throwing non-Error objects on JS side, which is a serious antipattern as it loses entire JS backtrace.
E.g. I myself found out only by accident that I was throwing literally strings when doing Err(err.to_string().into()) for conversion, while what I really wanted is proper Error instances.
Hence, #1017 sounds like a better solution for this IMO.
That's a compelling rationale to me! I think we'd probably at least limit this to E: Error to start off with, perhaps with special casing of failure if necessary
This was fixed in #2710 (anything which implements Into<JsValue> is now allowed).
The docs need to be updated, as https://rustwasm.github.io/wasm-bindgen/reference/types/result.html still says only Result<T, JsValue> is supported. If I didn't stumble upon this PR, there would be no way of knowing this is now implemented.