chore(storage): replace Canceled status with context.Canceled #4896
Conversation
google-cla
bot
added
the
cla: yes
product-auto-label
bot
added
the
api: storage
| @@ -637,3 +642,11 @@ func read(r io.Reader, buf []byte) (int, bool, error) { | |||
| } | |||
| return recvd, done, err | |||
| } | |||
|
|
|||
| func checkCanceled(err error) error { | |||
| if status.Code(err) == codes.Canceled { | |||
There was a problem hiding this comment.
This seems like the right thing to do, but just to check: are we ever losing any information here by obscuring the difference between a user-side context cancelation and the RPC call returning codes.Canceled?
There was a problem hiding this comment.
We might be, but I'm not entirely sure. I don't think we would be since this error is induced by client-side request settings i.e. the user sets a deadline on the context, and the server notices that the deadline it received has expired. Nothing for the server to add there imo.
One other option would be to change how gax.APIError handles codes.Canceled and instead of storing the original grpc-go error, it stores a context.Canceled after parsing the Status. But that's a bit more far reaching and I'd be hesitant to do that.
The gRPC Storage client replaces
Canceledstatuses withcontext.Canceledto conform to what JSON does.Unskip the
CancelWriteGRPCtest by fixing theCanceledstatus handling and updating the test to provoke network calls using after cancelation, resulting inWriteerrors. See notes in #4772 WRT the context cancelation race condition behavior.Fixes #4772