Skip to content

Commit d421cdb

Browse files
daniel-chambershasura-bot
daniel-chambers
authored and
hasura-bot
committed
Put the message property first when encoding error JSON
PR-URL: hasura/graphql-engine-mono#9379 GitOrigin-RevId: 385035810190786f54a1db86e3a2e4a2c3bd5dee
1 parent 0034ae8 commit d421cdb

File tree

11 files changed

+78
-64
lines changed

11 files changed

+78
-64
lines changed

‎cli/internal/hasura/v1graphql/v1graphql_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ func TestClient_GetIntrospectionSchema(t *testing.T) {
6767
require.Equal(tt, errors.KindHasuraAPI.String(), errors.GetKind(err).String())
6868
require.EqualError(tt, err.Err, ` getIntrospectionSchema : 404
6969
{
70-
"code": "not-found",
7170
"error": "resource does not exist",
72-
"path": "$"
71+
"path": "$",
72+
"code": "not-found"
7373
}`)
7474
}),
7575
},

‎server/lib/hasura-base/src/Hasura/Base/Error.hs‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ module Hasura.Base.Error
1010
showQErr,
1111
encodeQErr,
1212
encodeGQLErr,
13-
noInternalQErrEnc,
1413
err400,
1514
err404,
1615
err405,
@@ -61,6 +60,7 @@ where
6160
import Control.Arrow.Extended
6261
import Control.Lens (makeLensesFor, makePrisms)
6362
import Data.Aeson
63+
import Data.Aeson.Encoding qualified as J
6464
import Data.Aeson.Internal
6565
import Data.Aeson.Key qualified as K
6666
import Data.Aeson.Types
@@ -196,23 +196,31 @@ instance ToJSON QErrExtra where
196196
HideInconsistencies -> Null
197197

198198
instance ToJSON QErr where
199-
toJSON (QErr jPath _ msg code Nothing) =
200-
object
201-
[ "path" .= encodeJSONPath jPath,
202-
"error" .= msg,
203-
"code" .= code
204-
]
205-
toJSON (QErr jPath _ msg code (Just extra)) = object
206-
$ case extra of
207-
ExtraInternal e -> err ++ ["internal" .= e]
208-
ExtraExtensions {} -> err
209-
HideInconsistencies -> []
199+
toJSON (QErr jPath _ msg code extra) =
200+
object $ case extra of
201+
Just (ExtraInternal e) -> "internal" .= e : err
202+
Just ExtraExtensions {} -> err
203+
Just HideInconsistencies -> []
204+
Nothing -> err
210205
where
211206
err =
212-
[ "path" .= encodeJSONPath jPath,
213-
"error" .= msg,
207+
[ "error" .= msg,
208+
"path" .= encodeJSONPath jPath,
214209
"code" .= code
215210
]
211+
toEncoding (QErr jPath _ msg code extra) =
212+
pairs $ case extra of
213+
Just (ExtraInternal e) -> err <> "internal" .= e -- Internal comes after all other properties so is the last thing the user sees
214+
Just ExtraExtensions {} -> err
215+
Just HideInconsistencies -> mempty
216+
Nothing -> err
217+
where
218+
err =
219+
-- error property comes first so the error message is the first
220+
-- thing the user sees
221+
("error" .= msg)
222+
<> ("path" .= encodeJSONPath jPath)
223+
<> ("code" .= code)
216224

217225
-- | Overrides the status and code of a QErr while retaining all other fields.
218226
overrideQErrStatus :: HTTP.Status -> Code -> QErr -> QErr
@@ -226,41 +234,32 @@ prefixQErr prefix err = err {qeError = prefix <> qeError err}
226234
showQErr :: QErr -> Text
227235
showQErr = TL.toStrict . TL.decodeUtf8 . encode
228236

229-
noInternalQErrEnc :: QErr -> Value
230-
noInternalQErrEnc (QErr jPath _ msg code _) =
231-
object
232-
[ "path" .= encodeJSONPath jPath,
233-
"error" .= msg,
234-
"code" .= code
235-
]
236-
237-
encodeGQLErr :: Bool -> QErr -> Value
237+
encodeGQLErr :: Bool -> QErr -> Encoding
238238
encodeGQLErr includeInternal (QErr jPath _ msg code maybeExtra) =
239-
object
240-
[ "message" .= msg,
241-
"extensions" .= extnsObj
242-
]
239+
pairs (("message" .= msg) <> (J.pair "extensions" extnsObj))
243240
where
244-
appendIf cond a b = if cond then a ++ b else a
241+
appendIf cond a b = if cond then a <> b else a
245242

246243
extnsObj = case maybeExtra of
247-
Nothing -> object codeAndPath
244+
Nothing -> pairs codeAndPath
248245
-- if an `extensions` key is given in the error response from the webhook,
249246
-- we ignore the `code` key regardless of whether the `extensions` object
250247
-- contains a `code` field:
251-
Just (ExtraExtensions v) -> v
248+
Just (ExtraExtensions v) -> toEncoding v
252249
Just (ExtraInternal v) ->
253-
object $ appendIf includeInternal codeAndPath ["internal" .= v]
254-
Just HideInconsistencies -> Null
250+
pairs $ appendIf includeInternal codeAndPath ("internal" .= v)
251+
Just HideInconsistencies -> toEncoding Null
255252
codeAndPath =
256-
[ "path" .= encodeJSONPath jPath,
257-
"code" .= code
258-
]
253+
("path" .= encodeJSONPath jPath)
254+
<> ("code" .= code)
259255

260256
-- whether internal should be included or not
261-
encodeQErr :: Bool -> QErr -> Value
262-
encodeQErr True = toJSON
263-
encodeQErr _ = noInternalQErrEnc
257+
encodeQErr :: Bool -> QErr -> Encoding
258+
encodeQErr True = toEncoding
259+
encodeQErr False = toEncoding . removeInternalErr
260+
where
261+
removeInternalErr :: QErr -> QErr
262+
removeInternalErr err = err {qeInternal = Nothing}
264263

265264
-- Postgres Connection Errors
266265
instance PG.FromPGConnErr QErr where

‎server/lib/hasura-json-encoding/src/Hasura/EncJSON.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ module Hasura.EncJSON
77
encJToLBS,
88
encJToBS,
99
encJFromJValue,
10+
encJFromJEncoding,
1011
encJFromChar,
1112
encJFromText,
1213
encJFromNonEmptyText,
@@ -150,6 +151,10 @@ encJFromJValue :: (J.ToJSON a) => a -> EncJSON
150151
encJFromJValue = encJFromBuilder . J.fromEncoding . J.toEncoding
151152
{-# INLINE encJFromJValue #-}
152153

154+
encJFromJEncoding :: J.Encoding -> EncJSON
155+
encJFromJEncoding = encJFromBuilder . J.fromEncoding
156+
{-# INLINE encJFromJEncoding #-}
157+
153158
encJFromChar :: Char -> EncJSON
154159
encJFromChar = EncJSON . BB.charUtf8
155160
{-# INLINE encJFromChar #-}

‎server/src-lib/Hasura/Backends/DataConnector/Adapter/Execute.hs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import Hasura.Backends.DataConnector.Plan.MutationPlan qualified as Plan
2121
import Hasura.Backends.DataConnector.Plan.QueryPlan qualified as Plan
2222
import Hasura.Backends.DataConnector.Plan.RemoteRelationshipPlan qualified as Plan
2323
import Hasura.Base.Error (Code (..), QErr, throw400)
24-
import Hasura.EncJSON (EncJSON, encJFromBuilder, encJFromJValue)
24+
import Hasura.EncJSON (EncJSON, encJFromJEncoding, encJFromJValue)
2525
import Hasura.GraphQL.Execute.Backend (BackendExecute (..), DBStepInfo (..), ExplainPlan (..), OnBaseMonad (..), withNoStatistics)
2626
import Hasura.GraphQL.Namespace qualified as GQL
2727
import Hasura.Prelude
@@ -114,7 +114,7 @@ buildQueryAction :: (MonadIO m, MonadTrace m, MonadError QErr m) => RQL.SourceNa
114114
buildQueryAction sourceName SourceConfig {..} Plan {..} = do
115115
queryResponse <- Client.query sourceName _scConfig _pRequest
116116
reshapedResponse <- Tracing.newSpan "QueryResponse reshaping" $ _pResponseReshaper queryResponse
117-
pure . encJFromBuilder $ J.fromEncoding reshapedResponse
117+
pure $ encJFromJEncoding reshapedResponse
118118

119119
-- Delegates the generation to the Agent's /explain endpoint if it has that capability,
120120
-- otherwise, returns the IR sent to the agent.
@@ -139,4 +139,4 @@ buildMutationAction :: (MonadIO m, MonadTrace m, MonadError QErr m) => RQL.Sourc
139139
buildMutationAction sourceName SourceConfig {..} Plan {..} = do
140140
mutationResponse <- Client.mutation sourceName _scConfig _pRequest
141141
reshapedResponse <- Tracing.newSpan "MutationResponse reshaping" $ _pResponseReshaper mutationResponse
142-
pure . encJFromBuilder $ J.fromEncoding reshapedResponse
142+
pure $ encJFromJEncoding reshapedResponse

‎server/src-lib/Hasura/GraphQL/Transport/HTTP.hs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,7 @@ extractFieldFromResponse fieldName resultCustomizer resp = do
674674
return fieldVal
675675
where
676676
do400 = withExceptT Right . throw400 RemoteSchemaError
677-
doGQExecError = withExceptT Left . throwError . GQExecError
677+
doGQExecError = withExceptT Left . throwError . GQExecError . fmap J.toEncoding
678678

679679
buildRaw :: (Applicative m) => JO.Value -> m AnnotatedResponsePart
680680
buildRaw json = do
@@ -740,7 +740,7 @@ runGQBatched env sqlGenCtx sc scVer enableAL readOnlyMode prometheusMetrics logg
740740
removeHeaders =
741741
flip HttpResponse []
742742
. encJFromList
743-
. map (either (encJFromJValue . encodeGQErr includeInternal) _hrBody)
743+
. map (either (encJFromJEncoding . encodeGQErr includeInternal) _hrBody)
744744
responses <- for reqs \req -> fmap (req,) $ try $ (fmap . fmap . fmap) snd $ runGQ env sqlGenCtx sc scVer enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHdrs queryType req
745745
let requestsOperationLogs = map fst $ rights $ map snd responses
746746
batchOperationLogs =

‎server/src-lib/Hasura/GraphQL/Transport/HTTP/Protocol.hs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ module Hasura.GraphQL.Transport.HTTP.Protocol
1515
OperationName (..),
1616
VariableValues,
1717
encodeGQErr,
18+
encodeGQExecError,
1819
encodeGQResp,
1920
decodeGQResp,
2021
encodeHTTPResp,
@@ -28,6 +29,7 @@ where
2829

2930
import Data.Aeson qualified as J
3031
import Data.Aeson.Casing qualified as J
32+
import Data.Aeson.Encoding qualified as J
3133
import Data.Aeson.KeyMap qualified as KM
3234
import Data.Aeson.TH qualified as J
3335
import Data.ByteString.Lazy qualified as BL
@@ -190,25 +192,28 @@ toParsed req = case G.parseExecutableDoc gqlText of
190192
where
191193
gqlText = _unGQLQueryText $ _grQuery req
192194

193-
encodeGQErr :: Bool -> QErr -> J.Value
195+
encodeGQErr :: Bool -> QErr -> J.Encoding
194196
encodeGQErr includeInternal qErr =
195-
J.object ["errors" J..= [encodeGQLErr includeInternal qErr]]
197+
J.pairs (J.pair "errors" $ J.list id [encodeGQLErr includeInternal qErr])
196198

197199
type GQResult a = Either GQExecError a
198200

199-
newtype GQExecError = GQExecError [J.Value]
200-
deriving (Show, Eq, J.ToJSON)
201+
newtype GQExecError = GQExecError [J.Encoding]
202+
deriving (Show, Eq)
201203

202204
type GQResponse = GQResult BL.ByteString
203205

204206
isExecError :: GQResult a -> Bool
205207
isExecError = isLeft
206208

209+
encodeGQExecError :: GQExecError -> J.Encoding
210+
encodeGQExecError (GQExecError errs) = J.list id errs
211+
207212
encodeGQResp :: GQResponse -> EncJSON
208213
encodeGQResp gqResp =
209214
encJFromAssocList $ case gqResp of
210215
Right r -> [("data", encJFromLbsWithoutSoh r)]
211-
Left e -> [("data", encJFromBuilder "null"), ("errors", encJFromJValue e)]
216+
Left e -> [("data", encJFromBuilder "null"), ("errors", encJFromJEncoding $ encodeGQExecError e)]
212217

213218
-- We don't want to force the `Maybe GQResponse` unless absolutely necessary
214219
-- Decode EncJSON from Cache for HTTP endpoints
@@ -227,4 +232,4 @@ decodeGQResp encJson =
227232
encodeHTTPResp :: GQResponse -> EncJSON
228233
encodeHTTPResp = \case
229234
Right r -> encJFromLBS r
230-
Left e -> encJFromJValue e
235+
Left e -> encJFromJEncoding $ encodeGQExecError e

‎server/src-lib/Hasura/GraphQL/Transport/WSServerApp.hs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import Control.Concurrent.Async.Lifted.Safe qualified as LA
99
import Control.Concurrent.STM qualified as STM
1010
import Control.Exception.Lifted
1111
import Control.Monad.Trans.Control qualified as MC
12-
import Data.Aeson (object, toJSON, (.=))
12+
import Data.Aeson qualified as J
13+
import Data.Aeson.Encoding qualified as J
1314
import Data.ByteString.Char8 qualified as B (pack)
1415
import Data.Text (pack)
1516
import Hasura.App.State
@@ -18,6 +19,7 @@ import Hasura.CredentialCache
1819
import Hasura.GraphQL.Execute qualified as E
1920
import Hasura.GraphQL.Logging
2021
import Hasura.GraphQL.Transport.HTTP (MonadExecuteQuery)
22+
import Hasura.GraphQL.Transport.HTTP.Protocol (encodeGQExecError)
2123
import Hasura.GraphQL.Transport.Instances ()
2224
import Hasura.GraphQL.Transport.WebSocket
2325
import Hasura.GraphQL.Transport.WebSocket.Protocol
@@ -151,7 +153,7 @@ mkWSActions logger subProtocol =
151153
mkPostExecErrMessageAction wsConn opId execErr =
152154
sendMsg wsConn $ case subProtocol of
153155
Apollo -> SMData $ DataMsg opId $ throwError execErr
154-
GraphQLWS -> SMErr $ ErrorMsg opId $ toJSON execErr
156+
GraphQLWS -> SMErr $ ErrorMsg opId $ encodeGQExecError execErr
155157

156158
mkOnErrorMessageAction wsConn err mErrMsg =
157159
case subProtocol of
@@ -163,7 +165,7 @@ mkWSActions logger subProtocol =
163165

164166
mkConnectionCloseAction wsConn opId errMsg =
165167
when (subProtocol == GraphQLWS)
166-
$ sendCloseWithMsg logger wsConn (GenericError4400 errMsg) (Just . SMErr $ ErrorMsg opId $ toJSON (pack errMsg)) (Just 1000)
168+
$ sendCloseWithMsg logger wsConn (GenericError4400 errMsg) (Just . SMErr $ ErrorMsg opId $ J.toEncoding (pack errMsg)) (Just 1000)
167169

168170
getServerMsgType = case subProtocol of
169171
Apollo -> SMData
@@ -180,5 +182,5 @@ mkWSActions logger subProtocol =
180182
}
181183

182184
fmtErrorMessage errMsgs = case subProtocol of
183-
Apollo -> object ["errors" .= errMsgs]
184-
GraphQLWS -> toJSON errMsgs
185+
Apollo -> J.pairs (J.pair "errors" $ J.list id errMsgs)
186+
GraphQLWS -> J.list id errMsgs

‎server/src-lib/Hasura/GraphQL/Transport/WebSocket.hs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import Control.Monad.Morph (hoist)
2626
import Control.Monad.Trans.Control qualified as MC
2727
import Data.Aeson qualified as J
2828
import Data.Aeson.Casing qualified as J
29+
import Data.Aeson.Encoding qualified as J
2930
import Data.Aeson.TH qualified as J
3031
import Data.ByteString (ByteString)
3132
import Data.ByteString.Lazy qualified as LBS
@@ -356,7 +357,7 @@ onConn wsId requestHead ipAddress onConnHActions = do
356357
(HTTP.statusCode $ qeStatus qErr)
357358
(HTTP.statusMessage $ qeStatus qErr)
358359
[]
359-
(LBS.toStrict $ J.encode $ encodeGQLErr False qErr)
360+
(LBS.toStrict $ J.encodingToLazyByteString $ encodeGQLErr False qErr)
360361

361362
checkPath = case WS.requestPath requestHead of
362363
"/v1alpha1/graphql" -> return (ERTLegacy, E.QueryHasura)

‎server/src-lib/Hasura/GraphQL/Transport/WebSocket/Protocol.hs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ data DataMsg = DataMsg
181181

182182
data ErrorMsg = ErrorMsg
183183
{ _emId :: !OperationId,
184-
_emPayload :: !J.Value
184+
_emPayload :: !J.Encoding
185185
}
186186
deriving (Show, Eq)
187187

@@ -260,7 +260,7 @@ encodeServerMsg msg =
260260
SMErr (ErrorMsg opId payload) ->
261261
[ encTy SMT_GQL_ERROR,
262262
("id", encJFromJValue opId),
263-
("payload", encJFromJValue payload)
263+
("payload", encJFromJEncoding payload)
264264
]
265265
SMComplete compMsg ->
266266
[ encTy SMT_GQL_COMPLETE,

‎server/src-lib/Hasura/GraphQL/Transport/WebSocket/Server.hs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ data WSActions a = WSActions
320320
_wsaKeepAliveAction :: !(WSKeepAliveMessageAction a),
321321
_wsaGetDataMessageType :: !(DataMsg -> ServerMsg),
322322
_wsaAcceptRequest :: !WS.AcceptRequest,
323-
_wsaErrorMsgFormat :: !([J.Value] -> J.Value)
323+
_wsaErrorMsgFormat :: !([J.Encoding] -> J.Encoding)
324324
}
325325

326326
data WSErrorMessage = ClientMessageParseFailed | ConnInitFailed

0 commit comments

Comments
 (0)