Return error when fetching the snapshot fails #233
Conversation
| @@ -169,6 +169,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) | |||
| if resp != nil && resp.StatusCode == http.StatusNotFound { | |||
| return nil, status.Errorf(codes.NotFound, "snapshot %q not found", snapshotID) | |||
| } | |||
| return nil, err | |||
There was a problem hiding this comment.
We do not seem to return a proper gRPC status code in similar code paths either, so I copied the pattern. Not sure if good or not, my expectation/hope would be that gRPC maps this to a generic "internal error" or similar.
@adamwg you may know this one better than me.
There was a problem hiding this comment.
I believe the go gRPC framework wraps non-gRPC errors from RPC handlers using the Unknown status code. I always prefer to return explicit gRPC errors with appropriate codes from handlers, but gRPC is OK with it either way.
Makes sense to follow the general pattern here, and if we want to return explicit Internals everywhere we can do that in a separate PR.
There was a problem hiding this comment.
Makes sense to me -- will keep that in mind for a follow up PR.
|
I added |
timoreimann
force-pushed
the
return-error-when-fetching-a-snapshot-fails
branch
2 times, most recently
from
7c2708e
to
ed1d4c3
Compare
We previously continued processing when fetching the snapshot failed for errors other than 404.
timoreimann
force-pushed
the
return-error-when-fetching-a-snapshot-fails
branch
from
ed1d4c3
to
8e89fc3
Compare
We previously continued processing when fetching the snapshot failed for errors other than 404.