Repository navigation
fix: await response.code() and use sys.exc_info() in async gRPC interceptor - #1066
danishashko wants to merge 1 commit into
Conversation
grpc.aio.Call objects don't have .exception() (AttributeError on issue googleads#1059). response.code() is a coroutine in async gRPC and must be awaited.
ZLeventer
left a comment
ZLeventer
left a comment
There was a problem hiding this comment.
Good fix. Two real bugs here:
-
await response.code()—grpc.aio.Call.code()is a coroutine, unlike the syncgrpc.Future.code()which returns immediately. Calling it withoutawaitwould return a coroutine object, not the status code, causing thenot in _RETRY_STATUS_CODEScheck to always beTrue(a coroutine is never equal to a status code enum). -
sys.exc_info()[1]instead ofresponse.exception()—grpc.aio.Calldoesn't expose.exception()the same way syncgrpc.Futuredoes. Usingsys.exc_info()within theexceptblock is the correct approach since this method is always called from an exception handler.
Also good cleanup: removing the commented-out reasoning/notes and the unreachable raise response.exception() at the bottom. The import of _RETRY_STATUS_CODES from the module instead of redefining it locally is the right call — DRY and ensures any future changes to the retry set apply everywhere.
4 participants
Fixes #1059
Two bugs in
_AsyncExceptionInterceptor._handle_grpc_failure_async():Bug 1:
response.exception()raisesAttributeErrorgrpc.aio.Callobjects don't have an.exception()method. That method only exists on synchronousgrpc.Futureobjects. When a timeout or temporary service error triggers the async interceptor, callingresponse.exception()raisesAttributeError: 'UnaryUnaryCall' object has no attribute 'exception', masking the original gRPC error entirely.Fix: use
sys.exc_info()[1]to capture the live exception — this works since_handle_grpc_failure_asyncis always called from within anexcept grpc.RpcErrorblock in the async wrappers.Bug 2:
response.code()not awaitedIn
grpc.aio,Call.code()is a coroutine and must be awaited. Withoutawait,status_codewould be a coroutine object, making thestatus_code not in _RETRY_STATUS_CODEScheck always evaluate toTrue, meaning retry-able errors (INTERNAL, RESOURCE_EXHAUSTED) would incorrectly be parsed as GoogleAdsFailures instead of being re-raised as-is.Other cleanup:
_RETRY_STATUS_CODESfrominterceptor.pyinstead of redefining it locally