fix Federator's streamingResponseToWai (side effect: deflake testNotificationsForOfflineBackends)

changelog.d/3-bug-fixes/federator-streaming-response-framing

@@ -0,0 +1 @@
+Federator: stop forwarding the upstream `Content-Length`/`Transfer-Encoding` headers when re-streaming a federation response. Warp re-frames the streamed body itself (chunked); forwarding a `Content-Length` made Warp serve the body under that declared length instead, so a truncated/reset cold-start upstream (declared length ≠ streamed bytes) desynchronised the caller's keep-alive connection and could deliver an unrelated response (e.g. a `/i/metrics` page in reply to a `POST /rpc/.../brig/api-version`). This was the cause of the flaky `Federation.testNotificationsForOfflineBackends` / `Federator.testFederatorNumRequestsMetrics` failures.

services/federator/default.nix

@@ -168,6 +168,7 @@ mkDerivation {
   ];
   testHaskellDepends = [
     aeson
+    async
     base
     bytestring
     bytestring-conversion
@@ -178,13 +179,15 @@ mkDerivation {
     dns-util
     filepath
     HsOpenSSL
+    http-client
     http-types
     http2
     http2-manager
     imports
     interpolate
     kan-extensions
     mtl
+    network
     polysemy
     polysemy-wire-zoo
     QuickCheck

services/federator/federator.cabal

@@ -326,6 +326,7 @@ test-suite federator-tests
     Test.Federator.Monitor
     Test.Federator.Options
     Test.Federator.Remote
+    Test.Federator.Response
     Test.Federator.Util
     Test.Federator.Validation
 
@@ -380,6 +381,7 @@ test-suite federator-tests
 
   build-depends:
       aeson
+    , async
     , base
     , bytestring
     , bytestring-conversion
@@ -391,13 +393,15 @@ test-suite federator-tests
     , federator
     , filepath
     , HsOpenSSL
+    , http-client
     , http-types
     , http2
     , http2-manager
     , imports
     , interpolate
     , kan-extensions
     , mtl
+    , network
     , polysemy
     , polysemy-wire-zoo
     , QuickCheck

services/federator/src/Federator/Response.hs

@@ -19,17 +19,38 @@ module Federator.Response where
 
 import Data.ByteString.Builder
 import Imports
+import Network.HTTP.Types.Header (hContentLength, hTransferEncoding)
 import Network.Wai qualified as Wai
 import Servant.Client.Core
 import Servant.Types.SourceT
 
+-- | Turn a streaming upstream response (from the outward federation call) into a
+-- WAI response that Warp serves back to the caller.
+--
+-- We re-frame the body: Warp streams it with chunked transfer-encoding (it does
+-- not know the length up front). We must therefore DROP the upstream's own
+-- framing headers ('Content-Length', 'Transfer-Encoding'); forwarding them is a
+-- keep-alive desync waiting to happen.
+--
+-- In particular, if we forward a 'Content-Length', Warp honours it verbatim and
+-- sends the streamed body raw under that declared length instead of chunking it.
+-- The moment the declared length disagrees with the number of bytes we actually
+-- stream — a truncated or reset cold-start upstream, a stale @Content-Length@ —
+-- the client reads exactly the declared number of bytes and runs straight past
+-- the response boundary into the next response on the reused connection. On the
+-- integration suite's shared, pooled HTTP/1.1 connection that surfaces as a
+-- @POST /rpc/…@ coming back with an unrelated @/i/metrics@ body. Stripping the
+-- framing headers lets Warp frame exactly what we stream, so the length on the
+-- wire can never disagree with the body and the connection stays in sync.
 streamingResponseToWai :: StreamingResponse -> Wai.Response
 streamingResponseToWai resp =
-  let headers = toList (responseHeaders resp)
+  let headers = filter (not . isFramingHeader . fst) (toList (responseHeaders resp))
       status = responseStatusCode resp
       streamingBody output flush =
         foreach
           (const (pure ()))
           (\chunk -> output (byteString chunk) *> flush)
           (responseBody resp)
    in Wai.responseStream status headers streamingBody
+  where
+    isFramingHeader h = h == hContentLength || h == hTransferEncoding

services/federator/test/unit/Main.hs

@@ -28,6 +28,7 @@ import Test.Federator.InternalServer qualified
 import Test.Federator.Monitor qualified
 import Test.Federator.Options qualified
 import Test.Federator.Remote qualified
+import Test.Federator.Response qualified
 import Test.Federator.Validation qualified
 import Test.Tasty
 
@@ -43,5 +44,6 @@ main =
           Test.Federator.InternalServer.tests,
           Test.Federator.ExternalServer.tests,
           Test.Federator.Monitor.tests,
-          Test.Federator.Remote.tests
+          Test.Federator.Remote.tests,
+          Test.Federator.Response.tests
         ]