Skip to content

fix: retry transport-sourced Cancelled errors and count them in metrics - #1269

Merged
Sushisource merged 8 commits into
temporalio:mainfrom
brucearctor:fix/transport-cancelled-retry-and-metrics
May 18, 2026
Merged

Sushisource merged 8 commits into
temporalio:mainfrom
brucearctor:fix/transport-cancelled-retry-and-metrics

Conversation

@brucearctor

Copy link
Copy Markdown
Contributor

Summary

When Code::Cancelled originates from a transport-level failure (tonic::transport::Errorhyper::Error, e.g. connection closed during an AZ outage), the SDK now treats it as Unavailable-class and retries using the standard bounded backoff budget instead of the previous single-retry goaway workaround.

Additionally, transport-level failures that never produce a grpc-status HTTP header are now counted in the request_failure metric with a status_code=TRANSPORT_ERROR label.

Fixes #1252

Problem

Three compounding issues cause silent activity completion drops during sustained network turbulence:

  1. One-shot retry limit: The goaway-cancel workaround in retry.rs only allows a single retry. During sustained outages (not a single GOAWAY frame), that one retry burns immediately against a still-dead connection.

  2. Silent drop: After the retry exhausts, activities.rs catches the forwarded error and logs warn!("Network error while completing activity") — no metric, no callback, no error propagation.

  3. Metrics blind spot: GrpcMetricSvc::call() only increments request_failure when it can parse a grpc-status header from the HTTP response. Transport-killed calls return Err(...) at the HTTP level, so the counter is never incremented.

Changes

crates/client/src/retry.rs

  • Replaced the one-shot have_retried_goaway_cancel flag with is_transport_cancelled() — a helper that inspects the error source chain for tonic::transport::Errorhyper::Error
  • Transport-sourced Cancelled errors now flow through the standard backoff budget (same as Unavailable)
  • True application/caller-initiated cancellations (no transport error in source chain) remain non-retryable per gRPC spec

crates/client/src/metrics.rs

  • Added svc_request_failed_transport() method with status_code=TRANSPORT_ERROR label
  • GrpcMetricSvc::call() now counts Err(...) responses (transport failures) in addition to Ok(...) responses with non-OK grpc-status headers

Testing

  • All 11 existing retry unit tests pass
  • Clippy clean with -D warnings

@brucearctor
brucearctor requested a review from a team as a code owner May 14, 2026 03:05
When Code::Cancelled originates from a transport-level failure
(tonic::transport::Error → hyper::Error, e.g. connection closed during
an AZ outage), the SDK now treats it as Unavailable-class and retries
using the standard bounded backoff budget instead of the previous
single-retry goaway workaround.

This prevents silent activity completion drops during sustained network
turbulence, where the one-shot retry would exhaust immediately against
a still-dead connection and the completion would be silently dropped
with only a warn! log.

Additionally, transport-level failures that never produce a grpc-status
HTTP header are now counted in the request_failure metric with a
status_code=TRANSPORT_ERROR label, making them visible in dashboards.

Fixes temporalio#1252
@brucearctor
brucearctor force-pushed the fix/transport-cancelled-retry-and-metrics branch from 4bc1818 to d821fac Compare May 14, 2026 03:44

@Sushisource Sushisource left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor comment cleanup. Thanks a lot for this, the idea is a good one.

Comment thread crates/client/src/retry.rs Outdated
Comment thread crates/client/src/retry.rs Outdated
brucearctor and others added 2 commits May 14, 2026 17:18
Co-authored-by: Spencer Judge <sjudge@hey.com>
Co-authored-by: Spencer Judge <sjudge@hey.com>
@brucearctor

Copy link
Copy Markdown
Contributor Author

committed/accepted suggestions.

Thanks @Sushisource !

@brucearctor

Copy link
Copy Markdown
Contributor Author

I can't rerun, but seems like flaky CI test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dropped activity completions on transport-level Cancelled are invisible and not retried sufficiently

3 participants