Improve error messages of Metadata API.

### Description

This PR improves error messages in our metadata API by displaying a message with the name of the failing command and a link to our documentation. Furthermore, it harmonizes our internal uses of `withObject`, to respect the convention of using the Haskell type name, now that the Aeson error message is displayed as an "internal error message".

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

GitOrigin-RevId: e4064ba3290306437aa7e45faa316c60e51bc6b6
This commit is contained in:
Antoine Leblanc 2021-09-20 20:49:33 +01:00 committed by hasura-bot
parent e77b79ae02
commit 21254256a1
15 changed files with 37 additions and 28 deletions

View File

@ -39,7 +39,7 @@ instance (Backend b) => ToJSON (AddComputedField b) where
toJSON = genericToJSON hasuraJSON
instance (Backend b) => FromJSON (AddComputedField b) where
parseJSON = withObject "add computed field" $ \o ->
parseJSON = withObject "AddComputedField" $ \o ->
AddComputedField
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
@ -78,7 +78,7 @@ data DropComputedField b
}
instance (Backend b) => FromJSON (DropComputedField b) where
parseJSON = withObject "drop computed field" $ \o ->
parseJSON = withObject "DropComputedField" $ \o ->
DropComputedField
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -57,7 +57,7 @@ data CreateEventTriggerQuery (b :: BackendType)
}
instance Backend b => FromJSON (CreateEventTriggerQuery b) where
parseJSON = withObject "create event trigger" \o -> do
parseJSON = withObject "CreateEventTriggerQuery" \o -> do
sourceName <- o .:? "source" .!= defaultSource
name <- o .: "name"
table <- o .: "table"
@ -102,7 +102,7 @@ data DeleteEventTriggerQuery (b :: BackendType)
}
instance FromJSON (DeleteEventTriggerQuery b) where
parseJSON = withObject "delete event trigger" $ \o ->
parseJSON = withObject "DeleteEventTriggerQuery" $ \o ->
DeleteEventTriggerQuery
<$> o .:? "source" .!= defaultSource
<*> o .: "name"
@ -115,7 +115,7 @@ data RedeliverEventQuery (b :: BackendType)
}
instance FromJSON (RedeliverEventQuery b) where
parseJSON = withObject "redeliver event trigger" $ \o ->
parseJSON = withObject "RedeliverEventQuery" $ \o ->
RedeliverEventQuery
<$> o .: "event_id"
<*> o .:? "source" .!= defaultSource
@ -129,7 +129,7 @@ data InvokeEventTriggerQuery (b :: BackendType)
}
instance Backend b => FromJSON (InvokeEventTriggerQuery b) where
parseJSON = withObject "invoke event trigger" $ \o ->
parseJSON = withObject "InvokeEventTriggerQuery" $ \o ->
InvokeEventTriggerQuery
<$> o .: "name"
<*> o .:? "source" .!= defaultSource

View File

@ -124,7 +124,7 @@ data ReplaceMetadataV1
deriving (Eq)
instance FromJSON ReplaceMetadataV1 where
parseJSON = withObject "Object" $ \o -> do
parseJSON = withObject "ReplaceMetadataV1" $ \o -> do
version <- o .:? "version" .!= MVVersion1
case version of
MVVersion3 -> RMWithSources <$> parseJSON (Object o)

View File

@ -375,7 +375,7 @@ data SetPermComment b
}
instance (Backend b) => FromJSON (SetPermComment b) where
parseJSON = withObject "set permission comment" $ \o ->
parseJSON = withObject "SetPermComment" $ \o ->
SetPermComment
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -111,7 +111,7 @@ data DropPerm (a :: BackendType -> Type) b
}
instance (Backend b) => FromJSON (DropPerm a b) where
parseJSON = withObject "drop permission" $ \o ->
parseJSON = withObject "DropPerm" $ \o ->
DropPerm
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -238,7 +238,7 @@ data DropRel b
}
instance (Backend b) => FromJSON (DropRel b) where
parseJSON = withObject "drop relationship" $ \o ->
parseJSON = withObject "DropRel" $ \o ->
DropRel
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
@ -306,7 +306,7 @@ deriving instance (Backend b) => Show (SetRelComment b)
deriving instance (Backend b) => Eq (SetRelComment b)
instance (Backend b) => FromJSON (SetRelComment b) where
parseJSON = withObject "set relationship comment" $ \o ->
parseJSON = withObject "SetRelComment" $ \o ->
SetRelComment
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -25,7 +25,7 @@ data RenameRel b
}
instance (Backend b) => FromJSON (RenameRel b) where
parseJSON = withObject "rename relationship" $ \o ->
parseJSON = withObject "RenameRel" $ \o ->
RenameRel
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -72,7 +72,7 @@ data DeleteRemoteRelationship (b :: BackendType)
}
instance Backend b => FromJSON (DeleteRemoteRelationship b) where
parseJSON = withObject "delete remote relationship" $ \o ->
parseJSON = withObject "DeleteRemoteRelationship" $ \o ->
DeleteRemoteRelationship
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -88,7 +88,7 @@ data TrackFunctionV2 (b :: BackendType) = TrackFunctionV2
}
instance Backend b => FromJSON (TrackFunctionV2 b) where
parseJSON = withObject "track function" $ \o ->
parseJSON = withObject "TrackFunctionV2" $ \o ->
TrackFunctionV2
<$> o .:? "source" .!= defaultSource
<*> o .: "function"
@ -202,7 +202,7 @@ data FunctionPermissionArgument b = FunctionPermissionArgument
instance (Backend b) => FromJSON (FunctionPermissionArgument b) where
parseJSON v =
flip (withObject "function permission") v $ \o ->
flip (withObject "FunctionPermissionArgument") v $ \o ->
FunctionPermissionArgument
<$> o .: "function"
<*> o .:? "source" .!= defaultSource

View File

@ -35,7 +35,7 @@ data AddSource b
}
instance (Backend b) => FromJSON (AddSource b) where
parseJSON = withObject "add source" $ \o ->
parseJSON = withObject "AddSource" $ \o ->
AddSource
<$> o .: "name"
<*> o .: "configuration"
@ -121,7 +121,7 @@ data DropSource
} deriving (Show, Eq)
instance FromJSON DropSource where
parseJSON = withObject "drop source" $ \o ->
parseJSON = withObject "DropSource" $ \o ->
DropSource <$> o .: "name" <*> o .:? "cascade" .!= False
runDropSource

View File

@ -71,7 +71,7 @@ deriving instance (Backend b) => Eq (TrackTable b)
instance (Backend b) => FromJSON (TrackTable b) where
parseJSON v = withOptions v <|> withoutOptions
where
withOptions = withObject "track table" \o -> TrackTable
withOptions = withObject "TrackTable" \o -> TrackTable
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
<*> o .:? "is_enum" .!= False
@ -85,7 +85,7 @@ data SetTableIsEnum
} deriving (Show, Eq)
instance FromJSON SetTableIsEnum where
parseJSON = withObject "set table is enum" $ \o ->
parseJSON = withObject "SetTableIsEnum" $ \o ->
SetTableIsEnum
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
@ -101,7 +101,7 @@ deriving instance (Backend b) => Show (UntrackTable b)
deriving instance (Backend b) => Eq (UntrackTable b)
instance (Backend b) => FromJSON (UntrackTable b) where
parseJSON = withObject "untrack table" $ \o ->
parseJSON = withObject "UntrackTable" $ \o ->
UntrackTable
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
@ -219,7 +219,7 @@ data TrackTableV2 b
} deriving (Show, Eq)
instance (Backend b) => FromJSON (TrackTableV2 b) where
parseJSON = withObject "track table" $ \o -> do
parseJSON = withObject "TrackTableV2" $ \o -> do
table <- parseJSON $ Object o
configuration <- o .:? "configuration" .!= emptyTableConfig
pure $ TrackTableV2 table configuration
@ -250,7 +250,7 @@ data SetTableCustomization b
} deriving (Show, Eq)
instance (Backend b) => FromJSON (SetTableCustomization b) where
parseJSON = withObject "set table customization" $ \o ->
parseJSON = withObject "SetTableCustomization" $ \o ->
SetTableCustomization
<$> o .:? "source" .!= defaultSource
<*> o .: "table"
@ -265,7 +265,7 @@ data SetTableCustomFields
} deriving (Show, Eq)
instance FromJSON SetTableCustomFields where
parseJSON = withObject "set table custom fields" $ \o ->
parseJSON = withObject "SetTableCustomFields" $ \o ->
SetTableCustomFields
<$> o .:? "source" .!= defaultSource
<*> o .: "table"

View File

@ -16,8 +16,10 @@ module Hasura.Server.API.Backend where
import Hasura.Prelude
import qualified Data.Aeson.Types as J
import qualified Data.Text as T
import Data.Aeson ((<?>))
import Data.Aeson.Types (modifyFailure)
import Hasura.RQL.Types.Backend
import Hasura.SQL.AnyBackend
@ -45,7 +47,14 @@ commandParser expected constructor provided arguments =
-- instance backtracks: if we used 'fail', we would not be able to distinguish between "this is
-- the correct branch, the name matches, but the argument fails to parse, we must fail" and "this
-- is not the command we were expecting here, it is fine to continue with another".
whenMaybe (expected == provided) $ constructor <$> (J.parseJSON arguments <?> J.Key "args")
whenMaybe (expected == provided) $
modifyFailure withDetails $ constructor <$> (J.parseJSON arguments <?> J.Key "args")
where
withDetails internalErrorMessage = intercalate "\n"
[ "Error when parsing command " <> T.unpack expected <> "."
, "See our documentation at https://hasura.io/docs/latest/graphql/core/api-reference/metadata-api/index.html#metadata-apis."
, "Internal error message: " <> internalErrorMessage
]
sourceCommands, tableCommands, tablePermissionsCommands, functionCommands,
functionPermissionsCommands, relationshipCommands, remoteRelationshipCommands, eventTriggerCommands

View File

@ -3,7 +3,7 @@
status: 400
response:
path: $.args.transform.content_type
error: Invalid ContentType
error: "Error when parsing command create_event_trigger.\nSee our documentation at https://hasura.io/docs/latest/graphql/core/api-reference/metadata-api/index.html#metadata-apis.\nInternal error message: Invalid ContentType"
code: parse-failed
query:
type: pg_create_event_trigger

View File

@ -3,7 +3,7 @@
status: 400
response:
path: $.args.transform.request_headers.remove_headers[0]
error: 'Restricted Header: Content-Type'
error: "Error when parsing command create_event_trigger.\nSee our documentation at https://hasura.io/docs/latest/graphql/core/api-reference/metadata-api/index.html#metadata-apis.\nInternal error message: Restricted Header: Content-Type"
code: parse-failed
query:
type: pg_create_event_trigger

View File

@ -346,7 +346,7 @@ class TestGraphQLQueryBasicCitus:
def test_create_invalid_fkey_relationship(self, hge_ctx, transport):
st_code, resp = hge_ctx.v1metadataq_f(self.dir() + '/setup_invalid_fkey_relationship.yaml')
assert st_code == 400, resp
assert resp['error'] == "Expecting object { table, columns }."
assert resp['error'] == "Error when parsing command create_array_relationship.\nSee our documentation at https://hasura.io/docs/latest/graphql/core/api-reference/metadata-api/index.html#metadata-apis.\nInternal error message: Expecting object { table, columns }."
@classmethod
def dir(cls):
@ -574,7 +574,7 @@ class TestGraphQLQueryBoolExpBasicMSSQL:
def test_create_invalid_fkey_relationship(self, hge_ctx, transport):
st_code, resp = hge_ctx.v1metadataq_f(self.dir() + '/setup_invalid_fkey_relationship_mssql.yaml')
assert st_code == 400, resp
assert resp['error'] == "Expecting object { table, columns }."
assert resp['error'] == "Error when parsing command create_array_relationship.\nSee our documentation at https://hasura.io/docs/latest/graphql/core/api-reference/metadata-api/index.html#metadata-apis.\nInternal error message: Expecting object { table, columns }."
@classmethod
def dir(cls):