-
Notifications
You must be signed in to change notification settings - Fork 327
Add error::ResultDynErrExt #216
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
Conversation
08937a1
to
a981059
Compare
Seeking for code review |
@updogliu thanks for this PR! -- I think I'm missing a bit of context in how this would be used. Could you perhaps provide a (small) example? |
The added trait Specifically, when a step of processing the request returns a # Example:
async fn my_endpoint(mut cx: Context<State>) -> EndpointResult<u8> {
let result: Result<u8, Box<dyn std::error::Error + Send + Sync>> = process(cx);
result.client_err()?
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after the rebase!
Thanks. Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks!
Add
error::ResultDynErrExt
, which provide the same set of methods inerror::ResultErr
forResult<T, Box<dyn Error>>
.Motivation and Context
Trying to resolve issue #208. With this PR, a
e: Result<T, Box<dyn Error>>
can calle.client_err()
,e.server_err()
conveniently.This introduces duplicate codes with
error::ResultErr
. But I did not figure out a way to avoid that.How Has This Been Tested?
Not tested - mostly just copy and paste
error::ResultErr
, which does not have a test case for now.I have no problem adding a unit test if necessary.
Types of changes
Checklist: