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

crates/tinymold-remote/src/remote/mod.rs

@@ -629,9 +629,8 @@ where
 }
 
 /// Whether `error` may not happen again: a call that timed out, a server
-/// that said it is unavailable, overloaded or aborted the call, or a
-/// connection it closed under the call (an HTTP/2 GOAWAY, which a proxy such
-/// as nginx sends after its per-connection request limit; the channel
+/// that said it is unavailable, overloaded or aborted the call, or a proxy
+/// in front of it that failed the call (see [`proxy_hiccup`]; the channel
 /// reconnects). A refusal, such as a missing write credential, is not.
 fn transient(error: &Error) -> bool {
     match error {
@@ -643,13 +642,24 @@ fn transient(error: &Error) -> bool {
                     | tonic::Code::DeadlineExceeded
                     | tonic::Code::ResourceExhausted
                     | tonic::Code::Aborted
-            ) || (status.code() == tonic::Code::Internal
-                && status.message().starts_with("h2 protocol error"))
+            ) || (status.code() == tonic::Code::Internal && proxy_hiccup(status.message()))
         }
         _ => false,
     }
 }
 
+/// Whether an `INTERNAL` status's `message` is tonic's report of a proxy
+/// in front of the server failing the call rather than the server refusing
+/// it: a connection closed under the call (an HTTP/2 GOAWAY, which nginx
+/// sends after its per-connection request limit), or a 502, 503 or 504 with
+/// a body that is not gRPC (nginx while `tinymold serve` restarts).
+fn proxy_hiccup(message: &str) -> bool {
+    message.starts_with("h2 protocol error")
+        || ["502", "503", "504"]
+            .iter()
+            .any(|code| message.contains(&format!("while receiving response with status: {code} ")))
+}
+
 /// The digest of the output file at `path` in `result`.
 fn find_file(result: &ActionResult, path: &str, key: &proto::Digest) -> Result<proto::Digest> {
     result

crates/tinymold-remote/src/remote/test.rs

@@ -374,6 +374,29 @@ fn a_connection_the_server_closed_is_retried() {
     assert!(!transient(&status("the store is broken")));
 }
 
+#[test]
+fn a_proxy_whose_server_is_restarting_is_retried() {
+    // nginx answers 502 while `tinymold serve` restarts (a deploy), with an
+    // HTML body tonic cannot parse as gRPC; 503 and 504 are the same story.
+    let status = |message: &str| Error::Status {
+        call: "Write",
+        status: Box::new(tonic::Status::internal(message.to_owned())),
+    };
+    for code in [
+        "502 Bad Gateway",
+        "503 Service Unavailable",
+        "504 Gateway Timeout",
+    ] {
+        let message = format!(
+            "protocol error: received message with invalid compression flag: 60 (valid flags are 0 and 1) while receiving response with status: {code}"
+        );
+        assert!(transient(&status(&message)), "{code}");
+    }
+    assert!(!transient(&status(
+        "protocol error: received message with invalid compression flag: 60 (valid flags are 0 and 1) while receiving response with status: 400 Bad Request"
+    )));
+}
+
 #[test]
 fn an_upload_that_keeps_failing_skips_only_the_entries_that_need_it() {
     let server = Server::start();

docs/specs/remote.md

@@ -135,7 +135,9 @@ A call that times out, or that the server answers as `UNAVAILABLE`,
 4 s and 15 s. So is a call cut off by an HTTP/2 GOAWAY (tonic reports it as
 an `INTERNAL` "h2 protocol error"): a TLS proxy in front of the server, such
 as nginx, closes a connection after its per-connection request limit, and
-the channel reconnects for the retry. A server sharing a saturated disk with the builds can stall a
+the channel reconnects for the retry. A 502, 503 or 504 from such a proxy
+(tonic reports an `INTERNAL` error over its non-gRPC body) is retried too:
+nginx answers that way while the server behind it restarts. A server sharing a saturated disk with the builds can stall a
 write for seconds, and before the retries one stall threw the whole push
 away: blobs went up, but no action did. A blob that still fails leaves out
 only the entries that name it; the report counts them as "not pushed after