Merge pull request #205 from well-typed/edsko/broken-deployments

Synthesize errors for invalid `content-type`

interop/Interop/Client/Common.hs

@@ -108,8 +108,8 @@ verifyStreamingOutputCallResponse expectedSize resp = do
 verifyStreamingOutputs :: forall rpc.
      HasCallStack
   => Call rpc
-  -> (ProperTrailers' -> IO ())               -- ^ Verify trailers
-  -> [(InboundMeta, Output rpc) -> IO ()] -- ^ Verifier per expected output
+  -> (ProperTrailers' -> IO ())            -- ^ Verify trailers
+  -> [(InboundMeta, Output rpc) -> IO ()]  -- ^ Verifier per expected output
   -> IO ()
 verifyStreamingOutputs call verifyTrailers = go
   where

interop/Interop/Client/TestCase/EmptyUnary.hs

@@ -19,15 +19,14 @@ runTest cmdline =
     withConnection def (testServer cmdline) $ \conn ->
       withRPC conn def (Proxy @EmptyCall) $ \call -> do
         sendFinalInput call empty
-        streamElem :: StreamElem ProperTrailers' (InboundMeta, Proto Empty)
-          <- recvOutputWithMeta call
+        streamElem <- StreamElem.value <$> recvOutputWithMeta call
 
         -- The test description asks us to also verify the size of the /outgoing/
         -- message if possible. This information is not readily available in
         -- @grapesy@, but we will test it implicitly when running the @grapesy@
         -- interop client against the @grapesy@ interop server.
 
-        case StreamElem.value streamElem of
+        case streamElem of
           Just (meta, resp) -> do
             assertEqual empty $ resp
             assertEqual 0     $ inboundUncompressedSize meta

src/Network/GRPC/Client/Call.hs

@@ -60,6 +60,8 @@ import Network.GRPC.Util.Session qualified as Session
 import Network.GRPC.Util.Thread qualified as Thread
 
 import Paths_grapesy qualified as Grapesy
+import Network.GRPC.Util.HKD qualified as HKD
+import Data.Bifunctor
 
 {-------------------------------------------------------------------------------
   Open a call
@@ -196,12 +198,12 @@ startRPC conn _ callParams = do
           }
 
     let serverClosedConnection ::
-             Either TrailersOnly' ProperTrailers'
+             Either (TrailersOnly' HandledSynthesized) ProperTrailers'
           -> SomeException
         serverClosedConnection =
               either toException toException
             . grpcClassifyTermination
-            . either (fst . trailersOnlyToProperTrailers) id
+            . either trailersOnlyToProperTrailers' id
 
     channel <-
       Session.setupRequestChannel
@@ -374,7 +376,10 @@ recvResponseInitialMetadata :: forall rpc. Call rpc -> IO (ResponseMetadata rpc)
 recvResponseInitialMetadata call@Call{} =
     recvInitialResponse call >>= aux
   where
-    aux :: Either TrailersOnly' ResponseHeaders' -> IO (ResponseMetadata rpc)
+    aux ::
+         Either (TrailersOnly'    HandledSynthesized)
+                (ResponseHeaders' HandledSynthesized)
+      -> IO (ResponseMetadata rpc)
     aux (Left trailers) =
         case grpcClassifyTermination properTrailers of
           Left exception ->
@@ -383,7 +388,7 @@ recvResponseInitialMetadata call@Call{} =
             ResponseTrailingMetadata <$>
               parseMetadata (grpcTerminatedMetadata terminatedNormally)
       where
-        (properTrailers, _contentType) = trailersOnlyToProperTrailers trailers
+        properTrailers = trailersOnlyToProperTrailers' trailers
     aux (Right headers) =
         ResponseInitialMetadata <$>
           parseMetadata (customMetadataMapToList $ responseMetadata headers)
@@ -397,7 +402,9 @@ recvResponseInitialMetadata call@Call{} =
 -- Most applications will never need to use this function.
 recvInitialResponse ::
      Call rpc
-  -> IO (Either TrailersOnly' ResponseHeaders')
+  -> IO ( Either (TrailersOnly'    HandledSynthesized)
+                 (ResponseHeaders' HandledSynthesized)
+        )
 recvInitialResponse Call{callChannel} =
     fmap inbHeaders <$> Session.getInboundHeaders callChannel
 
@@ -547,11 +554,11 @@ recvBoth Call{callChannel} = liftIO $
     -- We lose type information here: Trailers-Only is no longer visible
     flatten ::
          Either
-           TrailersOnly'
+           (TrailersOnly' HandledSynthesized)
            (StreamElem ProperTrailers' (InboundMeta, Output rpc))
       -> StreamElem ProperTrailers' (InboundMeta, Output rpc)
     flatten (Left trailersOnly) =
-        NoMoreElems $ fst $ trailersOnlyToProperTrailers trailersOnly
+        NoMoreElems $ trailersOnlyToProperTrailers' trailersOnly
     flatten (Right streamElem) =
         streamElem
 
@@ -564,11 +571,11 @@ recvEither Call{callChannel} = liftIO $
   where
     flatten ::
          Either
-           TrailersOnly'
+           (TrailersOnly' HandledSynthesized)
            (Either ProperTrailers' (InboundMeta, Output rpc))
       -> Either ProperTrailers' (InboundMeta, Output rpc)
     flatten (Left trailersOnly) =
-        Left $ fst $ trailersOnlyToProperTrailers trailersOnly
+        Left $ trailersOnlyToProperTrailers' trailersOnly
     flatten (Right (Left properTrailers)) =
         Left $ properTrailers
     flatten (Right (Right msg)) =
@@ -584,3 +591,25 @@ responseTrailingMetadata Call{} trailers = liftIO $
         parseMetadata $ grpcTerminatedMetadata terminatedNormally
       Left exception ->
         throwM exception
+
+
+-- | Forget that we are in the Trailers-Only case
+--
+-- Error handling is a bit subtle here. If we are in the Trailers-Only case:
+--
+-- * Any synthesized errors have already been dealt with
+--   (the type @TrailersOnly' Void@ tell us this)
+-- * If 'connVerifyHeaders' is enabled, /all/ trailers have been verified
+--   (unfortunately this we cannot see from type).
+--
+-- This means that we might only have a (non-synthesized) error for the
+-- content-type if 'connVerifyHeaders' is /not/ enabled; since we are not
+-- actually interested in the content-type here, we can therefore just ignore
+-- these errors.
+trailersOnlyToProperTrailers' ::
+     TrailersOnly' HandledSynthesized
+  -> ProperTrailers'
+trailersOnlyToProperTrailers' =
+      fst                                   -- justified by the comment above
+    . trailersOnlyToProperTrailers
+    . HKD.map (first $ mapSynthesized handledSynthesized) -- simple injection

src/Network/GRPC/Client/Connection.hs

@@ -321,7 +321,10 @@ getOutboundCompression Connection{connMetaVar} =
 --
 -- Amongst other things, this updates the compression algorithm to be used
 -- (see also 'getOutboundCompression').
-updateConnectionMeta :: Connection -> ResponseHeaders' -> IO ()
+updateConnectionMeta ::
+     Connection
+  -> ResponseHeaders' HandledSynthesized
+  -> IO ()
 updateConnectionMeta Connection{connMetaVar, connParams} hdrs =
     modifyMVar_ connMetaVar $ Meta.update (connCompression connParams) hdrs
 

src/Network/GRPC/Client/Meta.hs

@@ -45,7 +45,9 @@ init initCompr = Meta {
 -- | Update 'Meta' given response headers
 --
 -- Returns the updated 'Meta'.
-update :: MonadThrow m => Compr.Negotation -> ResponseHeaders' -> Meta -> m Meta
+update ::
+     MonadThrow m
+  => Compr.Negotation -> ResponseHeaders' HandledSynthesized -> Meta -> m Meta
 update compr hdrs meta =
     Meta
       <$> updateCompression
@@ -64,7 +66,7 @@ update compr hdrs meta =
 updateCompression :: forall m.
      MonadThrow m
   => Compr.Negotation
-  -> Either InvalidHeaders (Maybe (NonEmpty CompressionId))
+  -> Either (InvalidHeaders HandledSynthesized) (Maybe (NonEmpty CompressionId))
   -> Maybe Compression -> m (Maybe Compression)
 updateCompression negotation (Right accepted) = go
   where

src/Network/GRPC/Client/Session.hs

@@ -41,14 +41,14 @@ data ClientOutbound rpc
 
 instance IsRPC rpc => DataFlow (ClientInbound rpc) where
   data Headers (ClientInbound rpc) = InboundHeaders {
-        inbHeaders     :: ResponseHeaders'
+        inbHeaders     :: ResponseHeaders' HandledSynthesized
       , inbCompression :: Compression
       }
     deriving (Show)
 
   type Message    (ClientInbound rpc) = (InboundMeta, Output rpc)
   type Trailers   (ClientInbound rpc) = ProperTrailers'
-  type NoMessages (ClientInbound rpc) = TrailersOnly'
+  type NoMessages (ClientInbound rpc) = TrailersOnly' HandledSynthesized
 
 instance IsRPC rpc => DataFlow (ClientOutbound rpc) where
   data Headers (ClientOutbound rpc) = OutboundHeaders {
@@ -87,15 +87,17 @@ instance SupportsClientRpc rpc => IsSession (ClientSession rpc) where
 instance SupportsClientRpc rpc => InitiateSession (ClientSession rpc) where
   parseResponse (ClientSession conn) (ResponseInfo status headers body) =
       case classifyServerResponse (Proxy @rpc) status headers body of
-        Left trailersOnly ->
+        Left parsed -> do
+          trailersOnly <- throwSynthesized throwIO parsed
           -- We classify the response as Trailers-Only if the grpc-status header
           -- is present, or when the HTTP status is anything other than 200 OK
           -- (which we treat, as per the spec, as an implicit grpc-status).
           -- The 'CallClosedWithoutTrailers' case is therefore not relevant.
           case verifyAllIf connVerifyHeaders trailersOnly of
             Left  err   -> throwIO $ CallSetupInvalidResponseHeaders err
             Right _hdrs -> return $ FlowStartNoMessages trailersOnly
-        Right responseHeaders -> do
+        Right parsed -> do
+          responseHeaders <- throwSynthesized throwIO parsed
           case verifyAllIf connVerifyHeaders responseHeaders of
             Left  err  -> throwIO $ CallSetupInvalidResponseHeaders err
             Right hdrs -> do
@@ -147,7 +149,7 @@ data CallSetupFailure =
     CallSetupUnsupportedCompression CompressionId
 
     -- | We failed to parse the response headers
-  | CallSetupInvalidResponseHeaders InvalidHeaders
+  | CallSetupInvalidResponseHeaders (InvalidHeaders HandledSynthesized)
   deriving stock (Show)
   deriving anyclass (Exception)
 

src/Network/GRPC/Common/Headers.hs

@@ -43,17 +43,19 @@ requiredHeadersVerified =
 -- By default, we only check those headers @grapesy@ needs to function.
 verifyRequired ::
      HasRequiredHeaders h
-  => h (Checked e) -> Either e (RequiredHeaders h)
+  => h (Checked (InvalidHeaders HandledSynthesized))
+  -> Either (InvalidHeaders HandledSynthesized) (RequiredHeaders h)
 verifyRequired = requiredHeaders
 
 -- | Validate /all/ headers
 --
 -- Validate all headers; we do this only if
 -- 'Network.GRPC.Client.connVerifyHeaders' (on the client) or
 -- 'Network.GRPC.Server.serverVerifyHeaders' (on the server) is enabled.
-verifyAll :: forall h e.
+verifyAll :: forall h.
      HasRequiredHeaders h
-  => h (Checked e) -> Either e (h Undecorated, RequiredHeaders h)
+  => h (Checked (InvalidHeaders HandledSynthesized))
+  -> Either (InvalidHeaders HandledSynthesized) (h Undecorated, RequiredHeaders h)
 verifyAll = fmap aux . HKD.sequence
   where
     aux :: h Undecorated -> (h Undecorated, RequiredHeaders h)
@@ -62,7 +64,9 @@ verifyAll = fmap aux . HKD.sequence
 -- | Convenience wrapper, conditionally verifying all headers
 verifyAllIf ::
      HasRequiredHeaders h
-  => Bool -> h (Checked e) -> Either e (RequiredHeaders h)
+  => Bool
+  -> h (Checked (InvalidHeaders HandledSynthesized))
+  -> Either (InvalidHeaders HandledSynthesized) (RequiredHeaders h)
 verifyAllIf False = verifyRequired
 verifyAllIf True  = fmap snd . verifyAll
 

src/Network/GRPC/Server/Call.hs

@@ -81,7 +81,7 @@ data Call rpc = SupportsServerRpc rpc => Call {
     , callChannel :: Session.Channel (ServerSession rpc)
 
       -- | Request headers
-    , callRequestHeaders :: RequestHeaders'
+    , callRequestHeaders :: RequestHeaders' HandledSynthesized
 
       -- | Response metadata
       --
@@ -190,6 +190,7 @@ determineInbound :: forall rpc.
   -> HTTP2.Request
   -> IO (Headers (ServerInbound rpc), Maybe Timeout)
 determineInbound session req = do
+    requestHeaders' <- throwSynthesized throwIO parsed
     case verifyAllIf serverVerifyHeaders requestHeaders' of
       Left  err  -> throwIO $ CallSetupInvalidRequestHeaders err
       Right hdrs -> do
@@ -206,9 +207,9 @@ determineInbound session req = do
     ServerContext{serverParams} = serverSessionContext
     ServerParams{serverVerifyHeaders} = serverParams
 
-    requestHeaders' :: RequestHeaders'
-    requestHeaders' = parseRequestHeaders' (Proxy @rpc) $
-                        fromHeaderTable $ HTTP2.requestHeaders req
+    parsed :: RequestHeaders' GrpcException
+    parsed = parseRequestHeaders' (Proxy @rpc) $
+               fromHeaderTable $ HTTP2.requestHeaders req
 
 -- | Determine outbound flow start
 --
@@ -296,7 +297,7 @@ getInboundCompression session = \case
 -- simply use no compression.
 getOutboundCompression ::
      ServerSession rpc
-  -> Either InvalidHeaders (Maybe (NonEmpty CompressionId))
+  -> Either (InvalidHeaders HandledSynthesized) (Maybe (NonEmpty CompressionId))
   -> Compression
 getOutboundCompression session = \case
     Left _invalidHeader -> noCompression
@@ -537,7 +538,7 @@ sendTrailersOnly Call{callContext, callResponseKickoff} metadata = do
 -- NOTE: When 'serverVerifyHeaders' is enabled the caller can be sure that the
 -- 'RequestHeaders'' do not contain any errors, even though unfortunately this
 -- is not visible from the type.
-getRequestHeaders :: Call rpc -> IO RequestHeaders'
+getRequestHeaders :: Call rpc -> IO (RequestHeaders' HandledSynthesized)
 getRequestHeaders Call{callRequestHeaders} =
     return callRequestHeaders
 

src/Network/GRPC/Server/Session.hs

@@ -33,7 +33,7 @@ data ServerOutbound rpc
 
 instance IsRPC rpc => DataFlow (ServerInbound rpc) where
   data Headers (ServerInbound rpc) = InboundHeaders {
-        inbHeaders     :: RequestHeaders'
+        inbHeaders     :: RequestHeaders' HandledSynthesized
       , inbCompression :: Compression
       }
     deriving (Show)
@@ -79,7 +79,7 @@ data CallSetupFailure =
     -- 'CallSetupInvalidResourceHeaders' refers to an invalid method (anything
     -- other than POST) or an invalid path; 'CallSetupInvalidRequestHeaders'
     -- means we could not parse the HTTP headers according to the gRPC spec.
-  | CallSetupInvalidRequestHeaders InvalidHeaders
+  | CallSetupInvalidRequestHeaders (InvalidHeaders HandledSynthesized)
 
     -- | Client chose unsupported compression algorithm
     --

src/Network/GRPC/Spec.hs

@@ -142,10 +142,17 @@ module Network.GRPC.Spec (
   , InvalidHeader(..)
     -- ** Construction
   , invalidHeader
-  , invalidHeaderWith
   , missingHeader
   , unexpectedHeader
+  , invalidHeaderSynthesize
   , throwInvalidHeader
+    -- ** Synthesized errors
+  , HandledSynthesized
+  , handledSynthesized
+  , dropSynthesized
+  , mapSynthesizedM
+  , mapSynthesized
+  , throwSynthesized
     -- ** Use
   , prettyInvalidHeaders
   , statusInvalidHeaders