-
Notifications
You must be signed in to change notification settings - Fork 18
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
Refactor ResponseStatus #56
Comments
Agree with those three points! To lay it down: type Result<T> = Result<T, ErrorStatus>;
enum ResponseStatus {
Success,
Error(ErrorStatus),
}
enum ErrorStatus {
Service(ServiceError),
Psa(psa_crypto::types::status::Error),
}
enum ServiceError {
WrongProviderID,
...
} With That would also mean that we lose the automatic conversion to the integer representation but that is fine I think. We can implement a method for that, it would be cleaner as well in my opinion. Last point: do we agree on the names of each type? Do we want to use namespacing to trim some of those names? |
I think that makes sense! |
How about we drop the |
Totally agree with that! I would think that having a |
It seems the problem with the type hierarchy described in the first comment is that of converting from |
To be done with #35 |
The
ResponseStatus
enum is currently defined as a monolithic enum holding all possible values. While not entirely wrong, the usability of the type could be improved by re-arranging the variants into logical groups. This would also entail a refactoring of all types related to error management.Proposed changes:
Result<T>
type would still be available, however the error type would not be aResponseStatus
but a newErrorStatus
type; this is to prevent a logical error of having an error of typeSuccess
ResponseStatus
will be an enum with two variants:Success
andError(ErrorStatus)
ErrorStatus
will be further split intoService(ServiceError)
(for internal service-related errors) andPsa(Error)
(for errors related to the functionality of Parsec, generally returned from providers) - the error contained within the variant will be directly drawn from thepsa-crypto
Rust crateThe text was updated successfully, but these errors were encountered: