fix: Re-raise python asyncio.CancelledErrors (#31982)

This is motivated by #31227 which I also encountered recently. I'm not familiar with the call lifecycle details, but it seems clear that there is some kind of race condition between user-initiated cancellations and other completion status updates.

If a user cancels a call before being notified of its completion, I believe it should be considered cancelled unconditionally, regardless of whether the call actually completed successfully before its cancellation status could be set.

So it seems better to re-raise explicitly rather than using self._raise_for_status(), which is a no-op in this race condition and can lead to an UnboundedLocalError.
pull/32014/head^2
Nick Hill 2 years ago committed by GitHub
parent aa83319695
commit 353afc2dd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 7
      src/python/grpcio/grpc/aio/_call.py

@ -341,7 +341,7 @@ class _StreamResponseMixin(Call):
except asyncio.CancelledError:
if not self.cancelled():
self.cancel()
await self._raise_for_status()
raise
if raw_response is cygrpc.EOF:
return cygrpc.EOF
@ -449,7 +449,7 @@ class _StreamRequestMixin(Call):
except asyncio.CancelledError:
if not self.cancelled():
self.cancel()
await self._raise_for_status()
raise
async def _done_writing(self) -> None:
if self.done():
@ -463,7 +463,7 @@ class _StreamRequestMixin(Call):
except asyncio.CancelledError:
if not self.cancelled():
self.cancel()
await self._raise_for_status()
raise
async def write(self, request: RequestType) -> None:
self._raise_for_different_style(_APIStyle.READER_WRITER)
@ -602,6 +602,7 @@ class StreamUnaryCall(_StreamRequestMixin, _UnaryResponseMixin, Call,
except asyncio.CancelledError:
if not self.cancelled():
self.cancel()
raise
if self._cython_call.is_ok():
return _common.deserialize(serialized_response,

Loading…
Cancel
Save