Electric-Coin-Company / zcash-light-client-ffi

Light Client FFI Layer for librustzcash
MIT License
6 stars 16 forks source link

Replace thread-local stateful error handling with use of Results #37

Open daira opened 2 years ago

daira commented 2 years ago

Currently this crate uses stateful error reporting with a thread-local variable to hold the error state (implemented by the ffi_helpers::error_handling module).

Once we have https://github.com/zcash/zcash/issues/4816 (Define a "result" type for use in C++ error handling) we should replace this error handling with that. [Edit: that's a zcashd ticket, not directly relevant to this repo.]

_Originally posted by @daira in https://github.com/zcash-hackworks/zcash-light-client-ffi/pull/35#discussion_r955125130_

str4d commented 2 years ago

That type will be useless here, as this FFI goes to Swift, not C++. And in any case, errors here will be handled however UniFFI handles them.

daira commented 2 years ago

We decide how they are reported even when using UniFFI, no? There's nothing about the Result style of reporting that is incompatible with UniFFI, I think. (But yes you're right, it won't use C++ expected.)

pacu commented 2 years ago

This is a very important problem to solve. The FFI throws a myriad of unstructured errors that are hard to classify and handle, that pours out to the client code and makes error handling a nightmare and represents one of the most important developer experience problems we have on the wallet team.

Swift is designed to interop with C and C++ so I don't think it would be far much more complicated than what we do now. I don't know if it would be of any help for the JNI side of things. Probably an effort that is better spent on leveraging a tool like UniFFi or similar.