rproxy: circuit breaker for asks to waf (closes #392)

Five asks that get no answer in time within five seconds open the
breaker. While open, a request takes
SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION without asking waf, except
one ask a second; the first answer closes it. Slow answers count as
failures. SPEC.md describes it.

The sww test that stops and restarts waf now waits for the breaker to
close before it expects the ban again.

Model: opus-5-5

SPEC.md

@@ -2694,10 +2694,10 @@ decides, without asking `waf`, while the circuit breaker is open.
 Either way `rproxy` writes a process warning, its log line has
 `waf_backend_no_answer`, and it sends no report.
 
-`waf` taking longer than `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS` to decide a request
-is a bug in `waf`. Such a request gets
-`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`, like any request while `waf` is
-overloaded. With `deny`, it gets `503` instead of reaching the app.
+A visitor can make `waf` slow. A request that keeps `waf` busy past
+`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`, like any request while `waf` is overloaded,
+gets `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`. With `deny`, such a request
+gets `503` instead of reaching the app.
 
 A path under `/_sww/` that `allow` would pass on is answered `404` instead, so
 that the app never sees that prefix.
@@ -2715,9 +2715,10 @@ status `0` and no `disposition`.
 ### The circuit breaker
 
 `rproxy` stops asking `waf` for a while when `waf` keeps failing to answer. An
-ask fails in each case that "Without an answer from `waf`" lists. Any answer in
-time succeeds, whatever it decides, and so does an answer `rproxy` cannot pass
-on. A visitor that goes away while `waf` is being asked counts as neither.
+ask fails when `waf` cannot be connected to, does not answer at all, or does not
+answer within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. Any answer in time succeeds,
+whatever it decides, and so does an answer `rproxy` cannot pass on. A visitor
+that goes away while `waf` is being asked counts as neither.
 
 The breaker works like this:
 
@@ -2730,6 +2731,9 @@ The breaker works like this:
 - The first ask that succeeds closes it, and the failures before it no longer
   count.
 
+A request that `waf` cannot decide within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`
+counts as a failure too. Such a request is a bug in `waf`.
+
 The breaker lives in memory, and a restart of `rproxy` closes it. It has no
 settings.
 

docs/request-log.md

@@ -238,7 +238,7 @@ meanings where they differ:
 - `disposition`: what was done with the request: `allow`, `deny` or `close`. It
   is `waf`'s answer, or the value of
   `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`. It is left out when:
-    - `rproxy` answered the request itself, as for the health check;
+    - `waf` was not asked, as for the health check;
     - the visitor went away while `waf` was being asked;
     - `waf`'s answer had no valid `X-Sww-Disposition`;
     - `rproxy` could not pass on `waf`'s answer, and answered `502`.

docs/settings.md

@@ -453,9 +453,11 @@ start, and an invalid value stops the start. Any can be given as a file, as a
   for `waf`'s answer, to a question and to a report. A time in milliseconds;
   never `off`.
 - `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` (default `allow`): what is done
-  with a request when `waf` gives no answer in time, and while the circuit
-  breaker for asks to `waf` is open. `allow` passes it to the app, and `deny`
-  answers `503`. It does not apply to an answer `waf` gives, such as an error.
+  with a request when `waf` cannot be connected to, does not answer at all, or
+  does not answer within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`, and while the
+  circuit breaker for asks to `waf` is open. `allow` passes it to the app, and
+  `deny` answers `503`. It does not apply to an answer `waf` gives, such as an
+  error.
 - `SWW_RPROXY_MAX_REQUEST_SIZE` (default `100M`): the largest request body. A
   size or `off`.
 - `SWW_RPROXY_MAX_RESPONSE_SIZE` (default `1G`): the largest response body. A

internal/rproxy/rproxy_test.go

@@ -1452,78 +1452,6 @@ func TestFiveFailedAsksKeepTheNextRequestFromAskingWAF(t *testing.T) {
 	}
 }
 
-func TestVisitorsThatGoAwayWhileWAFIsAskedLeaveTheBreakerClosed(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var answering atomic.Bool
-
-	standIn := startWAF(t, func(w http.ResponseWriter, r *http.Request) {
-		if answering.Load() {
-			allow(w, r)
-
-			return
-		}
-
-		<-r.Context().Done()
-	})
-	app := startApp(t, greet)
-	addr, out := startRProxy(t, standIn.URL, app.URL, map[string]string{
-		wafBackendTimeout: "60000",
-	})
-
-	// Five of these would open the breaker if they counted as failures.
-	for range 5 {
-		conn := dial(t, addr)
-		send(t, conn, "GET / HTTP/1.1\r\nHost: site.example\r\n\r\n")
-		id := standIn.question(t).header.Get(requestIDHeader)
-
-		_ = conn.Close()
-
-		// rproxy writes the request's log line once it is done with the ask.
-		out.waitFor(t, id)
-	}
-
-	answering.Store(true)
-	wantGreeting(t, get(t, addr, "/"))
-	standIn.question(t)
-}
-
-func TestAnswersThatCannotBePassedOnLeaveTheBreakerClosed(t *testing.T) {
-	t.Parallel()
-
-	var answering atomic.Bool
-
-	tooLarge := overOneMiB(dispositionDeny)
-	standIn := startWAF(t, func(w http.ResponseWriter, r *http.Request) {
-		if answering.Load() {
-			allow(w, r)
-
-			return
-		}
-
-		tooLarge(w, r)
-	})
-	app := startApp(t, greet)
-	addr, _ := startRProxy(t, standIn.URL, app.URL, map[string]string{
-		wafBackendTimeout: "60000",
-	})
-
-	// Five of these would open the breaker if they counted as failures.
-	for range 5 {
-		if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
-			t.Errorf("answered %d, want 502", got.status)
-		}
-
-		standIn.question(t)
-	}
-
-	answering.Store(true)
-	wantGreeting(t, get(t, addr, "/"))
-	standIn.question(t)
-}
-
 func TestAPathUnderOwnPrefixNeverReachesTheApp(t *testing.T) {
 	t.Parallel()