server/bigquery: improve throwing upstream exceptions

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4095
GitOrigin-RevId: e19ffe058aaffa1cfa8d155f2e3a6ecafd6aab13

CHANGELOG.md

@@ -99,7 +99,7 @@ the right-hand side for now.
     </tr>
   </tbody>
   </table>
-  
+
   This change is a result of fixing some inconsistencies and edge cases in writing array elements.
   `hasura metadata export` will write YAML files in this format going forward. Also, note that this is a backwards compatible change.
 
@@ -146,6 +146,7 @@ the right-hand side for now.
 
 ### Bug fixes and improvements
 
+- server: improve error messages in BigQuery upstream API exceptions
 - server: Fix regression in MSSQL subscriptions when results exceed 2048 characters (#8267)
 - server: refactor OpenAPI spec generation (for REST endpoints) and improve OpenAPI warnings
 - server: add jsonb to string cast support - postgres (#7818)
@@ -161,9 +162,9 @@ the right-hand side for now.
 - console: enable searching tables within a schema in the sidebar
 - console: add support for setting comments on the custom root fields of tables/views
 - console: add feature flags section in settings
-- console: improved support for setting comments on computed fields 
+- console: improved support for setting comments on computed fields
 - console: fix the ability to create updated_at and created_at in the modify page (#8239)
-- console: fix an issue where editing both a column's name and its GraphQL field name at the same time caused an error 
+- console: fix an issue where editing both a column's name and its GraphQL field name at the same time caused an error
 - console: fix redirect to metadata status page on inconsistent inherited role (#8343)
 - console: fix malformed request with REST live preview section (#8316)
 - cli: add support for customization field in sources metadata (#8292)

server/graphql-engine.cabal

@@ -1060,3 +1060,4 @@ test-suite tests-hspec
     Test.ServiceLivenessSpec
     Test.ViewsSpec
     Test.WhereSpec
+    Test.RunSQLSpec

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

@@ -57,9 +57,14 @@ data TokenProblem
   | BearerTokenSignsaferProblem Cry.Error
   | TokenFetchProblem JSONException
   | TokenRequestNonOK Status
-  deriving (Show)
+  deriving (Show, Generic)
 
-instance Exception TokenProblem
+tokenProblemMessage :: TokenProblem -> Text
+tokenProblemMessage = \case
+  BearerTokenDecodeProblem _ -> "Cannot decode bearer token"
+  BearerTokenSignsaferProblem _ -> "Cannot sign bearer token"
+  TokenFetchProblem _ -> "JSON exception occurred while fetching token"
+  TokenRequestNonOK status -> "HTTP request to fetch token failed with status " <> tshow status
 
 data ServiceAccountProblem
   = ServiceAccountFileDecodeProblem String
@@ -194,7 +199,11 @@ getUsableToken BigQueryConnection {_bqServiceAccount, _bqAccessTokenMVar} =
 
 data BigQueryProblem
   = TokenProblem TokenProblem
-  deriving (Show)
+  deriving (Show, Generic)
+
+instance J.ToJSON BigQueryProblem where
+  toJSON (TokenProblem tokenProblem) =
+    J.object ["token_problem" J..= tokenProblemMessage tokenProblem]
 
 runBigQuery ::
   (MonadIO m) =>

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

@@ -80,7 +80,9 @@ runSQL_ f (BigQueryRunSQL query source) = do
       (_scConnection sourceConfig)
       Execute.BigQuery {query = LT.fromStrict query, parameters = mempty}
   case result of
-    Left queryError -> throw400 BigQueryError (tshow queryError) -- TODO: Pretty print the error type.
+    Left executeProblem -> do
+      let errorMessage = Execute.executeProblemMessage executeProblem
+      throwError (err400 BigQueryError errorMessage) {qeInternal = Just $ ExtraInternal $ J.toJSON executeProblem}
     Right recordSet ->
       pure
         ( encJFromJValue

server/src-lib/Hasura/Backends/BigQuery/Execute.hs

@@ -8,10 +8,12 @@ module Hasura.Backends.BigQuery.Execute
     runExecute,
     streamBigQuery,
     executeBigQuery,
+    executeProblemMessage,
     BigQuery (..),
     OutputValue (..),
     RecordSet (..),
     Execute,
+    ExecuteProblem (..),
     Value (..),
     FieldNameText (..),
   )
@@ -19,13 +21,12 @@ where
 
 import Control.Applicative
 import Control.Concurrent
-import Control.Exception.Safe
 import Control.Monad.Except
 import Control.Monad.Reader
 import Data.Aeson ((.!=), (.:), (.:?), (.=))
 import Data.Aeson qualified as Aeson
 import Data.Aeson.Types qualified as Aeson
-import Data.ByteString.Lazy qualified as L
+import Data.ByteString.Lazy qualified as BL
 import Data.Foldable
 import Data.HashMap.Strict.InsOrd qualified as OMap
 import Data.Maybe
@@ -111,12 +112,24 @@ data ExecuteReader = ExecuteReader
 data ExecuteProblem
   = GetJobDecodeProblem String
   | CreateQueryJobDecodeProblem String
-  | ErrorResponseFromServer Status L.ByteString
-  | GetJobResultsProblem SomeException
-  | RESTRequestNonOK Status Text
-  | CreateQueryJobProblem SomeException
   | ExecuteRunBigQueryProblem BigQueryProblem
-  deriving (Show)
+  | RESTRequestNonOK Status Aeson.Value
+  deriving (Generic)
+
+instance Aeson.ToJSON ExecuteProblem where
+  toJSON =
+    Aeson.object . \case
+      GetJobDecodeProblem err -> ["get_job_decode_problem" Aeson..= err]
+      CreateQueryJobDecodeProblem err -> ["create_query_job_decode_problem" Aeson..= err]
+      ExecuteRunBigQueryProblem problem -> ["execute_run_bigquery_problem" Aeson..= problem]
+      RESTRequestNonOK _ resp -> ["rest_request_non_ok" Aeson..= resp]
+
+executeProblemMessage :: ExecuteProblem -> Text
+executeProblemMessage = \case
+  GetJobDecodeProblem err -> "Fetching bigquery job status, cannot decode  HTTP response; " <> tshow err
+  CreateQueryJobDecodeProblem err -> "Creating bigquery job, cannot decode HTTP response: " <> tshow err
+  ExecuteRunBigQueryProblem _ -> "Cannot execute bigquery request"
+  RESTRequestNonOK status _ -> "Bigquery HTTP request failed with status code " <> tshow (statusCode status) <> " and status message " <> tshow (statusMessage status)
 
 -- | Execute monad; as queries are performed, the record sets are
 -- stored in the map.
@@ -458,7 +471,7 @@ getJobResults ::
   Fetch ->
   m (Either ExecuteProblem JobResultsResponse)
 getJobResults conn Job {jobId, location} Fetch {pageToken} =
-  liftIO (catchAny run (pure . Left . GetJobResultsProblem))
+  liftIO run
   where
     -- https://cloud.google.com/bigquery/docs/reference/rest/v2/jobs/get#query-parameters
     url =
@@ -484,7 +497,7 @@ getJobResults conn Job {jobId, location} Fetch {pageToken} =
               Left e -> pure (Left (GetJobDecodeProblem e))
               Right results -> pure (Right results)
             _ -> do
-              pure $ Left $ RESTRequestNonOK (getResponseStatus resp) $ lbsToTxt $ getResponseBody resp
+              pure $ Left $ RESTRequestNonOK (getResponseStatus resp) $ parseAsJsonOrText $ getResponseBody resp
     extraParameters = pageTokenParam
       where
         pageTokenParam =
@@ -526,11 +539,7 @@ instance Aeson.FromJSON Job where
 -- | Create a job asynchronously.
 createQueryJob :: MonadIO m => BigQueryConnection -> BigQuery -> m (Either ExecuteProblem Job)
 createQueryJob conn BigQuery {..} =
-  liftIO
-    ( do
-        -- putStrLn (LT.unpack query)
-        catchAny run (pure . Left . CreateQueryJobProblem)
-    )
+  liftIO run
   where
     run = do
       let url =
@@ -551,7 +560,7 @@ createQueryJob conn BigQuery {..} =
                 Left e -> pure (Left (CreateQueryJobDecodeProblem e))
                 Right job -> pure (Right job)
             _ -> do
-              pure $ Left $ RESTRequestNonOK (getResponseStatus resp) $ lbsToTxt $ getResponseBody resp
+              pure $ Left $ RESTRequestNonOK (getResponseStatus resp) $ parseAsJsonOrText $ getResponseBody resp
     body =
       Aeson.encode
         ( Aeson.object
@@ -578,6 +587,11 @@ createQueryJob conn BigQuery {..} =
             ]
         )
 
+-- | Parse given @'ByteString' as JSON value. If not a valid JSON, encode to plain text.
+parseAsJsonOrText :: BL.ByteString -> Aeson.Value
+parseAsJsonOrText bytestring =
+  fromMaybe (Aeson.String $ lbsToTxt bytestring) $ Aeson.decode bytestring
+
 --------------------------------------------------------------------------------
 -- Consuming recordset from big query
 

server/src-lib/Hasura/Backends/BigQuery/Instances/Execute.hs

@@ -10,6 +10,7 @@ import Data.Text qualified as T
 import Data.Text.Lazy qualified as LT
 import Data.Text.Lazy.Builder qualified as LT
 import Data.Vector qualified as V
+import Hasura.Backends.BigQuery.Execute (executeProblemMessage)
 import Hasura.Backends.BigQuery.Execute qualified as DataLoader
 import Hasura.Backends.BigQuery.FromIr qualified as BigQuery
 import Hasura.Backends.BigQuery.Plan
@@ -72,7 +73,7 @@ bqDBQueryPlan userInfo sourceName sourceConfig qrf = do
             sourceConfig
             (DataLoader.executeSelect select)
         case result of
-          Left err -> throw500WithDetail "dataLoader error" $ Aeson.toJSON $ show err
+          Left err -> throw500WithDetail (executeProblemMessage err) $ Aeson.toJSON err
           Right recordSet -> pure $! recordSetToEncJSON (BigQuery.selectCardinality select) recordSet
   pure $ DBStepInfo @'BigQuery sourceName sourceConfig (Just (selectSQLTextForExplain select)) action
 

server/tests-hspec/Harness/Backend/BigQuery.hs

@@ -27,15 +27,15 @@ import Data.Text.Extended (commaSeparated)
 import GHC.Stack
 import Harness.Constants as Constants
 import Harness.Env
-import Harness.Exceptions (SomeException, handle)
 import Harness.GraphqlEngine qualified as GraphqlEngine
 import Harness.Quoter.Yaml (yaml)
 import Harness.State (State)
 import Harness.Test.Context (BackendType (BigQuery), defaultBackendTypeString, defaultSource)
 import Harness.Test.Schema qualified as Schema
 import Hasura.Backends.BigQuery.Connection (initConnection)
-import Hasura.Backends.BigQuery.Execute qualified as Execute (BigQuery (..), executeBigQuery)
+import Hasura.Backends.BigQuery.Execute qualified as Execute
 import Hasura.Backends.BigQuery.Source (ServiceAccount)
+import Hasura.Prelude (onLeft)
 import Prelude
 
 getServiceAccount :: HasCallStack => IO ServiceAccount
@@ -47,20 +47,17 @@ getProjectId = getEnvString Constants.bigqueryProjectIdVar
 -- | Run a plain Standard SQL string against the server, ignore the
 -- result. Just checks for errors.
 run_ :: (HasCallStack) => ServiceAccount -> Text -> String -> IO ()
-run_ serviceAccount projectId query =
-  handle (\(e :: SomeException) -> bigQueryError e query) $ do
-    conn <- initConnection serviceAccount projectId Nothing
-    res <- Execute.executeBigQuery conn Execute.BigQuery {Execute.query = fromString query, Execute.parameters = mempty}
-    case res of
-      Left err -> bigQueryError err query
-      Right () -> pure ()
+run_ serviceAccount projectId query = do
+  conn <- initConnection serviceAccount projectId Nothing
+  res <- Execute.executeBigQuery conn Execute.BigQuery {Execute.query = fromString query, Execute.parameters = mempty}
+  res `onLeft` (`bigQueryError` query)
 
-bigQueryError :: (Show e, HasCallStack) => e -> String -> IO ()
+bigQueryError :: HasCallStack => Execute.ExecuteProblem -> String -> IO ()
 bigQueryError e query =
   error
     ( unlines
         [ "BigQuery query error:",
-          show e,
+          T.unpack (Execute.executeProblemMessage e),
           "SQL was:",
           query
         ]

server/tests-hspec/Harness/GraphqlEngine.hs

@@ -16,6 +16,7 @@ module Harness.GraphqlEngine
     postGraphql,
     postGraphqlWithHeaders,
     clearMetadata,
+    postV2Query,
 
     -- ** Misc.
     setSource,
@@ -85,8 +86,18 @@ post_ state path = void . withFrozenCallStack . postWithHeaders_ state path memp
 -- Note: We add 'withFrozenCallStack' to reduce stack trace clutter.
 postWithHeaders ::
   HasCallStack => State -> String -> Http.RequestHeaders -> Value -> IO Value
-postWithHeaders (getServer -> Server {urlPrefix, port}) path headers =
-  withFrozenCallStack . Http.postValue (urlPrefix ++ ":" ++ show port ++ path) headers
+postWithHeaders = withFrozenCallStack . postWithHeadersStatus 200
+
+-- | Post some JSON to graphql-engine, getting back more JSON.
+--
+-- Expecting non-200 status code; use @'postWithHeaders' if you want to test for
+-- success reponse.
+--
+-- Note: We add 'withFrozenCallStack' to reduce stack trace clutter.
+postWithHeadersStatus ::
+  HasCallStack => Int -> State -> String -> Http.RequestHeaders -> Value -> IO Value
+postWithHeadersStatus statusCode (getServer -> Server {urlPrefix, port}) path headers =
+  withFrozenCallStack . Http.postValueWithStatus statusCode (urlPrefix ++ ":" ++ show port ++ path) headers
 
 -- | Post some JSON to graphql-engine, getting back more JSON.
 --
@@ -143,6 +154,15 @@ postMetadata_ state = withFrozenCallStack $ post_ state "/v1/metadata"
 clearMetadata :: HasCallStack => State -> IO ()
 clearMetadata s = withFrozenCallStack $ postMetadata_ s [yaml|{type: clear_metadata, args: {}}|]
 
+-- | Same as 'postWithHeadersStatus', but defaults to the @"/v2/query"@ endpoint
+--
+-- @headers@ are mostly irrelevant for the admin endpoint @v2/query@.
+--
+-- Note: We add 'withFrozenCallStack' to reduce stack trace clutter.
+postV2Query :: HasCallStack => Int -> State -> Value -> IO Value
+postV2Query statusCode state =
+  withFrozenCallStack $ postWithHeadersStatus statusCode state "/v2/query" mempty
+
 -------------------------------------------------------------------------------
 
 -- HTTP Calls - Misc.

server/tests-hspec/Harness/Http.hs

@@ -2,6 +2,7 @@
 module Harness.Http
   ( get_,
     postValue,
+    postValueWithStatus,
     healthCheck,
     Http.RequestHeaders,
   )
@@ -33,7 +34,12 @@ get_ url = do
 -- | Post the JSON to the given URL, and produces a very descriptive
 -- exception on failure.
 postValue :: HasCallStack => String -> Http.RequestHeaders -> Value -> IO Value
-postValue url headers value = do
+postValue = postValueWithStatus 200
+
+-- | Post the JSON to the given URL and expected HTTP response code.
+-- Produces a very descriptive exception or failure.
+postValueWithStatus :: HasCallStack => Int -> String -> Http.RequestHeaders -> Value -> IO Value
+postValueWithStatus statusCode url headers value = do
   let request =
         Http.setRequestHeaders headers $
           Http.setRequestMethod Http.methodPost $
@@ -41,7 +47,8 @@ postValue url headers value = do
   response <- Http.httpLbs request
   let requestBodyString = L8.unpack $ encode value
       responseBodyString = L8.unpack $ Http.getResponseBody response
-  if Http.getResponseStatusCode response == 200
+      responseStatusCode = Http.getResponseStatusCode response
+  if responseStatusCode == statusCode
     then
       eitherDecode (Http.getResponseBody response)
         `onLeft` \err ->
@@ -56,7 +63,11 @@ postValue url headers value = do
             ]
     else
       reportError
-        [ "Non-200 response code from HTTP request: ",
+        [ "Expecting reponse code ",
+          show statusCode,
+          " but got ",
+          show responseStatusCode,
+          " from HTTP request: ",
           url,
           "With body:",
           requestBodyString,

server/tests-hspec/Test/RunSQLSpec.hs

@@ -0,0 +1,63 @@
+{-# LANGUAGE QuasiQuotes #-}
+
+-- | Test *_run_sql query API
+module Test.RunSQLSpec (spec) where
+
+import Harness.Backend.BigQuery qualified as BigQuery
+import Harness.GraphqlEngine qualified as GraphqlEngine
+import Harness.Quoter.Yaml
+import Harness.State (State)
+import Harness.Test.Context qualified as Context
+import Test.Hspec
+import Prelude
+
+--------------------------------------------------------------------------------
+-- Preamble
+spec :: SpecWith State
+spec =
+  Context.run
+    [ Context.Context
+        { name = Context.Backend Context.BigQuery,
+          mkLocalState = Context.noLocalState,
+          setup = BigQuery.setup [],
+          teardown = BigQuery.teardown [],
+          customOptions = Nothing
+        }
+    ]
+    tests
+
+--------------------------------------------------------------------------------
+-- Tests
+
+tests :: Context.Options -> SpecWith State
+tests opts = do
+  it "BigQuery - running invalid SQL" \state ->
+    shouldReturnYaml
+      opts
+      ( GraphqlEngine.postV2Query
+          400
+          state
+          [yaml|
+type: bigquery_run_sql
+args:
+  source: bigquery
+  sql: 'some invalid SQL'
+|]
+      )
+      [yaml|
+internal:
+  rest_request_non_ok:
+    error:
+      status: INVALID_ARGUMENT
+      code: 400
+      message: 'Syntax error: Expected end of input but got keyword SOME at [1:1]'
+      errors:
+      - location: q
+        domain: global
+        reason: invalidQuery
+        locationType: parameter
+        message: 'Syntax error: Expected end of input but got keyword SOME at [1:1]'
+path: "$"
+error: Bigquery HTTP request failed with status code 400 and status message "Bad Request"
+code: bigquery-error
+|]