fix: don't return internal error for data not found #1821
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf Proto / buf (pull_request).
|
| @@ -188,8 +188,12 @@ func (s *Server) GetBlob(ctx context.Context, request *pb.GetBlobRequest) (*pb.G | |||
| keys := []v2.BlobKey{key} | |||
| mMap, err := s.metadataProvider.GetMetadataForBlobs(ctx, keys) | |||
| if err != nil { | |||
| return nil, api.NewErrorInternal(fmt.Sprintf( | |||
| "error fetching metadata for blob, check if blob exists and is assigned to this relay: %v", err)) | |||
| if strings.Contains(err.Error(), blobstore.ErrMetadataNotFound.Error()) { | |||
There was a problem hiding this comment.
Shouldn't this also check for ErrBlobNotFound?
There was a problem hiding this comment.
I looked at the code, and it looks like ErrBlobNotFound cannot be returned here, since this is only fetching metadata. I did a search for uses of ErrBlobNotFound, and made those places return the correct error code though.
| @@ -307,8 +311,12 @@ func (s *Server) GetChunks(ctx context.Context, request *pb.GetChunksRequest) (* | |||
|
|
|||
| mMap, err := s.metadataProvider.GetMetadataForBlobs(ctx, keys) | |||
| if err != nil { | |||
| return nil, api.NewErrorInternal(fmt.Sprintf( | |||
| "error fetching metadata for blob, check if blob exists and is assigned to this relay: %v", err)) | |||
| if strings.Contains(err.Error(), blobstore.ErrMetadataNotFound.Error()) { | |||
There was a problem hiding this comment.
similar question, shouldn't we check for some error like "ChunkNotFound"?
There was a problem hiding this comment.
Still need to do this, will push another commit
There was a problem hiding this comment.
Hmm... I'm actually not sure what strings are returned when there is a "not found" error for chunk data. Would it be ok to handle this in a future PR?
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Why are these changes needed?
Avoid returning internal errors when data can't be found.
https://linear.app/eigenlabs/issue/EGDA-1797/dont-return-internal-error-when-data-not-found