fix: silent-stall RPC failures (DEVOP-594)

[gh-allora]

Fix RPC silent-stall failures (DEVOP-594)

Parent diagnostic: DEVOP-593
Fixes: DEVOP-594

What this fixes

~30% of testnet offchain-node pods (50/162 at time of investigation) had been silently stuck — Ready=True, restartCount=0, no error-level logs, but no successful tx in 24+ hours. Three compounding bugs prevented recovery from any RPC transport failure when only one gRPC endpoint is configured (which is true for all 467 production pods today).

Bug 1: ParseHTTPStatus regex never matched real gRPC errors

lib/errors.go:362 required the literal substring Status: to extract HTTP status codes. grpc-go surfaces upstream HTTP failures as unexpected HTTP status code received from server: NNN (Reason) — never matched. The 502/503/504-switching branch in triageHTTPStatusError was unreachable.

Bug 2: No string handler for common transport failures

connection reset by peer, unexpected EOF, and code = Unavailable desc = error reading from server: ... had zero handlers. All fell through to the info-level Unknown error catch-all in ProcessErrorTx and were treated as plain retryable errors → retry on the same dead conn → silent stall.

Bug 3: Single-endpoint switchToNodeLocked was a no-op

if len(nodes) == 1 {
    return &nodes[0], nil
}

Every deployed pod has exactly one nodeRpcs and one nodeGrpcs entry (verified across 467 snapshot configs in allora-offchain-config). So even if the switching path were reached, it accomplished nothing.

Changes (5 commits)

65a831c fix(errors): recognize gRPC transport-style HTTP status in ParseHTTPStatus
b32a4c7 fix(errors): treat gRPC transport failures as switching-node errors
61025f3 fix(errors): raise log level for unhandled RPC errors so alerts can fire
68b2d41 fix(connection): force re-dial when only one endpoint is configured
70204c6 fix(grpcclient): react to sustained Idle state and plug monitor leak

1. ParseHTTPStatus now recognizes both Status: NNN and HTTP status code received from server: NNN [(Reason)] phrasings. 3 new tests for real Cloudflare-fronted gRPC error strings.
2. isGRPCTransportError + ErrGRPCTransport (code 27): explicit handler in triageStringMatchingError for connection reset by peer, unexpected EOF, code = Unavailable desc = error reading from server: → returns ErrorProcessingSwitchingNode. IsErrorSwitchingNode updated.
3. Log level fix: log.Info().Err() "Unknown error" → log.Error(). Per-retry log in QueryDataWithRetry → log.Warn(). Alerting keyed on level=error now fires for these cases.
4. forceReconnectLocked replaces the single-node short-circuit. Rebuilds the chain client (GenerateNodeConfig), best-effort closes the old conn, replaces the NodeConfig slice entry in place under the existing write lock. GRPCReconnectionCount metric tracks success.
5. monitorGRPCConnection also reacts to sustained Idle state (30s+); InitializeGRPCClient returns a context.CancelFunc so closeOld can stop the per-conn monitor cleanly — no goroutine leak per reconnect.

Verification done locally

• go test ./... — all green
• go test -race ./... — all green (lock invariants for switchToNodeLocked / forceReconnectLocked hold)
• go build ./... — clean
• TestParseHTTPStatus — 13 cases pass (3 new for gRPC phrasings)
• TestIsGRPCTransportError — 8 cases pass

Rollout (suggested)

1. Merge → tag new image (e.g. v0.13.3)
2. Bump default tag in allora-internal-helm-charts/offchain-service
3. Deploy one testnet canary pod. Watch for 24h:
   • goroutine count flat across reconnects (no leak)
   • GRPCReconnectionCount metric increments when Cloudflare-induced transport errors occur
   • time-since-last-tx SLI stays under 30 min
4. Roll to full testnet
5. Roll to mainnet after 24-48h testnet soak

Out of scope (tracked in parent DEVOP-593)

• Adding a second gRPC fallback endpoint to allora-offchain-config
• Prometheus alert rule for time_since_last_tx > 30m
• Bypassing Cloudflare for chain gRPC traffic

Sibling PR

The same class of bugs (different shape) was found in allora-sdk-py. Companion PR: see DEVOP-595.

---

Summary by cubic

Fixes silent-stall RPC failures in allora-offchain-node by parsing gRPC/HTTP errors correctly, classifying transport failures, and forcing reconnects even with a single endpoint. Aligns with DEVOP-594 to restore automatic recovery and surface real errors for alerting.

• Bug Fixes

  • ParseHTTPStatus now reads gRPC transport-style messages (e.g., “unexpected HTTP status code… 502/503/504”) and preserves the full reason phrase.
  • Detect connection reset by peer, unexpected EOF, and code = Unavailable desc = error reading from server as transport errors that trigger node switching (ErrGRPCTransport, code 27; included in IsErrorSwitchingNode).
  • Single-endpoint “switch” now force re-dials the same endpoint, closes the old client and monitor, and increments GRPCReconnectionCount.
  • gRPC monitor reacts to sustained Idle (30s), ticks every 5s; InitializeGRPCClient returns a cancel function to stop the monitor and prevent goroutine leaks.
  • Raise log levels: unknown errors to error; per-retry logs to warn.

• Dependencies

  • Added .beads project metadata and updated .gitignore (no runtime impact).

^{Written for commit b5ecd9d. Summary will update on new commits.}

[linear]

DEVOP-594 Fix: allora-offchain-node silent-stall — gRPC error classification + single-endpoint recovery

allora-offchain-node — fix silent-stall RPC failures (PR draft ready)

Parent diagnostic: DEVOP-593 (DEVOP-593)

Scope

Code-only fix to allora-network/allora-offchain-node Go service that runs as the node container in all 467 production offchain pods. No helm chart, flux, or config changes.

Branch

fix/rpc-silent-stall (local in ~/src/allora-network/allora-offchain-node) — 5 commits, ~282 LoC across 7 files, all tests + race detector pass. Not pushed pending review.

65a831c fix(errors): recognize gRPC transport-style HTTP status in ParseHTTPStatus
b32a4c7 fix(errors): treat gRPC transport failures as switching-node errors
61025f3 fix(errors): raise log level for unhandled RPC errors so alerts can fire
68b2d41 fix(connection): force re-dial when only one endpoint is configured
70204c6 fix(grpcclient): react to sustained Idle state and plug monitor leak

What changes

1. ParseHTTPStatus now recognizes both Status: NNN and HTTP status code received from server: NNN [(Reason)] phrasings. 3 new test cases for real Cloudflare-fronted gRPC error strings.
2. New isGRPCTransportError helper + ErrGRPCTransport error code (27); explicit handler in triageStringMatchingError for connection reset by peer, unexpected EOF, code = Unavailable desc = error reading from server: → returns ErrorProcessingSwitchingNode. IsErrorSwitchingNode updated.
3. log.Info().Err(err).Msg("Unknown error") → log.Error(), and the per-retry log in QueryDataWithRetry → log.Warn() (was info). Existing alerting rules keyed on level=error will now fire.
4. Replaced the single-node short-circuit in switchToNodeLocked with forceReconnectLocked — rebuilds the chain client (GenerateNodeConfig), best-effort closes the old conn, replaces the NodeConfig slice entry in place under the existing write lock. GRPCReconnectionCount metric tracks success rate.
5. monitorGRPCConnection now also reacts to sustained Idle state (30s+); InitializeGRPCClient returns a context.CancelFunc so closeOld can stop the per-conn monitor goroutine cleanly — no leak per reconnect.

Verification done

• go test ./... — all green
• go test -race ./... — all green (lock invariants for switchToNodeLocked/forceReconnectLocked hold)
• go build ./... — clean
• TestParseHTTPStatus 13 cases pass (3 new for gRPC phrasings)
• TestIsGRPCTransportError 8 cases pass

Rollout

1. Push branch + open PR against dev
2. Merge → tag new image (e.g. v0.13.3)
3. Update helm chart in allora-internal-helm-charts/offchain-service to bump default tag
4. Deploy one testnet canary pod, watch for 24h: goroutine count flat across reconnects, GRPCReconnectionCount metric incrementing, time-since-last-tx SLI stays under 30 min
5. Roll to full testnet
6. Roll to mainnet after 24-48h testnet soak

Out of scope for this ticket

• Adding a second gRPC fallback endpoint to allora-offchain-config (tracked in parent diagnostic)
• Prometheus alert rule for time_since_last_tx > 30m (tracked in parent diagnostic)
• Bypassing Cloudflare for chain gRPC traffic (tracked in parent diagnostic)

Review in Linear

[gh-allora]

Self-review of fix/rpc-silent-stall

Reviewed by Hermes Agent at author's request. Verdict: conditional approve — code is correct and well-tested; one real bug + one architectural concern below that should be addressed before merge to mainnet (testnet canary can proceed with current code).

⚠️ Warnings

• lib/connection_manager.go:270 — forceReconnectLocked calls GenerateNodeConfig with context.Background(). The monitor goroutine spawned inside InitializeGRPCClient derives its lifetime from this context. If the process's parent context is later cancelled (graceful shutdown, SIGTERM), the original connection's monitor exits cleanly via closeOld's GRPCMonitorCancel, but any monitors spawned from reconnects continue running until process exit. The cancel-chain only stops on the next reconnect. Suggested fix: add a ctx context.Context field to ConnectionManager (saved during NewConnectionManager) and pass that here instead of Background(). Or document explicitly that reconnect monitors are process-lifetime by design and not tied to a parent.

• lib/grpcclient/grpc_connection.go:96 — Treating sustained connectivity.Idle (30s) the same as TransientFailure is correct for our use case (we set PermitWithoutStream: true, so a healthy keepalive should prevent Idle entirely), but modern grpc-go also has a separate IdleTimeout dial option (default 30 min in 1.55+) that can independently transition the conn to Idle. If that fires after low-traffic periods, the monitor will now force a reconnect every 30 min on otherwise-healthy connections. Worth confirming the in-use grpc-go version's IdleTimeout default, or explicitly setting grpc.WithIdleTimeout(0) to disable it.

💡 Suggestions

• lib/errors.go:319 — The new isGRPCTransportError branch is wedged between ErrorMessageConnectionRefused and ErrorReputerNonceWindowNotAvailable. Ordering is currently correct (none of the transport-error substrings overlap with adjacent matches), but the order-dependence is invisible from the surrounding code. Consider extracting the long if/else if chain into a table-driven dispatch, or at minimum add a comment at the top of triageStringMatchingError noting that branch ordering matters and adding a new branch requires checking adjacency.

• lib/errors.go:362 — The new regex captures more of the reason phrase than the old one did. Existing test case "With extra text" was updated from expected "Internal Server Error" to "Internal Server Error - more details" to reflect this. That's a beneficial behavior change but also a silent change to the public function's return value for inputs the old regex already matched. If anything downstream depends on the truncated form (substring matching on the returned message?), that consumer breaks. grep -r 'ParseHTTPStatus' ./... returns only the call site in triageHTTPStatusError and the tests, so the practical risk is zero — but worth noting in the PR description so reviewers don't miss it.

• lib/connection_manager.go:280 — Minor: the in-place slice mutation nodes[index] = *newNode is correct under the existing write-lock invariants, but a reader of the code in 6 months will have to derive that invariant from convention. A one-line // Caller holds queryMu/txMu — see switchToNodeLocked comment above the assignment would save them the trip.

✅ Looks good

• The GRPCMonitorCancel field + cancel-before-close ordering in closeOld cleanly solves the goroutine-leak class. Linear flow (cancel → close → nil) is easy to verify.
• isGRPCTransportError has both a positive substring set AND a negative test ("Unavailable alone is NOT a transport error") — the boundary between this handler and the HTTP-status handler is asserted, not implicit.
• Log-level changes (info → error on catch-all, info → warn on retry) are the minimum change needed to make existing level=error alerting rules work.
• The regex change preserves all 5 of the original TestParseHTTPStatus cases AND adds 3 real-production gRPC strings. Negative cases (Status: Not Found, Error: 404 Not Found, empty, abc, -404) are retained.
• Race detector clean across the whole package — the in-place NodeConfig replacement in forceReconnectLocked is the kind of code that often surfaces races, and the test suite doesn't trip them.

---

Self-review at author's request. Recommended action: address the context.Background() warning before mainnet rollout (testnet canary OK as-is). Suggestions are non-blocking polish.