fix(api): isRetryableError in volume_util.go does not recognize gRPC codes.Unavailable and codes.DeadlineExceeded
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 438
- PR merge metrics
- No merged PRs in 30d
Description
Problem
In packages/api/internal/handlers/volume_util.go, executeOnOrchestratorByClusterID coordinates volume operations (CreateVolume, DeleteVolume) across orchestrator nodes in a cluster. When an RPC to a node fails, the loop calls isRetryableError(err) to decide whether to log the error and retry the operation against the next candidate node in the cluster pool.
However, isRetryableError only inspects:
func isRetryableError(err error) bool {
if errors.Is(err, net.ErrClosed) || errors.Is(err, context.DeadlineExceeded) {
return true
}
return false
}
When an orchestrator node daemon is temporarily unreachable (e.g. host reboot, daemon restarting, connection refused, or node timeout), gRPC transport errors are wrapped in status.Status with codes:
codes.Unavailable(desc = "connection error: desc = transport: Error while dialing: dial tcp ... connect: connection refused")codes.DeadlineExceeded(desc = "context deadline exceeded")codes.Canceled(desc = "context canceled")
Because errors.Is(err, net.ErrClosed) does not unwrap gRPC status.Error instances, isRetryableError(err) returns false. As a result, the loop immediately terminates on the very first offline or restarting node, failing the volume creation/deletion request and returning an HTTP 500 error to the client, even when multiple healthy candidate orchestrator nodes are ready and available in the same cluster.
Root Cause
isRetryableError failed to inspect gRPC status errors using status.FromError(err) and match transient gRPC error codes:
| Error Type | Example | Current isRetryableError |
Expected isRetryableError |
|---|---|---|---|
net.ErrClosed |
Local socket close | true |
true |
context.DeadlineExceeded |
Context timeout | true |
true |
status.Error(codes.Unavailable) |
Node offline / connection refused | false (Fail fast, no retry) |
true (Retry next node) |
status.Error(codes.DeadlineExceeded) |
gRPC call timeout | false (Fail fast, no retry) |
true (Retry next node) |
status.Error(codes.Canceled) |
gRPC call canceled | false (Fail fast, no retry) |
true (Retry next node) |
status.Error(codes.InvalidArgument) |
Bad volume parameter | false |
false |
Reproduction Steps
- Configure a cluster with two orchestrator nodes:
node-1(stopped/offline) andnode-2(running/healthy). - Issue a volume creation request:
POST /volumes. executeOnOrchestratorByClusterIDselectsnode-1first.- The gRPC call fails with
status.Error(codes.Unavailable, "connection refused"). - Observed:
isRetryableErrorreturnsfalse, causing the loop to abort immediately and return an error without tryingnode-2. - Expected:
isRetryableErrorreturnstrue, logs the retry onnode-1, and successfully executes onnode-2.
Technical Context
- File affected:
packages/api/internal/handlers/volume_util.go - Subsystem: Control Plane API / Volumes / High Availability & Node Failover
- Impact: High (Cluster node failover reliability and volume provisioning resilience)
Proposed Changes
| # | Change | File(s) Affected | Complexity |
|---|---|---|---|
| 1 | Add status.FromError(err) inspection in isRetryableError matching codes.Unavailable, codes.DeadlineExceeded, codes.Canceled |
volume_util.go |
Trivial |
| 2 | Add comprehensive table-driven unit tests for isRetryableError in volume_util_test.go |
volume_util_test.go |
Low |
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/api/internal/handlers/volume_util.go at isRetryableError and review how executeOnOrchestratorByClusterID uses its result. Then inspect or add the table-driven tests in packages/api/internal/handlers/volume_util_test.go for the listed network, context, and gRPC status errors. Done means transient node failures are retryable while InvalidArgument remains non-retryable, with the tests passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100