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

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

Open
daira opened this issue Aug 25, 2022 · 3 comments
Open

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

daira opened this issue Aug 25, 2022 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@daira
Copy link
Contributor

daira commented Aug 25, 2022

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 zcash/zcash#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 #35 (comment)

@daira daira added the enhancement New feature or request label Aug 25, 2022
@str4d
Copy link
Contributor

str4d commented Aug 25, 2022

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
Copy link
Contributor Author

daira commented Aug 27, 2022

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
Copy link
Contributor

pacu commented Aug 28, 2022

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.

@daira daira added this to the UniFFI milestone Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants