[jaideeppyne]

Problem

get_multi_with_metadata can report a cache miss or incomplete batch when the request actually failed:

• Its single-server path rescues every NetworkError into {}, including a transient RetryableNetworkError, without retrying.
• Its multi-server path already retries transient failures, but swallows a terminal failure and returns the other servers' metadata. Block callers receive that partial result too.
• get_multi_with_metadata_result inherits the same behavior.

This brings metadata bulk reads into line with the retry-and-raise contract adopted for the other bulk methods in #1149. The problem remains on current main 429d35f and is not covered by an open PR at submission.

Change

• Retry only RetryableNetworkError in the single-server metadata path; validate into a new array so retries do not apply the namespace twice.
• Let terminal NetworkError propagate on both paths. Preserve the multi-server path's existing handling of non-network DalliError.
• Document the observable error-contract change and add the requested changelog credit.

Compatibility: callers that previously treated {} or partial results as success on a terminal request failure must now rescue Dalli::NetworkError, as with other bulk operations. Finding no live server to route to at all remains unchanged. Real misses, tombstones, metadata shape, namespace removal and the adapter's return type are unchanged. No dependency or workflow changes are included.

Verification

Pure source head: 658073f2f1ab00da3a0dd0721335d99795f936a6.

Tests-first baseline: unchanged production against the exact base, Ruby 3.4 / memcached 1.6.27: 7 cases, 6 behavioral failures, 1 passing multi-server retry control, 0 errors/skips. The baseline and fixed revision use byte-identical regression tests.

The tests use real memcached for successful reads, CAS metadata, key routing and subsequent connection use, plus the existing server.stub(:request, ...) pattern for deterministic transient/terminal fault injection. They do not claim to reproduce a particular live TCP teardown timing. Cases cover single-server retry/namespace preservation, terminal failure, transient-to-terminal exhaustion, multi-server retry, partial-result rejection, block non-delivery and the CacheResult adapter.

Full fixed verification:

Runtime	memcached	bundle exec rubocop	bundle exec rake	Fresh-process regression repeats
Ruby 3.4.11	1.6.27	No offenses	976 runs, 0 failures/errors, 1 skip	20 x 7 passed
Ruby 3.4.11	1.6.45	No offenses	976 runs, 0 failures/errors, 1 skip	20 x 7 passed
Ruby 4.0.7	1.6.27	No offenses	976 runs, 0 failures/errors, 1 skip	20 x 7 passed
Ruby 4.0.7	1.6.45	No offenses	976 runs, 0 failures/errors, 1 skip	20 x 7 passed

All four exact source hashes, native summaries and 88 artifact log hashes were checked independently after download. Repeat processes use seeds 1-20 without retries: 560 additional case executions, no failures/errors/skips. No tracked-file changes during verification.

The first verification harness mistakenly left the downloaded memcached source inside the checkout; Rubocop parsed its RPM .spec as Ruby and stopped before tests. All reported offense paths were those generated third-party files. The corrected harness runs the unchanged installer outside the checkout, with the same full lint/test commands and no lint-rule or test relaxation. Earlier harness syntax/shallow-history errors also happened before baseline tests and are not presented as bug evidence.

Validation ran on disposable Linux GitHub runners, not on the author's host. Ruby head, 3.3, JRuby, TruffleRuby and other operating systems remain for upstream CI; this is not an all-platform claim.

AI Disclosure

This contribution was authored with OpenAI Codex. The implementation, tests and this description are AI-generated; no personal human review is claimed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-12T22:17:27.015622Z	658073f	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 658073f2f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Propagate terminal reconnect failures from single-server retries

With the default socket_max_failures: 2, if a connected memcached instance drops this request and then refuses the reconnect, the first I/O error closes the socket and raises RetryableNetworkError, but retry re-enters single_server; Base#alive? consumes the final NetworkError as false, so the method returns {}. This is the common “server remains unreachable after retrying” case that the new API contract says will raise, yet it still looks like a cache miss; the new tests inject exceptions without disconnecting the socket, so they do not cover it. Retry the already-selected server or otherwise propagate the failed liveness check.

Useful? React with 👍 / 👎.

[jaideeppyne]

Thank you for identifying the real reconnect path. The new regression injects the request exception while retaining a live socket, so it does not prove recovery after the connection is closed and alive? swallows the final reconnect failure.

The initial no-live-server behavior was deliberately left unchanged to match the other bulk APIs, but I agree that losing a server after an attempted request is a separate boundary. I am investigating a real disconnect/refused-reconnect fixture before proposing a follow-up, including whether sticky retry or current failover routing is the intended contract. This finding is not claimed fixed by the merged patch.

[chatgpt-codex-connector]

Surface terminal failures while regrouping a retried batch

With the default retry limit, if one selected server disconnects this request and then remains down, the outer retry starts again at groups_for_keys; Ring calls Base#alive?, which consumes the terminal reconnect NetworkError and either reroutes those keys via failover or drops their group. No request then reaches this rescue, and the method can still return fallback misses or a partial result despite the new contract. Preserve the failing group across the retry or propagate the terminal liveness failure instead.

Useful? React with 👍 / 👎.

[jaideeppyne]

Thanks. This is the multi-server counterpart of the single-server reconnect finding: regrouping can hide the already-attempted server failure before another request reaches the rescue clause.

I am checking both paths together with a wire-level disconnect fixture and the existing failover tests. No claim that this regrouping case is resolved yet; the follow-up needs to distinguish an initially unroutable key from a failed request without unintentionally disabling successful failover.

[petergoldstein]

Merged. Thanks for the fix, @jaideeppyne. The retry and error handling now match get_multi, and the tests cover the multi-server and block cases well.