Make federator error logs more informative. (#3919)

wireapp · Mar 6, 2024 · 08b2540 · 08b2540
1 parent 97d2ef1
commit 08b2540
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 28 deletions.
diff --git a/changelog.d/5-internal/wpb6998-improve-federator-error-logs b/changelog.d/5-internal/wpb6998-improve-federator-error-logs
@@ -0,0 +1 @@
+Include remote domain in federator error logs
diff --git a/libs/wire-api-federation/default.nix b/libs/wire-api-federation/default.nix
@@ -11,6 +11,7 @@
 , bytestring
 , bytestring-conversion
 , containers
+, dns-util
 , exceptions
 , gitignoreSource
 , HsOpenSSL
@@ -58,6 +59,7 @@ mkDerivation {
     bytestring
     bytestring-conversion
     containers
+    dns-util
     exceptions
     HsOpenSSL
     http-media

diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Error.hs b/libs/wire-api-federation/src/Wire/API/Federation/Error.hs
@@ -84,6 +84,7 @@ module Wire.API.Federation.Error
 where
 
 import Data.Aeson qualified as Aeson
+import Data.Domain
 import Data.Text qualified as T
 import Data.Text.Encoding qualified as T
 import Data.Text.Encoding.Error qualified as T
@@ -97,6 +98,7 @@ import Network.Wai.Utilities.Error qualified as Wai
 import OpenSSL.Session (SomeSSLException)
 import Servant.Client
 import Wire.API.Error
+import Wire.Network.DNS.SRV
 
 -- | Transport-layer errors in federator client.
 data FederatorClientHTTP2Error
@@ -209,27 +211,43 @@ federationClientErrorToWai FederatorClientVersionMismatch =
     "internal-error"
     "Endpoint version mismatch in federation client"
 
-federationRemoteHTTP2Error :: FederatorClientHTTP2Error -> Wai.Error
-federationRemoteHTTP2Error FederatorClientNoStatusCode =
-  Wai.mkError
-    unexpectedFederationResponseStatus
-    "federation-http2-error"
-    "No status code in HTTP2 response"
-federationRemoteHTTP2Error (FederatorClientHTTP2Exception e) =
-  Wai.mkError
-    unexpectedFederationResponseStatus
-    "federation-http2-error"
-    (LT.pack (displayException e))
-federationRemoteHTTP2Error (FederatorClientTLSException e) =
-  Wai.mkError
-    (HTTP.mkStatus 525 "SSL Handshake Failure")
-    "federation-tls-error"
-    (LT.pack (displayException e))
-federationRemoteHTTP2Error (FederatorClientConnectionError e) =
-  Wai.mkError
-    federatorConnectionRefusedStatus
-    "federation-connection-refused"
-    (LT.pack (displayException e))
+federationRemoteHTTP2Error :: SrvTarget -> Text -> FederatorClientHTTP2Error -> Wai.Error
+federationRemoteHTTP2Error target path = \case
+  FederatorClientNoStatusCode ->
+    ( Wai.mkError
+        unexpectedFederationResponseStatus
+        "federation-http2-error"
+        "No status code in HTTP2 response"
+    )
+      & addErrData
+  (FederatorClientHTTP2Exception e) ->
+    ( Wai.mkError
+        unexpectedFederationResponseStatus
+        "federation-http2-error"
+        (LT.pack (displayException e))
+    )
+      & addErrData
+  (FederatorClientTLSException e) ->
+    ( Wai.mkError
+        (HTTP.mkStatus 525 "SSL Handshake Failure")
+        "federation-tls-error"
+        (LT.pack (displayException e))
+    )
+      & addErrData
+  (FederatorClientConnectionError e) ->
+    ( Wai.mkError
+        federatorConnectionRefusedStatus
+        "federation-connection-refused"
+        (LT.pack (displayException e))
+    )
+      & addErrData
+  where
+    addErrData err =
+      err
+        { Wai.errorData =
+            ((mkDomain . cs . srvTargetDomain $ target) :: Either String Domain)
+              & either (const Nothing) (\dom -> Just (Wai.FederationErrorData dom path))
+        }
 
 federationClientHTTP2Error :: FederatorClientHTTP2Error -> Wai.Error
 federationClientHTTP2Error (FederatorClientConnectionError e) =
@@ -243,16 +261,19 @@ federationClientHTTP2Error e =
     "federation-local-error"
     (LT.pack (displayException e))
 
-federationRemoteResponseError :: HTTP.Status -> LByteString -> Wai.Error
-federationRemoteResponseError status body =
+federationRemoteResponseError :: SrvTarget -> Text -> HTTP.Status -> LByteString -> Wai.Error
+federationRemoteResponseError target path status body =
   ( Wai.mkError
       unexpectedFederationResponseStatus
       "federation-remote-error"
       ( "A remote federator failed with status code: "
           <> LT.pack (show (HTTP.statusCode status))
       )
   )
-    { Wai.innerError =
+    { Wai.errorData =
+        ((mkDomain . cs . srvTargetDomain $ target) :: Either String Domain)
+          & either (const Nothing) (\dom -> Just (Wai.FederationErrorData dom path)),
+      Wai.innerError =
         Just $
           fromMaybe
             ( Wai.mkError

diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal
@@ -90,6 +90,7 @@ library
     , bytestring
     , bytestring-conversion
     , containers
+    , dns-util
     , exceptions
     , HsOpenSSL
     , http-media

diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs
@@ -63,10 +63,10 @@ data RemoteError
   deriving (Show)
 
 instance AsWai RemoteError where
-  toWai (RemoteError _ _ e) =
-    federationRemoteHTTP2Error e
-  toWai (RemoteErrorResponse _ _ status body) =
-    federationRemoteResponseError status body
+  toWai (RemoteError target msg err) =
+    federationRemoteHTTP2Error target msg err
+  toWai (RemoteErrorResponse target msg status body) =
+    federationRemoteResponseError target msg status body
 
 data Remote m a where
   DiscoverAndCall ::