add a new field `extensions` in the ActionWebhookErrorResponse, close #4001 #4351

codingkarthik · 2020-04-09T11:54:03Z

the extensions can take any json

fixes #4001

Description

Changelog

CHANGELOG.md is updated with user-facing content relevant to this PR.

Affected components

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

No
Yes
- Updated docs with SQL for downgrading the catalog

Metadata

Does this PR add a new Metadata feature?

No
Yes
- Does run_sql auto manages the new metadata through schema diffing?
  - Yes
  - Not required
- Does run_sql auto manages the definitions of metadata on renaming?
  - Yes
  - Not required
- Does export_metadata/replace_metadata supports the new metadata added?
  - Yes
  - Not required

GraphQL

No new GraphQL schema is generated
New GraphQL schema is being generated:
- New types and typenames are correlated

Breaking changes

- the extensions can take any json

hasura-bot · 2020-04-09T12:06:55Z

Review app for commit 3b85b73 deployed to Heroku: https://hge-ci-pull-4351.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4351-3b85b73a

tirumaraiselvan · 2020-04-09T12:14:26Z

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


         | HTTP.statusIsClientError responseStatus -> do
-             ActionWebhookErrorResponse message maybeCode <-
+             ActionWebhookErrorResponse message maybeCode maybeExtension <-


We should not take code anymore.

See spec: #4001 (comment)

It is find to do this "breaking" change because we introduced it in beta.

I kept code thinking about the breaking change, I'll fix this

@codingkarthik Cool. Also, could you also update the docs with this PR itself? This section: https://hasura.io/docs/1.0/graphql/manual/actions/action-handlers.html#returning-an-error-response

Yes, I'll make the changes

netlify · 2020-04-09T12:50:24Z

Deploy preview for hasura-docs ready!

Built with commit 6b5c579

https://deploy-preview-4351--hasura-docs.netlify.com

hasura-bot · 2020-04-09T13:02:27Z

Review app for commit e6ffc0a deployed to Heroku: https://hge-ci-pull-4351.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4351-e6ffc0a6

rakeshkky · 2020-04-09T13:05:22Z

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

+  = ActionWebhookErrorExtension
+  { _aweeExtension :: !J.Value
+  }
+$(J.deriveJSON (J.aesonDrop 5 J.snakeCase) ''ActionWebhookErrorExtension)


I don't think we need a new type which is just a wrapper over J.Value.

rakeshkky · 2020-04-09T13:33:10Z

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

-                 qErr = QErr [] responseStatus message code Nothing
+             let qErr = QErr [] responseStatus message Unexpected $
+                   (maybe Nothing (Just . J.toJSON . ActionWebhookErrorExtension) maybeExtension)
             throwError qErr


qeInternal of QErr will go into extensions -> internal of the response json. So the response look like

extensions: internal: extensions: response-json-from-webhook

See encodeGQLErr function.

I'd suggest to model the extensions in the QErr type.

+data ErrorExtra + = EEInternal !Value + | EEExtensions !Value + deriving (Show, Eq) + +instance ToJSON ErrorExtra where + toJSON = \case + EEInternal v -> v + EEExtensions v -> v + data QErr = QErr - { qePath :: !JSONPath - , qeStatus :: !N.Status - , qeError :: !T.Text - , qeCode :: !Code - , qeInternal :: !(Maybe Value) + { qePath :: !JSONPath + , qeStatus :: !N.Status + , qeError :: !T.Text + , qeCode :: !Code + , qeExtra :: !(Maybe ErrorExtra) } deriving (Show, Eq) instance ToJSON QErr where @@ -157,12 +167,12 @@ instance ToJSON QErr where , "error" .= msg , "code" .= show code ] - toJSON (QErr jPath _ msg code (Just ie)) = + toJSON (QErr jPath _ msg code (Just extra)) = object [ "path" .= encodeJSONPath jPath , "error" .= msg , "code" .= show code - , "internal" .= ie + , "internal" .= extra ] noInternalQErrEnc :: QErr -> Value @@ -174,18 +184,20 @@ noInternalQErrEnc (QErr jPath _ msg code _) = ] encodeGQLErr :: Bool -> QErr -> Value -encodeGQLErr includeInternal (QErr jPath _ msg code mIE) = +encodeGQLErr includeInternal (QErr jPath _ msg code mExtra) = object [ "message" .= msg , "extensions" .= extnsObj ] where - extnsObj = object $ bool codeAndPath - (codeAndPath ++ internal) includeInternal + extnsObj = case mExtra of + Nothing -> object codeAndPath + Just (EEExtensions v) -> v + Just (EEInternal v) -> object $ + bool codeAndPath (codeAndPath ++ ["internal" .= v]) includeInternal codeAndPath = [ "code" .= show code , "path" .= encodeJSONPath jPath ] - internal = maybe [] (\ie -> ["internal" .= ie]) mIE

rakeshkky

Add python tests. Also try to document the new types the PR introduces. In this case, for the suggested ErrorExtra type,

data ErrorExtra
  = EEInternal !Value 
    -- ^ Additional information for the server internal errors. 
  | EEExtensions !Value 
    -- ^ Custom extensions json value found in actions webhook response. 
  deriving (Show, Eq)

marionschleifer

Changelog approved.

- ErrorExtra is used to give the errors context while deriving to JSON

hasura-bot · 2020-04-09T17:29:59Z

Review app for commit f67eb3a deployed to Heroku: https://hge-ci-pull-4351.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4351-f67eb3ab

hasura-bot · 2020-04-13T13:27:29Z

Review app for commit 37210c8 deployed to Heroku: https://hge-ci-pull-4351.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4351-37210c81

hasura-bot · 2020-04-13T15:36:52Z

Review app for commit 6b5c579 deployed to Heroku: https://hge-ci-pull-4351.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4351-6b5c5798

ecthiender · 2021-09-20T06:55:30Z

Superseded by 8bfcd9a

add a new field extensions in the ActionWebhookErrorResponse

3b85b73

- the extensions can take any json

codingkarthik requested a review from a team as a code owner April 9, 2020 11:54

0x777 requested a review from rakeshkky April 9, 2020 12:11

tirumaraiselvan reviewed Apr 9, 2020

View reviewed changes

codingkarthik added 2 commits April 9, 2020 18:18

remove code from ActionWebhookErrorResponse

628eff5

update docs and CHANGELOG

e6ffc0a

codingkarthik requested review from a team as code owners April 9, 2020 12:48

rakeshkky reviewed Apr 9, 2020

View reviewed changes

rakeshkky requested changes Apr 9, 2020

View reviewed changes

marionschleifer approved these changes Apr 9, 2020

View reviewed changes

codingkarthik added 2 commits April 9, 2020 21:45

add a new data type ErrorExtra

44a4b16

- ErrorExtra is used to give the errors context while deriving to JSON

add tests when a action handler returns an error status

f67eb3a

codingkarthik added 2 commits April 13, 2020 18:38

encode the error to JSON before writing to DB

2e98857

modify the action tests to include extensions in the error handler

37210c8

Merge branch 'master' into actions-error-extensions-4001

6b5c579

ecthiender closed this Sep 20, 2021

add a new field extensions in the ActionWebhookErrorResponse, close #4001 #4351

add a new field extensions in the ActionWebhookErrorResponse, close #4001 #4351

Uh oh!

Conversation

codingkarthik commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Affected components

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Metadata

GraphQL

Breaking changes

Uh oh!

hasura-bot commented Apr 9, 2020

Uh oh!

tirumaraiselvan Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

codingkarthik Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

tirumaraiselvan Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codingkarthik Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hasura-bot commented Apr 9, 2020

Uh oh!

rakeshkky Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

rakeshkky Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rakeshkky left a comment

Choose a reason for hiding this comment

Uh oh!

marionschleifer left a comment

Choose a reason for hiding this comment

Uh oh!

hasura-bot commented Apr 9, 2020

Uh oh!

hasura-bot commented Apr 13, 2020

Uh oh!

hasura-bot commented Apr 13, 2020

Uh oh!

ecthiender commented Sep 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

add a new field `extensions` in the ActionWebhookErrorResponse, close #4001 #4351

add a new field `extensions` in the ActionWebhookErrorResponse, close #4001 #4351

codingkarthik commented Apr 9, 2020 •

edited

Loading

tirumaraiselvan Apr 9, 2020 •

edited

Loading

netlify bot commented Apr 9, 2020 •

edited

Loading

rakeshkky Apr 9, 2020 •

edited

Loading