input-output-hk / chain-wallet-libs

Apache License 2.0
18 stars 13 forks source link

create Error instances in cordova api #140

Open ecioppettini opened 3 years ago

ecioppettini commented 3 years ago

Currently we are throwing errors to cordova that are the text descriptions of the errors (e.g. https://github.com/input-output-hk/chain-wallet-libs/blob/ef877f6e239dcf920bdccf94149a969079fafe5d/bindings/wallet-core/src/error.rs#L341)

This keeps the cordova code simple, in a way, and it's fine for developing/testing, but not so much for handling production-time errors.

I think the proper way would be to extend javascript Error with custom instances, the problematic thing about this is that it requires that we define them manually in javascript, and keep them in sync with the C-level ones (or just have two different things).

I'm still not sure, what's the best way to tackle this though.

One option could be to change jni and objective-c code to return error pointers instead of strings, then do catch that ourselves at the plugin level, and make another cordova/abi roundtrip in javascript to extract the error description, name, error description, etc. And then deallocate the error.

Also, the other problem is that making this right probably requires doing the same for wallet-jni with java Exception extensions or something, although this makes things harder.

The other problem is that the browser/electron implementations don't even use the c error types, although no one is using this platforms anyway.

mzabaluev commented 3 years ago

It would be good to expose the error code: https://github.com/input-output-hk/chain-wallet-libs/blob/ef877f6e239dcf920bdccf94149a969079fafe5d/bindings/wallet-core/src/error.rs#L30

In JavaScript, the error object could have a code member with this integer code in addition to the message. For discrimination, the JavaScript API may define named constants for the error codes, matching the definitions on the Rust side.

Also, the other problem is that making this right probably requires doing the same for wallet-jni with java Exception extensions or something

How about wallet-jni defines an application domain exception class which features the same code in the idiomatic Java way?