Standardize the error codes from API endpoints #317
Conversation
common/errors.go
Outdated
| @@ -0,0 +1,39 @@ | |||
| package common | |||
There was a problem hiding this comment.
Should this just go in the api package?
There was a problem hiding this comment.
Good point! The only question is api is currently all protobuf stuff, it otherwise fits better in api since these are all around APIs. I'll move it over unless other folks have different thought/suggestion.
disperser/apiserver/server.go
Outdated
| @@ -93,12 +93,12 @@ func (s *DispersalServer) DisperseBlobAuthenticated(stream pb.Disperser_Disperse | |||
| // Process disperse_request | |||
| in, err := stream.Recv() | |||
| if err != nil { | |||
| return fmt.Errorf("error receiving next message: %v", err) | |||
| return common.NewInvalidArgError(fmt.Sprintf("error receiving next message: %v", err)) | |||
There was a problem hiding this comment.
nit: Is this really due to invalid argument? Feel like stream.Revc() breaking could be due to pod dying in the middle or something like that.
There was a problem hiding this comment.
good point, this may be not a clear cut whether it's client side or server side issue, as it may be caused by various reasons, like network connection, malformed message as well as server side resource issue etc.
I think overall it may fit better as client-side, as this is still at the gate of the system at this point.
| } | ||
|
|
||
| s.logger.Debug("metadataKey", "metadataKey", metadataKey.String()) | ||
| metadata, err := s.blobStore.GetBlobMetadata(ctx, metadataKey) | ||
| if err != nil { | ||
| return nil, err | ||
| // TODO: we need to distinguish NOT_FOUND from actual internal error. |
There was a problem hiding this comment.
Would probably need to standarize the error message and then check that here.
There was a problem hiding this comment.
Yep, we could check the message, or surface an error with NewNotFoundError inside the GetBlobMetaata (this approach was used in the Churner, L95 in churner/server.go)
common/errors.go
Outdated
| "google.golang.org/grpc/status" | ||
| ) | ||
|
|
||
| // The canonical errors from the EigenDA gRPC API endpoints. |
There was a problem hiding this comment.
We may add if it's becoming a more clear case to cover
Yep, I'll follow up to revamp the metrics to use these codes. |
Why are these changes needed?
Standardize the error codes to make it easier to understand, communicate, account and for better UX.
Checks