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

StdError is not implemented for CacheLoadingError #3

Closed
Dessix opened this issue May 17, 2021 · 5 comments · Fixed by #5
Closed

StdError is not implemented for CacheLoadingError #3

Dessix opened this issue May 17, 2021 · 5 comments · Fixed by #5
Labels
enhancement New feature or request

Comments

@Dessix
Copy link
Contributor

Dessix commented May 17, 2021

Consider using thiserror or similar to support usage of generic rust error-handling libraries.

@Dessix
Copy link
Contributor Author

Dessix commented May 17, 2021

Additionally, it would be a nice benefit to allow generic parameterization of errors, such that an error in cache loading can be passed back to the caller with a known type.

@ByteAlex
Copy link
Owner

ByteAlex commented Jun 3, 2021

Hello @Dessix,
sorry I haven't seen your issue yet. I've started an implementation which handles errors in the loader. I'll see if it makes sense to extend it by a generic error parameter!

@ByteAlex ByteAlex added the enhancement New feature or request label Jun 3, 2021
@ByteAlex
Copy link
Owner

ByteAlex commented Jun 3, 2021

Your suggestion made me clean up a lot of the error handling and I implemented a solution in #5, would you please check if that fits your requirements? :)

@ByteAlex ByteAlex linked a pull request Jun 3, 2021 that will close this issue
@Dessix
Copy link
Contributor Author

Dessix commented Jun 3, 2021

It looks like it does all of what I wanted.
Potentially, a few helpers for the CacheLoadingError type may be warranted, such as into_loading_error(self) -> Option<E> / as_loading_error(&self) -> Option<&E> helpers. (This comment has been added to the PR as well, so you can close it there).

@ByteAlex
Copy link
Owner

ByteAlex commented Jun 3, 2021

Glad to hear that it solves the points you were missing!
Thank you for your feedback!

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

Successfully merging a pull request may close this issue.

2 participants