Retry remote calls a proxy fails with 502, 503 or 504

[senamakel]

A deploy restarted tinymold serve behind nginx during chottu-core's save step. nginx answered the Write in flight with 502 Bad Gateway and an HTML body, which tonic reports as:

code: 'Internal error', message: "protocol error: received message with invalid compression flag: 60 (valid flags are 0 and 1) while receiving response with status: 502 Bad Gateway"

push treated that as a refusal and gave up the whole push. A 502, 503 or 504 from a proxy now goes through the same 1/4/15 s retries as UNAVAILABLE. Together with the h2 GOAWAY case (#48), the check moves into proxy_hiccup.

Behavior change: INTERNAL statuses carrying a proxy 502/503/504 are retried. Other HTTP statuses (e.g. 400) still fail at once.

Validation

• Regression test a_proxy_whose_server_is_restarting_is_retried (502/503/504 retried, 400 not) fails before the fix and passes after.
• cargo test -p tinymold-remote: 85 pass. Clippy on the crate and fmt --check are clean.
• The full workspace contract runs in CI: the Mac I'm on is at ~3 GB free, which is too little for a full --all-targets build.

[tinysweeper]

Tiny Sweeper review

Tiny Sweeper completed its review; deterministic results follow.

State: Ready for maintainer review
Priority: none
Reviewed head: a452ff648591
Updated: 2026-10-15T22:46:24Z

Review snapshot

Change surface	Files	Review signal	Count
Production	1	Active findings	0
Tests	1	Noted findings	0
Documentation	1	Resolved findings	0
Configuration	0	Pending checks/questions	0

Completeness: Complete
Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

`transient` now delegates the `INTERNAL`-status classification to a new `proxy_hiccup(message: &str) -> bool` helper in `crates/tinymold-remote/src/remote/mod.rs`. The helper preserves the existing `starts_with("h2 protocol error")` check and adds a `contains` match for u201cwhile receiving response with status: 502 ”, 503, or 504 (the trailing space is required). The comment on `transient` is reworded to reference [`proxy_hiccup`] and the connection-close explanation moves to the helper's doc comment. `docs/specs/remote.md` documents the new retry behavior, and `crates/tinymold-remote/src/remote/test.rs` gains `a_proxy_whose_server_is_restarting_is_retried` covering the 502/503/504 positives and a 400 negative.

Features

• Modified — Proxy 502/503/504 responses classified as transient by `proxy_hiccup`: An `INTERNAL` status whose message contains u201cwhile receiving response with status: 502 ”, 503, or 504 is now retried instead of failing immediately; other `INTERNAL` statuses still return false, and the pre-existing h2-protocol-error prefix check is preserved. (crates/tinymold-remote/src/remote/mod.rs#fn transient(error: &Error) -> bool {, crates/tinymold-remote/src/remote/mod.rs#where, crates/tinymold-remote/src/remote/mod.rs, crates/tinymold-remote/src/remote/test.rs)
• Added — `proxy_hiccup` helper: A dedicated predicate encapsulates the `INTERNAL`-message matching (h2 protocol error or a 502/503/504 status suffix) so `transient` remains a thin classification of tonic error codes. (crates/tinymold-remote/src/remote/mod.rs, crates/tinymold-remote/src/remote/mod.rs#fn transient(error: &Error) -> bool {)
• Modified — Documentation of proxy-failure retries: The remote spec now states that a 502, 503, or 504 from a proxy is retried and describes the non-gRPC body the proxy returns while the server behind it restarts, aligning spec text with the code. (docs/specs/remote.md, docs/specs/remote.md#A call that times out, or that the server answers as `UNAVAILABLE`,)

Tests

• unit — `a_proxy_whose_server_is_restarting_is_retried` asserts `transient` returns true for `INTERNAL` messages containing 502, 503, and 504 and false for a 400 message, using the same invalid-compression-flag message shape.: Covers both directions of the new predicate with a failing-if-broken assertion for each; per the tests lane it does not exercise the `retrying` retry path end-to-end for a proxy status, but that is the pre-existing test shape rather than something this change altered. (crates/tinymold-remote/src/remote/test.rs#fn a_connection_the_server_closed_is_retried() {, crates/tinymold-remote/src/remote/test.rs)

Findings

No active actionable findings.

Before merge

None.

How this fits together

flowchart LR
  n0["a_connection_the_server_closed_is_retried<br/>changed"]:::changed
  n1["new"]:::impacted
  n2["remote"]:::impacted
  n3["with"]:::impacted
  n4["blob"]:::impacted
  n5["configured"]:::impacted
  n6["..._entry_whose_record_is_swapped_is_refused"]:::impacted
  n0 -->|calls| n1
  n0 -->|tests| n1
  n2 -->|calls| n1
  n5 -->|calls| n1
  n6 -->|calls| n1
  n6 -->|tests| n1
  n6 -->|calls| n3
  n6 -->|tests| n3
  n6 -->|calls| n4
  n6 -->|tests| n4
  n6 -->|calls| n5
  n6 -->|tests| n5
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

Loading

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: Reviewed 3 files; 0 findings. _The code index is behind this pull request (indexed at `06094be00138`), so retrieved context may be out of date._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: No untrusted input reaches a dangerous sink: the matched status message is produced by tonic, and treating it as retryable only causes an extra retry, never a trust decision or data write; the trailing-space requirement after the status code matches the test's message shape and the pre-existing h2 prefix check is preserved.
• Lane summary: The change adds a `proxy_hiccup` helper that widens what counts as a transient `INTERNAL` status to include tonic's reports of a proxy (502/503/504 with a non-gRPC body) failing a call, and documents it. No untrusted input reaches a dangerous sink here: the status message is produced by tonic, and treating it as retryable only causes an extra retry, never a trust decision or data write. The matcher requires the trailing space after the status code, so the shape in the test (`... with status: 502 ...`) matches, and the pre-existing `h2 protocol error` prefix check is preserved. No findings. 1 file was not security-reviewed: docs/specs/remote.md (prose or tabular data). _The code index is behind this pull request (indexed at `06094be00138`), so retrieved context may be out of date._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change makes `transient` recognise a second class of `INTERNAL` status — a 502/503/504 from a proxy — and pins it with `a_proxy_whose_server_is_restarting_is_retried`, which asserts the positive cases (502, 503, 504) and the negative (400). Both directions of the new predicate have a failing-if-broken assertion, and the existing h2 case and the `starts_with`/`contains` boundary are covered. No defect found; the widest remaining gap is that the retry path itself (`retrying`) is only exercised end-to-end for the timeout case, not for a proxy status, but that is the pre-existing test shape, not something this diff changed. _The code index is behind this pull request (indexed at `06094be00138`), so retrieved context may be out of date._

commits

• Conclusion: Neutral
• Scope reviewed: all assigned evidence
• Lane summary: Nothing sensitive found in what this pull request commits.

description

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The description matches the diff: the retry check is factored into `proxy_hiccup`, extended to 502/503/504 messages, tested including the 400 negative, and documented in the spec, with the stated behavior change matching the code.
• Lane summary: The description matches the diff: the retry check for tonic `INTERNAL` statuses is factored into `proxy_hiccup`, extended to messages containing `while receiving response with status: 502/503/504 `, tested by `a_proxy_whose_server_is_restarting_is_retried` (including the 400 negative case), and documented in `docs/specs/remote.md`. The stated behavior change (502/503/504 retried, other statuses still fail at once) is exactly what the code does. I checked the one thing worth verifying — the concrete tonic message format the substring match relies on, which is the documented reproduction from the PR itself and is pinned in the new test — and found nothing that would make the match fail on the reported input. _The code index is behind this pull request (indexed at `06094be00138`), so retrieved context may be out of date._

Evidence and run details

• Models: ladder/vectors-oai3, deepseek-v4.1-flash
• Spend: $0.003618
• Tokens: 188565 input · 20569 output · 48384 cached · 199 embedding

Head	State	Pass summary
a452ff648591	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-15T22:46:24Z)

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.75.}

Or wait 53 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 740c20ef-6146-4618-8894-0ced12601d3f

📥 Commits

Reviewing files that changed from the base of the PR and between e760c4d and a452ff6.

📒 Files selected for processing: 3

• crates/tinymold-remote/src/remote/mod.rs
• crates/tinymold-remote/src/remote/test.rs
• docs/specs/remote.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[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-15T22:45:04.976520Z	a452ff6	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.

[tinysweeper]

tinysweeper found nothing blocking. Approving.

             $0.0036 · 188,565 in / 20,569 out · 48,384 cached (26%) · ladder/vectors-oai3, deepseek-v4.1-flash · 199 embedded
critique:    $0.0032 · 105,082 in / 17,619 out · 20,608 cached (20%) · deepseek-v4.1-flash
security:    $0.0002 · 33,956 in  / 760 out    · 4,864 cached (14%)  · deepseek-v4.1-flash
tests:       $0.0000 · 32,152 in  / 500 out    · 15,488 cached (48%) · deepseek-v4.1-flash
description: $0.0000 · 14,399 in  / 461 out    · 7,424 cached (52%)  · deepseek-v4.1-flash