server: add "extensions" field to action webhook error schema

https://github.com/hasura/graphql-engine-mono/pull/1698

GitOrigin-RevId: c3b6f1048b6702a53ebe6c49f23dedc0f1d88090

CHANGELOG.md

@@ -1,7 +1,6 @@
 # Hasura GraphQL Engine Changelog
 
 ## Next release
-
 (Add entries below in the order of server, console, cli, docs, others)
 
 - server: add webhook transformations for Actions and EventTriggers
@@ -14,6 +13,8 @@
 - server: prevent empty subscription roots in the schema (#6898)
 - console: support tracking of functions with return a single row
 
+- server: support `extensions` field in error responses from action webhook endpoints (fix #4001)
+
 ## v2.0.9
 
 - server: fix export_metadata V2 bug which included cron triggers with `include_in_metadata: false`
@@ -107,8 +108,7 @@ generally, JSON-style aggregates.
 - server: Support computed fields in query filter (`where` argument) (close #7100)
 - server: add a `$.detail.operation.request_mode` field to `http-log` which takes the values `"single"` or `"batched"` to log whether a GraphQL request was executed on its own or as part of a batch
 - server: add `query` field to `http-log` and `websocket-log` in non-error cases
-- server: Add global limit to BigQuery via the `global_select_limit`
-  field in the connection configuration
+- server: Add global limit to BigQuery via the `global_select_limit` field in the connection configuration
 - server: include action and event names in log output
 - server: log all HTTP errors in remote schema calls as `remote-schema-error` with details
 - server: For BigQuery, make `global_select_limit` configuration optional with a default value of

docs/graphql/core/actions/action-handlers.rst

@@ -65,10 +65,28 @@ An error object looks like:
 
     {
       "message": "<mandatory-error-message>",
-      "code": "<optional-error-code>"
+      "extensions": "<optional-json-object>"
     }
 
-The HTTP status code must be ``4xx`` for an error response.
+where ``extensions`` is an optional JSON value. 
+
+If present, ``extensions`` should be a JSON object, which may have a status code
+field ``code``, along with other data you may want to add to your errors:
+
+.. code-block:: json
+
+    {
+      "code": "<optional-error-code>",
+      "optionalField1": "<custom-data-here>"
+    }
+
+The HTTP status code must be ``4xx`` in order to indicate an error response.
+
+For backwards compatibility with previous versions of Hasura, the ``code`` field
+may also be supplied at the root of the error object, i.e. at ``$.code``. This
+will be deprecated in a future release, and providing ``code`` within
+``extensions`` is preferred.
+
 
 
 Example

server/src-lib/Hasura/Backends/Postgres/Connection.hs

@@ -221,7 +221,7 @@ enablePgcryptoExtension = do
               _            -> requiredError
           where
             requiredError =
-              (err500 PostgresError requiredMessage) { qeInternal = Just $ toJSON e }
+              (err500 PostgresError requiredMessage) { qeInternal = Just $ ExtraInternal $ toJSON e }
             requiredMessage =
               "pgcrypto extension is required, but it could not be created;"
               <> " encountered unknown postgres error"

server/src-lib/Hasura/Backends/Postgres/DDL/RunSQL.hs

@@ -167,7 +167,7 @@ runRunSQL q@RunSQL{..} = do
       fmap (encJFromJValue @RunSQLRes) . liftTx . Q.multiQE rawSqlErrHandler . Q.fromText
       where
         rawSqlErrHandler txe =
-          (err400 PostgresError "query execution failed") { qeInternal = Just $ toJSON txe }
+          (err400 PostgresError "query execution failed") { qeInternal = Just $ ExtraInternal $ toJSON txe }
 
 -- | @'withMetadataCheck' cascade action@ runs @action@ and checks if the schema changed as a
 -- result. If it did, it checks to ensure the changes do not violate any integrity constraints, and

server/src-lib/Hasura/Backends/Postgres/Execute/Types.hs

@@ -75,7 +75,7 @@ defaultTxErrorHandler = mkTxErrorHandler (const False)
 mkTxErrorHandler :: (PGErrorType -> Bool) -> Q.PGTxErr -> QErr
 mkTxErrorHandler isExpectedError txe = fromMaybe unexpectedError expectedError
   where
-    unexpectedError = (internalError "database query error") { qeInternal = Just $ J.toJSON txe }
+    unexpectedError = (internalError "database query error") { qeInternal = Just $ ExtraInternal $ J.toJSON txe }
     expectedError = uncurry err400 <$> do
       errorDetail <- Q.getPGStmtErr txe
       message <- Q.edMessage errorDetail

server/src-lib/Hasura/Base/Error.hs

@@ -3,6 +3,7 @@
 module Hasura.Base.Error
   ( Code(..)
   , QErr(..)
+  , QErrExtra(..)
   , encodeJSONPath
   , encodeQErr
   , encodeGQLErr
@@ -152,9 +153,24 @@ data QErr
   , qeStatus   :: !N.Status
   , qeError    :: !Text
   , qeCode     :: !Code
-  , qeInternal :: !(Maybe Value)
+  , qeInternal :: !(Maybe QErrExtra)
   } deriving (Show, Eq)
 
+-- | Extra context for a QErr, which can either be information from an internal
+-- error (e.g. from Postgres, or from a network operation timing out), or
+-- context provided when an external service or operation fails, for instance, a
+-- webhook error response may provide additional context in the `extensions`
+-- key.
+data QErrExtra
+  = ExtraExtensions !Value
+  | ExtraInternal !Value
+  deriving (Show, Eq)
+
+instance ToJSON QErrExtra where
+  toJSON = \case
+    ExtraExtensions v -> v
+    ExtraInternal v   -> v
+
 instance ToJSON QErr where
   toJSON (QErr jPath _ msg code Nothing) =
     object
@@ -162,13 +178,16 @@ instance ToJSON QErr where
     , "error" .= msg
     , "code"  .= code
     ]
-  toJSON (QErr jPath _ msg code (Just ie)) =
-    object
-    [ "path"  .= encodeJSONPath jPath
-    , "error" .= msg
-    , "code"  .= code
-    , "internal" .= ie
-    ]
+  toJSON (QErr jPath _ msg code (Just extra)) = object $
+    case extra of
+      ExtraInternal e   -> err ++ [ "internal" .= e ]
+      ExtraExtensions{} -> err
+    where
+      err =
+        [ "path"  .= encodeJSONPath jPath
+        , "error" .= msg
+        , "code"  .= code
+        ]
 
 noInternalQErrEnc :: QErr -> Value
 noInternalQErrEnc (QErr jPath _ msg code _) =
@@ -179,18 +198,24 @@ noInternalQErrEnc (QErr jPath _ msg code _) =
   ]
 
 encodeGQLErr :: Bool -> QErr -> Value
-encodeGQLErr includeInternal (QErr jPath _ msg code mIE) =
+encodeGQLErr includeInternal (QErr jPath _ msg code maybeExtra) =
   object
   [ "message" .= msg
   , "extensions" .= extnsObj
   ]
   where
-    extnsObj = object $ bool codeAndPath
-               (codeAndPath ++ internal) includeInternal
-    codeAndPath = [ "code" .= code
-                  , "path" .= encodeJSONPath jPath
-                  ]
-    internal = maybe [] (\ie -> ["internal" .= ie]) mIE
+    appendIf cond a b = if cond then a ++ b else a
+
+    extnsObj = case maybeExtra of
+      Nothing -> object codeAndPath
+      -- if an `extensions` key is given in the error response from the webhook,
+      -- we ignore the `code` key regardless of whether the `extensions` object
+      -- contains a `code` field:
+      Just (ExtraExtensions v) -> v
+      Just (ExtraInternal v) ->
+        object $ appendIf includeInternal codeAndPath ["internal" .= v]
+    codeAndPath = ["path" .= encodeJSONPath jPath,
+                   "code" .= code]
 
 -- whether internal should be included or not
 encodeQErr :: Bool -> QErr -> Value
@@ -226,17 +251,17 @@ instance Q.FromPGConnErr QErr where
   fromPGConnErr c
     | "too many clients" `T.isInfixOf` (Q.getConnErr c) =
       let e = err500 PostgresMaxConnectionsError "max connections reached on postgres"
-      in e {qeInternal = Just $ toJSON c}
+      in e {qeInternal = Just $ ExtraInternal $ toJSON c}
 
   fromPGConnErr c =
     let e = err500 PostgresError "connection error"
-    in e {qeInternal = Just $ toJSON c}
+    in e {qeInternal = Just $ ExtraInternal $ toJSON c}
 
 -- Postgres Transaction error
 instance Q.FromPGTxErr QErr where
   fromPGTxErr txe =
     let e = err500 PostgresError "postgres tx error"
-    in e {qeInternal = Just $ toJSON txe}
+    in e {qeInternal = Just $ ExtraInternal $ toJSON txe}
 
 err400 :: Code -> Text -> QErr
 err400 c t = QErr [] N.status400 t c Nothing
@@ -284,7 +309,7 @@ internalError = err500 Unexpected
 
 throw500WithDetail :: (QErrM m) => Text -> Value -> m a
 throw500WithDetail t detail =
-  throwError $ (err500 Unexpected t) {qeInternal = Just detail}
+  throwError $ (err500 Unexpected t) {qeInternal = Just $ ExtraInternal detail}
 
 modifyQErr :: (QErrM m)
            => (QErr -> QErr) -> m a -> m a

server/src-lib/Hasura/GraphQL/Execute.hs

@@ -220,7 +220,7 @@ checkQueryInAllowlist enableAL userInfo req sc =
   where
     modErr e =
       let msg = "query is not in any of the allowlists"
-      in e{qeInternal = Just $ J.object [ "message" J..= J.String msg]}
+      in e{qeInternal = Just $ ExtraInternal $ J.object [ "message" J..= J.String msg]}
 
     isQueryInAllowlist q = HS.member gqlQuery
       where

server/src-lib/Hasura/GraphQL/Execute/Action.hs

@@ -439,7 +439,7 @@ callWebhook env manager outputType outputFields reqHeaders confHeaders
               addInternalToErr e =
                 let actionInternalError = J.toJSON $
                       ActionInternalError (J.String "unexpected response") requestInfo $ Just responseInfo
-                in e{qeInternal = Just actionInternalError}
+                in e{qeInternal = Just $ ExtraInternal actionInternalError}
 
           if | HTTP.statusIsSuccessful responseStatus  -> do
                  let expectingArray = isListType outputType
@@ -457,10 +457,10 @@ callWebhook env manager outputType outputFields reqHeaders confHeaders
                    pure (webhookResponse, mkSetCookieHeaders responseWreq)
 
              | HTTP.statusIsClientError responseStatus -> do
-                 ActionWebhookErrorResponse message maybeCode <-
+                 ActionWebhookErrorResponse message maybeCode maybeExtensions <-
                    modifyQErr addInternalToErr $ decodeValue responseValue
                  let code = maybe Unexpected ActionWebhookCode maybeCode
-                     qErr = QErr [] responseStatus message code Nothing
+                     qErr = QErr [] responseStatus message code (ExtraExtensions <$> maybeExtensions)
                  throwError qErr
 
              | otherwise -> do

server/src-lib/Hasura/GraphQL/Execute/Action/Types.hs

@@ -73,8 +73,9 @@ $(J.deriveJSON (J.aesonDrop 4 J.snakeCase) ''ActionWebhookPayload)
 
 data ActionWebhookErrorResponse
   = ActionWebhookErrorResponse
-  { _awerMessage :: !Text
-  , _awerCode    :: !(Maybe Text)
+  { _awerMessage    :: !Text
+  , _awerCode       :: !(Maybe Text)
+  , _awerExtensions :: !(Maybe J.Value)
   } deriving (Show, Eq)
 $(J.deriveJSON (J.aesonDrop 5 J.snakeCase) ''ActionWebhookErrorResponse)
 

server/src-lib/Hasura/GraphQL/Execute/RemoteJoin/RemoteSchema.hs

@@ -150,7 +150,7 @@ getRemoteSchemaResponse env manager requestHeaders userInfo (RemoteSchemaCall rs
           in AO.asObject v' `onLeft` throw500
     | otherwise ->
       throwError (err400 Unexpected "Errors from remote server") {
-        qeInternal = Just $ A.object ["errors" A..= (AO.fromOrdered <$> errors)]
+        qeInternal = Just $ ExtraInternal $ A.object ["errors" A..= (AO.fromOrdered <$> errors)]
       }
 
 -- | Attempt to extract a deeply nested value from a remote source's 'AO.Value'

server/src-lib/Hasura/GraphQL/RemoteServer.hs

@@ -527,7 +527,7 @@ throwRemoteSchemaWithInternal
   => Text -> a -> m b
 throwRemoteSchemaWithInternal msg v =
   let err = err400 RemoteSchemaError msg
-  in throwError err{qeInternal = Just $ J.toJSON v}
+  in throwError err{qeInternal = Just $ ExtraInternal $ J.toJSON v}
 
 throwRemoteSchemaHttp
   :: QErrM m

server/src-lib/Hasura/RQL/DDL/Metadata.hs

@@ -405,7 +405,7 @@ runDropInconsistentMetadata _ = do
   let droppableInconsistentObjects = filter droppableInconsistentMetadata newInconsistentObjects
   unless (null droppableInconsistentObjects) $
     throwError (err400 Unexpected "cannot continue due to new inconsistent metadata")
-      { qeInternal = Just $ toJSON newInconsistentObjects }
+      { qeInternal = Just $ ExtraInternal $ toJSON newInconsistentObjects }
   return successMsg
 
 purgeMetadataObj :: MetadataObjId -> MetadataModifier

server/src-lib/Hasura/RQL/DDL/Schema/Cache/Dependencies.hs

@@ -71,7 +71,7 @@ performIteration iterationNumber cache inconsistencies dependencies = do
           -- unless we did something very wrong, so halt the process and abort with some
           -- debugging information.
           throwError (err500 Unexpected "schema dependency resolution failed to terminate")
-            { qeInternal = Just $ object
+            { qeInternal = Just $ ExtraInternal $ object
                 [ "inconsistent_objects" .= object
                   [ "old" .= inconsistencies
                   , "new" .= newInconsistencies ]

server/src-lib/Hasura/RQL/Types/SchemaCache/Build.hs

@@ -110,7 +110,17 @@ withRecordInconsistency f = proc (e, (metadataObject, s)) -> do
   result <- runErrorA f -< (e, s)
   case result of
     Left err -> do
-      recordInconsistency -< ((qeInternal err, metadataObject), qeError err)
+      case qeInternal err of
+        Just (ExtraExtensions exts) ->
+          -- the QErr type contains an optional qeInternal :: Maybe QErrExtra field, which either stores an error coming
+          -- from an action webhook (ExtraExtensions) or an internal error thrown somewhere within graphql-engine.
+          --
+          -- if we do have an error here, it should be an internal error and hence never be of the former case:
+          recordInconsistency -< ((Just (toJSON exts), metadataObject), "withRecordInconsistency: unexpected ExtraExtensions")
+        Just (ExtraInternal internal) ->
+          recordInconsistency -< ((Just (toJSON internal), metadataObject), qeError err)
+        Nothing ->
+          recordInconsistency -< ((Nothing, metadataObject), qeError err)
       returnA -< Nothing
     Right v -> returnA -< Just v
 {-# INLINABLE withRecordInconsistency #-}
@@ -235,11 +245,11 @@ buildSchemaCacheFor objectId metadataModifier = do
 
   for_ (M.lookup objectId newInconsistentObjects) $ \matchingObjects -> do
     let reasons = commaSeparated $ imReason <$> matchingObjects
-    throwError (err400 InvalidConfiguration reasons) { qeInternal = Just $ toJSON matchingObjects }
+    throwError (err400 InvalidConfiguration reasons) { qeInternal = Just $ ExtraInternal $ toJSON matchingObjects }
 
   unless (null newInconsistentObjects) $
     throwError (err400 Unexpected "cannot continue due to new inconsistent metadata")
-      { qeInternal = Just $ toJSON (nub . concatMap toList $ M.elems newInconsistentObjects) }
+      { qeInternal = Just $ ExtraInternal $ toJSON (nub . concatMap toList $ M.elems newInconsistentObjects) }
 
 -- | Like 'buildSchemaCache', but fails if there is any inconsistent metadata.
 buildSchemaCacheStrict :: (QErrM m, CacheRWM m, MetadataM m) => m ()
@@ -249,7 +259,7 @@ buildSchemaCacheStrict = do
   let inconsObjs = scInconsistentObjs sc
   unless (null inconsObjs) $ do
     let err = err400 Unexpected "cannot continue due to inconsistent metadata"
-    throwError err{ qeInternal = Just $ toJSON inconsObjs }
+    throwError err{ qeInternal = Just $ ExtraInternal $ toJSON inconsObjs }
 
 -- | Executes the given action, and if any new 'InconsistentMetadata's are added to the schema
 -- cache as a result of its execution, raises an error.
@@ -264,6 +274,6 @@ withNewInconsistentObjsCheck action = do
         nub $ concatMap toList $ M.elems (currentObjects `diffInconsistentObjects` originalObjects)
   unless (null newInconsistentObjects) $
     throwError (err500 Unexpected "cannot continue due to newly found inconsistent metadata")
-      { qeInternal = Just $ toJSON newInconsistentObjects }
+      { qeInternal = Just $ ExtraInternal $ toJSON newInconsistentObjects }
 
   pure result

server/tests-py/context.py

@@ -349,10 +349,18 @@ class ActionsWebhookHandler(http.server.BaseHTTPRequestHandler):
             resp, status = self.get_users_by_email(False)
             self._send_response(status, resp)
 
+        elif req_path == "/intentional-error":
+            resp, status = self.intentional_error()
+            self._send_response(status, resp)
+
         else:
             self.send_response(HTTPStatus.NO_CONTENT)
             self.end_headers()
 
+    def intentional_error(self):
+        blob = self.req_json['input']['blob']
+        return blob, HTTPStatus.BAD_REQUEST
+
     def create_user(self):
         email_address = self.req_json['input']['email']
         name = self.req_json['input']['name']

server/tests-py/queries/actions/sync/extensions_code_both_codes.yaml

@@ -0,0 +1,24 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(
+        blob: {
+          message: "intentionally generated error"
+          code: "toplevel-error"
+          extensions: { foo: "bar", code: "action-error" }
+        }
+      ) {
+        id
+      }
+    }
+
+
+response:
+  errors:
+    - extensions:
+        foo: bar
+        code: action-error
+      message: intentionally generated error

server/tests-py/queries/actions/sync/extensions_code_nothing.yaml

@@ -0,0 +1,17 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(blob: { message: "intentionally generated error" }) {
+        id
+      }
+    }
+
+response:
+  errors:
+    - extensions:
+        path: $
+        code: unexpected
+      message: intentionally generated error

server/tests-py/queries/actions/sync/extensions_code_only_empty_extensions.yaml

@@ -0,0 +1,22 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(
+        blob: {
+          message: "intentionally generated error"
+          extensions: { foo: "bar" }
+        }
+      ) {
+        id
+      }
+    }
+
+
+response:
+  errors:
+    - extensions:
+        foo: bar
+      message: intentionally generated error

server/tests-py/queries/actions/sync/extensions_code_only_extensions_code.yaml

@@ -0,0 +1,22 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(
+        blob: {
+          message: "intentionally generated error"
+          extensions: { foo: "bar", code: "action-error" }
+        }
+      ) {
+        id
+      }
+    }
+
+response:
+  errors:
+    - extensions:
+        foo: bar
+        code: action-error
+      message: intentionally generated error

server/tests-py/queries/actions/sync/extensions_code_toplevel_empty_extensions.yaml

@@ -0,0 +1,22 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(
+        blob: {
+          message: "intentionally generated error"
+          code: "toplevel-error"
+          extensions: { foo: "bar" }
+        }
+      ) {
+        id
+      }
+    }
+
+response:
+  errors:
+    - extensions:
+        foo: bar
+      message: intentionally generated error

server/tests-py/queries/actions/sync/extensions_code_toplevel_no_extensions.yaml

@@ -0,0 +1,19 @@
+description: Run get_user_by_email query action with invalid email
+url: /v1/graphql
+status: 200
+query:
+  query: |
+    query {
+      intentional_error(
+        blob: { message: "intentionally generated error", code: "toplevel-error" }
+      ) {
+        id
+      }
+    }
+
+response:
+  errors:
+    - extensions:
+        path: $
+        code: toplevel-error
+      message: intentionally generated error

server/tests-py/queries/actions/sync/schema_setup.yaml

@@ -52,6 +52,15 @@ args:
       - name: age
         type: Int
 
+    - name: IntentionalErrorInput
+      fields:
+      - name: message
+        type: String!
+      - name: code
+        type: String
+      - name: extensions
+        type: json
+
     objects:
     - name: UserId
       fields:
@@ -155,6 +164,18 @@ args:
       output_type: '[UserId]!'
       handler: http://127.0.0.1:5593/get-users-by-email
 
+- type: create_action
+  args:
+    name: intentional_error
+    definition:
+      type: query
+      arguments:
+      - name: blob
+        type: IntentionalErrorInput!
+      # this can be anything, since this action never succeeds
+      output_type: '[UserId]!'
+      handler: http://127.0.0.1:5593/intentional-error
+
 - type: create_select_permission
   args:
     table: user

server/tests-py/queries/actions/sync/schema_teardown.yaml

@@ -24,6 +24,10 @@ args:
   args:
     name: get_users_by_email
     clear_data: true
+- type: drop_action
+  args:
+    name: intentional_error
+    clear_data: true
 # clear custom types
 - type: set_custom_types
   args: {}

server/tests-py/test_actions.py

@@ -152,6 +152,25 @@ class TestQueryActions:
     def dir(cls):
         return 'queries/actions/sync'
 
+    # toplevel, extensions with error
+    def test_query_action_extensions_code_both_codes_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_both_codes.yaml')
+    # toplevel, extensions with no error
+    def test_query_action_extensions_code_toplevel_empty_extensions_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_toplevel_empty_extensions.yaml')
+    # toplevel, no extensions
+    def test_query_action_extensions_code_toplevel_no_extensions_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_toplevel_no_extensions.yaml')
+    # no toplevel, extensions with error
+    def test_query_action_extensions_code_only_extensions_code_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_only_extensions_code.yaml')
+    # no toplevel, extensions with no error
+    def test_query_action_extensions_code_only_empty_extensions_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_only_empty_extensions.yaml')
+    # no toplevel, no extensions
+    def test_query_action_extensions_code_nothing_fail(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/extensions_code_nothing.yaml')
+
     def test_query_action_fail(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + '/get_user_by_email_fail.yaml')