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

Return status.Errorf instead of plain errors from gRPC functions #8619

Merged
merged 10 commits into from
Mar 17, 2021

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Mar 17, 2021

What type of PR is this?

Other

What does this PR do? Why is it needed?

It is not straightforward to reason about types of errors returned by gRPC functions when some code paths return status.Errorf and some return plain errors. The latter is correct when helper functions return status.Errorf, which is currently the case. It's also easier to make a mistake and use the wrong error type because there is no clear separation of responsibilities.

This PR simplifies things by always returning status.Errorf from top-level gRPC functions and plain errors from helper functions.

Which issues(s) does this PR fix?

N/A

Other notes for review

N/A

@rkapka rkapka added Ready For Review API Api related tasks labels Mar 17, 2021
@rkapka rkapka requested a review from a team as a code owner March 17, 2021 12:48
@rkapka rkapka marked this pull request as draft March 17, 2021 12:57
@rkapka rkapka marked this pull request as ready for review March 17, 2021 13:16
@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #8619 (8822228) into develop (034a287) will increase coverage by 0.07%.
The diff coverage is 28.57%.

@@             Coverage Diff             @@
##           develop    #8619      +/-   ##
===========================================
+ Coverage    61.28%   61.36%   +0.07%     
===========================================
  Files          488      488              
  Lines        34367    34307      -60     
===========================================
- Hits         21062    21052      -10     
+ Misses       10192    10146      -46     
+ Partials      3113     3109       -4     

@prylabs-bulldozer prylabs-bulldozer bot merged commit 0a73be7 into develop Mar 17, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix-grpc-error-type branch March 17, 2021 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants