diff --git a/crates/tinymold-remote/src/remote/mod.rs b/crates/tinymold-remote/src/remote/mod.rs index fd9347d..ed87443 100644 --- a/crates/tinymold-remote/src/remote/mod.rs +++ b/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 { result diff --git a/crates/tinymold-remote/src/remote/test.rs b/crates/tinymold-remote/src/remote/test.rs index 02c256a..f7e44ad 100644 --- a/crates/tinymold-remote/src/remote/test.rs +++ b/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(); diff --git a/docs/specs/remote.md b/docs/specs/remote.md index 3f95f1b..b3bcb66 100644 --- a/docs/specs/remote.md +++ b/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