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

Add better WASM errors #146

Closed
7 tasks
l1h3r opened this issue Mar 2, 2021 · 2 comments
Closed
7 tasks

Add better WASM errors #146

l1h3r opened this issue Mar 2, 2021 · 2 comments
Assignees
Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog

Comments

@l1h3r
Copy link
Contributor

l1h3r commented Mar 2, 2021

Description

Add better error messages to the wasm bindings - the current implementation doesn't provide a stack trace.

Currently most functions return Result<T, JsValue> and this gives an error message but the context is lost. The Error type from the js-sys crate should be used instead: Result<T, js_sys::Error>

Resources

  • js-sys
    • Should already be a version include by wasm-bindgen (check Cargo.lock) but will need to be included to access the API.

To-do list

  • Replace Result<T, JsValue> functions with Result<T, js_sys::Error> wherever appropriate

Change checklist

  • The feature or fix is implemented in Rust and across all bindings whereas possible.
  • The feature or fix has sufficient testing coverage
  • All tests and examples build and run locally as expected
  • Every piece of code has been document according to the documentation guidelines.
  • If conceptual documentation (mdbook) and examples highlighting the feature exist, they are properly updated.
  • If the feature is not currently documented, a documentation task Issue has been opened to address this.
@l1h3r l1h3r added good first issue Wasm Related to Wasm bindings. Becomes part of the Wasm changelog Added A new feature that requires a minor release. Part of "Added" section in changelog labels Mar 2, 2021
@l1h3r l1h3r added Help wanted Extra attention is needed and removed Help wanted Extra attention is needed labels Mar 17, 2021
@JelleMillenaar
Copy link
Collaborator

I believe this PR dependent on an improvement within wasm-bindgen that had an open PR. @PhilippGackstatter can you sync with @HenriqueNogara on that?

@PhilippGackstatter
Copy link
Contributor

Unless I'm mistaken, this was basically fixed by #344. Stacktrace support is also there now, for example:

const { doc, key } = new Document(KeyType.Ed25519, "invalid-network");

gives:

Err > InvalidNetworkName: Invalid Network Name: network name cannot exceed 6 characters
    at module.exports.__wbg_new_80811dcb66d1b53f (~/git/identity.rs/bindings/wasm/node/identity_wasm.js:4009:15)
    at js_sys::Error::new::he73bdbceea59ac14 (<anonymous>:wasm-function[5359]:0x31937b)
    at identity_wasm::error::wasm_error::h2992dc7d9d8702bc (<anonymous>:wasm-function[1645]:0x277fef)
    at identity_wasm::did::wasm_document::WasmDocument::new::ha36dfed4e964f01f (<anonymous>:wasm-function[418]:0x17ac2e)
    at document_new (<anonymous>:wasm-function[3069]:0x2ee198)
    at new Document (~/git/identity.rs/bindings/wasm/node/identity_wasm.js:1358:24)
    at o (~/git/identity.rs/bindings/wasm/examples/dist/node.js:1:338)
    at ~/git/identity.rs/bindings/wasm/examples/dist/node.js:1:7333
    at ~/git/identity.rs/bindings/wasm/examples/dist/node.js:1:8320
    at Object.<anonymous> (~/git/identity.rs/bindings/wasm/examples/dist/node.js:1:8396)

In particular, the returned error is also an instanceof Error.

I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

No branches or pull requests

4 participants