fix: bound upstream request failures

[tma]

Summary

This changes how mbproxy handles stalled or failed upstream Modbus requests.

• Apply one end-to-end request budget across cache coalescing, queueing, connection setup, retries, and request pacing.
• Retry reads once after transport failures.
• Never retry writes when their outcome may be ambiguous.
• Preserve genuine upstream Modbus exception codes.
• Return 0x0B for upstream transport, framing, and deadline failures.
• Prevent expired requests from reaching the upstream socket.
• Prevent reads racing with writes from repopulating stale cache values.

Why

Previously, requests could wait indefinitely for the serialized upstream connection and then execute after the downstream caller had already timed out. Reads and writes were retried using the same policy, and all final failures became exception 0x04.

That made stalled upstream connections difficult to handle safely, particularly for writes where the device may have applied the command before the response was lost.

Configuration

MODBUS_ATTEMPT_TIMEOUT is now the preferred per-attempt socket timeout.

The existing MODBUS_TIMEOUT variable remains supported as a deprecated alias, so existing configurations continue to work. If both variables are set, their parsed durations must match.

MODBUS_REQUEST_TIMEOUT controls the independent end-to-end request budget. Defaults are:

• MODBUS_ATTEMPT_TIMEOUT=10s
• MODBUS_REQUEST_TIMEOUT=30s

The total request budget always caps individual attempts.

Error handling

• Real upstream Modbus exceptions keep their original exception code.
• Upstream transport, framing, and total deadline failures return 0x0B.
• Local validation keeps the standard Modbus validation exception codes.
• Internal mbproxy failures return 0x04.

Cache safety

Allowed writes invalidate affected cache entries before and after forwarding. A write generation prevents older in-flight reads from caching values obtained before a concurrent write.

Testing

• Docker Go 1.24 format and vet checks
• Full race-enabled test suite
• Repeated cancellation and concurrency tests
• Loopback TCP tests for deadlines, reconnects, malformed responses, retries, and write safety
• docker build --target test .

The structured diagnostics and degraded-health changes are intentionally kept out of this PR.

[Copilot]

Pull request overview

This PR tightens mbproxy’s handling of stalled/failed upstream Modbus operations by introducing a single end-to-end request budget, classified error handling (including preserving real Modbus exceptions), and safer cache behavior under read/write concurrency.

Changes:

• Add request-level timeout enforcement in the Modbus TCP server and propagate it through proxy/cache/client paths.
• Rework upstream client behavior to classify failures, retry reads once after transport failures, and map transport/deadline failures to 0x0B while preserving genuine Modbus exception codes.
• Harden cache correctness around concurrent writes via write-generation gating, plus update docs and expand tests for deadlines/cancellation/retry/cache safety.

Show a summary per file

File	Description
SPEC.md	Updates documented timeout/retry/error-mapping semantics and cache write-generation behavior.
README.md	Documents new/renamed timeout env vars and the updated exception mapping/pacing model.
internal/proxy/proxy.go	Adds request validation, write-generation cache safety, and passes request timeout into the server.
internal/proxy/proxy_test.go	Adds tests for validation short-circuiting, stale-serving rules, and write/read interleavings.
internal/modbus/server.go	Enforces per-request timeout and maps handler failures to appropriate Modbus exceptions.
internal/modbus/server_test.go	Adds regression tests for preserving upstream exceptions and mapping deadlines to 0x0B.
internal/modbus/errors.go	Introduces error classification and downstream exception mapping logic.
internal/modbus/connection_test.go	Adds tests ensuring canceled/expired requests don’t write to the upstream socket and deadlines can’t be extended.
internal/modbus/client.go	Major client refactor: ownership serialization, pre-wire pacing, bounded attempts, read-only retry-on-transport, and deadline guarding.
internal/modbus/client_test.go	Adds extensive tests for retry policy, pacing semantics, cancellation/deadline behavior, and error mapping.
internal/config/config.go	Adds MODBUS_ATTEMPT_TIMEOUT + MODBUS_REQUEST_TIMEOUT with migration handling for deprecated MODBUS_TIMEOUT.
internal/config/config_test.go	Expands coverage for new timeout env vars and migration/validation rules.
internal/cache/cache.go	Adjusts coalescing behavior to better handle leader cancellation/deadline and follower retry behavior.
internal/cache/cache_test.go	Adds tests validating coalescing behavior under cancellation and retry-after-leader-expiry.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 14/14 changed files
• Comments generated: 0
• Review effort level: Low