graphql responses are now http 200 (fix #1368)

docs/graphql/manual/api-reference/index.rst

@@ -11,7 +11,14 @@ GraphQL API
 
 All GraphQL requests for queries, subscriptions and mutations are made to the GraphQL API.
 
-All requests are ``POST`` requests to the ``/v1alpha1/graphql`` endpoint.
+All requests are ``POST`` requests to the ``/v1alpha1/graphql`` and ``/v1/graphql`` endpoint.
+
+.. note::
+
+   ``/v1/graphql`` endpoint returns HTTP 200 status codes for all responses.
+   This is a **breaking** change from ``/v1alpha1/graphql`` behaviour, where
+   request errors and internal errors were responded with 4xx and 5xx status
+   codes.
 
 Request types
 ^^^^^^^^^^^^^

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

@@ -155,8 +155,12 @@ onConn (L.Logger logger) corsPolicy wsId requestHead = do
         (BL.toStrict $ J.encode $ encodeGQLErr False qErr)
 
     checkPath =
-      when (WS.requestPath requestHead /= "/v1alpha1/graphql") $
-      throw404 "only /v1alpha1/graphql is supported on websockets"
+      when (WS.requestPath requestHead `notElem` allowedPaths) $
+        throw404 $ "only [" <> T.intercalate ", " allowedPaths
+                   <> "] is supported on websockets"
+
+    allowedPaths :: (IsString a) => [a]
+    allowedPaths = ["/v1alpha1/graphql", "/v1/graphql"]
 
     getOrigin =
       find ((==) "Origin" . fst) (WS.requestHeaders requestHead)

server/src-lib/Hasura/Server/App.hs

@@ -193,10 +193,11 @@ logError userInfoM req reqBody sc qErr =
 mkSpockAction
   :: (MonadIO m)
   => (Bool -> QErr -> Value)
+  -> (QErr -> QErr)
   -> ServerCtx
   -> Handler EncJSON
   -> ActionT m ()
-mkSpockAction qErrEncoder serverCtx handler = do
+mkSpockAction qErrEncoder qErrModifier serverCtx handler = do
   req <- request
   reqBody <- liftIO $ strictRequestBody req
   let headers  = requestHeaders req
@@ -212,7 +213,7 @@ mkSpockAction qErrEncoder serverCtx handler = do
   result <- liftIO $ runReaderT (runExceptT handler) handlerState
   t2 <- liftIO getCurrentTime -- for measuring response time purposes
 
-  let resLBS = fmap encJToLBS result
+  let resLBS = fmapL qErrModifier $ fmap encJToLBS result
 
   -- log result
   logResult (Just userInfo) req reqBody serverCtx resLBS $ Just (t1, t2)
@@ -273,6 +274,9 @@ v1Alpha1GQHandler query = do
   GH.runGQ pgExecCtx userInfo sqlGenCtx planCache
     sc scVer manager reqHeaders query reqBody
 
+v1GQHandler :: GH.GQLReqUnparsed -> Handler EncJSON
+v1GQHandler = v1Alpha1GQHandler
+
 gqlExplainHandler :: GE.GQLExplain -> Handler EncJSON
 gqlExplainHandler query = do
   onlyAdmin
@@ -375,13 +379,9 @@ httpApp corsCfg serverCtx enableConsole enableTelemetry = do
     -- Health check endpoint
     get "healthz" $ do
       sc <- liftIO $ getSCFromRef $ scCacheRef serverCtx
-      let reportOK = do
-            setStatus N.status200
-            lazyBytes "OK"
-          reportError = do
-            setStatus N.status500
-            lazyBytes "ERROR"
-      bool reportError reportOK $ null $ scInconsistentObjs sc
+      if null $ scInconsistentObjs sc
+        then setStatus N.status200 >> lazyBytes "OK"
+        else setStatus N.status500 >> lazyBytes "ERROR"
 
     get "v1/version" $ do
       uncurry setHeader jsonHeader
@@ -393,23 +393,28 @@ httpApp corsCfg serverCtx enableConsole enableTelemetry = do
       put    ("v1/template" <//> var) tmpltPutOrPostH
       delete ("v1/template" <//> var) tmpltGetOrDeleteH
 
-      post "v1/query" $ mkSpockAction encodeQErr serverCtx $ do
+      post "v1/query" $ mkSpockAction encodeQErr id serverCtx $ do
         query <- parseBody
         v1QueryHandler query
 
       post ("api/1/table" <//> var <//> var) $ \tableName queryType ->
-        mkSpockAction encodeQErr serverCtx $
+        mkSpockAction encodeQErr id serverCtx $
         legacyQueryHandler (TableName tableName) queryType
 
     when enableGraphQL $ do
-      post "v1alpha1/graphql/explain" $ mkSpockAction encodeQErr serverCtx $ do
-        expQuery <- parseBody
-        gqlExplainHandler expQuery
+      post "v1alpha1/graphql/explain" gqlExplainAction
 
-      post "v1alpha1/graphql" $ mkSpockAction GH.encodeGQErr serverCtx $ do
+      post "v1alpha1/graphql" $ mkSpockAction GH.encodeGQErr id serverCtx $ do
         query <- parseBody
         v1Alpha1GQHandler query
 
+      post "v1/graphql/explain" gqlExplainAction
+
+      post "v1/graphql" $ mkSpockAction GH.encodeGQErr allMod200 serverCtx $ do
+        query <- parseBody
+        v1GQHandler query
+
+
 #ifdef InternalAPIs
     get "internal/plan_cache" $ do
       respJ <- liftIO $ E.dumpPlanCache $ scPlanCache serverCtx
@@ -424,15 +429,23 @@ httpApp corsCfg serverCtx enableConsole enableTelemetry = do
       raiseGenericApiError qErr
 
   where
+    -- all graphql errors should be of type 200
+    allMod200 qe = qe { qeStatus = N.status200 }
+
+    gqlExplainAction =
+      mkSpockAction encodeQErr id serverCtx $ do
+        expQuery <- parseBody
+        gqlExplainHandler expQuery
+
     enableGraphQL = isGraphQLEnabled serverCtx
     enableMetadata = isMetadataEnabled serverCtx
     tmpltGetOrDeleteH tmpltName = do
       tmpltArgs <- tmpltArgsFromQueryParams
-      mkSpockAction encodeQErr serverCtx $ mkQTemplateAction tmpltName tmpltArgs
+      mkSpockAction encodeQErr id serverCtx $ mkQTemplateAction tmpltName tmpltArgs
 
     tmpltPutOrPostH tmpltName = do
       tmpltArgs <- tmpltArgsFromQueryParams
-      mkSpockAction encodeQErr serverCtx $ do
+      mkSpockAction encodeQErr id serverCtx $ do
         bodyTmpltArgs <- parseBody
         mkQTemplateAction tmpltName $ M.union bodyTmpltArgs tmpltArgs
 

server/tests-py/queries/graphql_query/permissions/user_cannot_access_remarks_col.yaml

@@ -4,6 +4,12 @@
   headers:
     X-Hasura-Role: user
     X-Hasura-User-Id: '1'
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.author.selectionSet.remarks_internal
+        code: validation-failed
+      message: "field \"remarks_internal\" not found in type: 'author'"
   query:
     query: |
       query {

server/tests-py/test_apis_disabled.py

@@ -27,13 +27,14 @@ def test_metadata_api_1_disabled(self, hge_ctx):
 
 @pytest.mark.skipif(not pytest.config.getoption("--test-graphql-disabled"),
                     reason="--test-graphql-disabled is not set. Cannot run GraphQL disabled tests")
+@pytest.mark.parametrize('endpoint', ['/v1alpha1/graphql', '/v1/graphql'])
 class TestGraphQLDisabled:
 
-    def test_graphql_endpoint_disabled(self, hge_ctx):
-        check_post_404(hge_ctx,'/v1alpha1/graphql')
+    def test_graphql_endpoint_disabled(self, hge_ctx, endpoint):
+        check_post_404(hge_ctx, endpoint)
 
-    def test_graphql_explain_disabled(self, hge_ctx):
-        check_post_404(hge_ctx,'/v1alpha1/graphql/explain')
+    def test_graphql_explain_disabled(self, hge_ctx, endpoint):
+        check_post_404(hge_ctx, endpoint + '/explain')
 
 
 @pytest.mark.skipif(pytest.config.getoption("--test-graphql-disabled"),