Skip to content

Commit

Permalink
add config to prioritize data/error for remote schema fields
Browse files Browse the repository at this point in the history
PR-URL: hasura/graphql-engine-mono#10528
Co-authored-by: Rob Dominguez <[email protected]>
GitOrigin-RevId: 49d0d7cbcc73fb0876d7ac3f5a1a8ff61ff800e8
  • Loading branch information
2 people authored and hasura-bot committed Dec 19, 2023
1 parent e27e5b7 commit 7f1f060
Show file tree
Hide file tree
Showing 30 changed files with 484 additions and 33 deletions.
26 changes: 26 additions & 0 deletions .circleci/test-server.sh
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,32 @@ remote-schema-permissions)
kill_hge_servers
;;

remote-schema-prioritize-data)
echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH REMOTE SCHEMA PRIORITIZE DATA/ERRORS ########>\n"
export HASURA_GRAPHQL_ADMIN_SECRET="HGE$RANDOM$RANDOM"

run_hge_with_args serve
wait_for_port 8080

pytest "${PYTEST_COMMON_ARGS[@]}" \
test_remote_schema_prioritize_none.py

kill_hge_servers

export HASURA_GRAPHQL_REMOTE_SCHEMA_PRIORITIZE_DATA=true

run_hge_with_args serve
wait_for_port 8080

pytest "${PYTEST_COMMON_ARGS[@]}" \
test_remote_schema_prioritize_data.py

unset HASURA_GRAPHQL_REMOTE_SCHEMA_PRIORITIZE_DATA

kill_hge_servers

;;

function-permissions)
echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH FUNCTION PERMISSIONS ENABLED ########>\n"
export HASURA_GRAPHQL_INFER_FUNCTION_PERMISSIONS=false
Expand Down
13 changes: 13 additions & 0 deletions docs/docs/deployment/graphql-engine-flags/reference.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,19 @@ The Redis URL to use for [query caching](/caching/enterprise-caching.mdx) and
| **Example** | `redis://username:password@host:port/db` |
| **Supported in** | Enterprise Edition only |

### Remote Schema prioritize data

Setting this will prioritize `data` or `errors` if both fields are present in the Remote Schema response.

| | |
| ------------------- | ---------------------------------------------- |
| **Flag** | `--remote-schema-prioritize-data` |
| **Env var** | `HASURA_GRAPHQL_REMOTE_SCHEMA_PRIORITIZE_DATA` |
| **Accepted values** | Boolean |
| **Options** | `true` or `false` |
| **Default** | `false` |
| **Supported in** | CE, Enterprise Edition, Cloud |

### Schema Sync Poll Interval

The interval, in milliseconds, to poll Metadata storage for updates. To disable, set this value to `0`.
Expand Down
3 changes: 2 additions & 1 deletion server/lib/test-harness/src/Harness/Constants.hs
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ serveOptions =
soTriggersErrorLogLevelStatus = Init._default Init.triggersErrorLogLevelStatusOption,
soAsyncActionsFetchBatchSize = Init._default Init.asyncActionsFetchBatchSizeOption,
soPersistedQueries = Init._default Init.persistedQueriesOption,
soPersistedQueriesTtl = Init._default Init.persistedQueriesTtlOption
soPersistedQueriesTtl = Init._default Init.persistedQueriesTtlOption,
soRemoteSchemaResponsePriority = Init._default Init.remoteSchemaResponsePriorityOption
}

-- | What log level should be used by the engine; this is not exported, and
Expand Down
6 changes: 4 additions & 2 deletions server/src-lib/Hasura/App/State.hs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ data AppContext = AppContext
acAsyncActionsFetchInterval :: OptionalInterval,
acApolloFederationStatus :: ApolloFederationStatus,
acCloseWebsocketsOnMetadataChangeStatus :: CloseWebsocketsOnMetadataChangeStatus,
acSchemaSampledFeatureFlags :: SchemaSampledFeatureFlags
acSchemaSampledFeatureFlags :: SchemaSampledFeatureFlags,
acRemoteSchemaResponsePriority :: RemoteSchemaResponsePriority
}

-- | Collection of the LoggerCtx, the regular Logger and the PGLogger
Expand Down Expand Up @@ -292,7 +293,8 @@ buildAppContextRule = proc (ServeOptions {..}, env, _keys, checkFeatureFlag) ->
acAsyncActionsFetchInterval = soAsyncActionsFetchInterval,
acApolloFederationStatus = soApolloFederationStatus,
acCloseWebsocketsOnMetadataChangeStatus = soCloseWebsocketsOnMetadataChangeStatus,
acSchemaSampledFeatureFlags = schemaSampledFeatureFlags
acSchemaSampledFeatureFlags = schemaSampledFeatureFlags,
acRemoteSchemaResponsePriority = soRemoteSchemaResponsePriority
}
where
buildEventEngineCtx = Inc.cache proc (httpPoolSize, fetchInterval, fetchBatchSize) -> do
Expand Down
81 changes: 64 additions & 17 deletions server/src-lib/Hasura/GraphQL/Transport/HTTP.hs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import Data.ByteString.Lazy qualified as LBS
import Data.Dependent.Map qualified as DM
import Data.Environment qualified as Env
import Data.HashMap.Strict.InsOrd qualified as InsOrdHashMap
import Data.List.NonEmpty qualified as NE
import Data.Monoid (Any (..))
import Data.Text qualified as T
import Data.Text.Extended (toTxt, (<>>))
import Data.Vector qualified as Vec
import Hasura.Backends.DataConnector.Agent.Client (AgentLicenseKey)
import Hasura.Backends.Postgres.Instances.Transport (runPGMutationTransaction)
import Hasura.Base.Error
Expand Down Expand Up @@ -89,7 +91,7 @@ import Hasura.Server.Prometheus
PrometheusMetrics (..),
)
import Hasura.Server.Telemetry.Counters qualified as Telem
import Hasura.Server.Types (ModelInfoLogState (..), MonadGetPolicies (..), ReadOnlyMode (..), RequestId (..))
import Hasura.Server.Types (ModelInfoLogState (..), MonadGetPolicies (..), ReadOnlyMode (..), RemoteSchemaResponsePriority (..), RequestId (..))
import Hasura.Services
import Hasura.Session (SessionVariable, SessionVariableValue, SessionVariables, UserInfo (..), filterSessionVariables)
import Hasura.Tracing (MonadTrace, attachMetadata)
Expand Down Expand Up @@ -309,6 +311,7 @@ runGQ ::
SchemaCache ->
Init.AllowListStatus ->
ReadOnlyMode ->
RemoteSchemaResponsePriority ->
PrometheusMetrics ->
L.Logger L.Hasura ->
Maybe (CredentialCache AgentLicenseKey) ->
Expand All @@ -320,7 +323,7 @@ runGQ ::
GQLReqUnparsed ->
ResponseInternalErrorsConfig ->
m (GQLQueryOperationSuccessLog, HttpResponse (Maybe GQResponse, EncJSON))
runGQ env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHeaders queryType reqUnparsed responseErrorsConfig = do
runGQ env sqlGenCtx sc enableAL readOnlyMode remoteSchemaResponsePriority prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHeaders queryType reqUnparsed responseErrorsConfig = do
getModelInfoLogStatus' <- runGetModelInfoLogStatus
modelInfoLogStatus <- liftIO getModelInfoLogStatus'
let gqlMetrics = pmGraphQLRequestMetrics prometheusMetrics
Expand Down Expand Up @@ -557,7 +560,7 @@ runGQ env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicen
runRemoteGQ fieldName rsi resultCustomizer gqlReq remoteJoins = Tracing.newSpan ("Remote schema query for root field " <>> fieldName) $ do
(telemTimeIO_DT, remoteResponseHeaders, resp) <-
doQErr $ E.execRemoteGQ env tracesPropagator userInfo reqHeaders (rsDef rsi) gqlReq
value <- extractFieldFromResponse fieldName resultCustomizer resp
value <- extractFieldFromResponse remoteSchemaResponsePriority fieldName resultCustomizer resp
(finalResponse, modelInfo) <-
doQErr
$ RJ.processRemoteJoins
Expand Down Expand Up @@ -663,34 +666,77 @@ coalescePostgresMutations plan = do
_ -> Nothing
Just (oneSourceConfig, oneResolvedConnectionTemplate, mutations)

data RemoteGraphQLResponse
= -- | "data" is omitted or `null` and "errors" is non-empty list
RGROnlyErrors (NonEmpty J.Value)
| -- | "data" is present and non-null, "errors" is omitted
RGROnlyData JO.Value
| -- | "data" is present and non-null, "errors" is non-empty list
RGRDataAndErrors JO.Value (NonEmpty J.Value)

data GraphQLResponse
= GraphQLResponseErrors [J.Value]
| GraphQLResponseData JO.Value

decodeGraphQLResponse :: LBS.ByteString -> Either Text GraphQLResponse
decodeGraphQLResponse bs = do
-- | This function decodes the response from a remote server:
--
-- 1. First, errors are fetched from the response. Absence of errors field and `errors: null` both implies that there
-- are no errors.
-- 2. Next, the data field is fetched from the response.
-- 3. If a non-null data field is present in the response and there are no errors, then the data field is returned.
-- 4. If a non-null data field is not present in the response and there are errors, then the errors are thrown.
-- 5. If the data field is not present and there are no errors, then an error is thrown.
-- 6. If both data and errors are present, then we need to decide which one to pick based on the priority.
decodeGraphQLResponse :: RemoteSchemaResponsePriority -> LBS.ByteString -> Either Text GraphQLResponse
decodeGraphQLResponse remoteSchemaResponsePriority bs = do
val <- mapLeft T.pack $ JO.eitherDecode bs
valObj <- JO.asObject val
case JO.lookup "errors" valObj of
Just (JO.Array errs) -> Right $ GraphQLResponseErrors (toList $ JO.fromOrdered <$> errs)
Just _ -> Left "Invalid \"errors\" field in response from remote"
Nothing -> do
dataVal <- JO.lookup "data" valObj `onNothing` Left "Missing \"data\" field in response from remote"
Right $ GraphQLResponseData dataVal
response <- buildRemoteGraphQLResponse val
case response of
RGROnlyErrors errs -> Right $ GraphQLResponseErrors $ toList errs
RGROnlyData d -> Right $ GraphQLResponseData d
RGRDataAndErrors d errs ->
-- Both data (non-null) and errors (non-empty) is present, we need to decide which one to pick based on the
-- priority
case remoteSchemaResponsePriority of
RemoteSchemaResponseData -> Right $ GraphQLResponseData d
RemoteSchemaResponseErrors -> Right $ GraphQLResponseErrors $ toList errs

buildRemoteGraphQLResponse :: JO.Value -> Either Text RemoteGraphQLResponse
buildRemoteGraphQLResponse response = do
responseObj <- JO.asObject response
errors <-
case JO.lookup "errors" responseObj of
-- Absence of errors field and errors: null both implies that there are no errors
Just (JO.Array errs) -> do
neErrors <- maybeToEither "Empty \"errors\" field in response from remote" $ NE.nonEmpty $ Vec.toList errs
pure $ Just neErrors
Just JO.Null -> pure Nothing
Nothing -> pure Nothing
Just _ -> Left "Invalid \"errors\" field in response from remote"
case (JO.lookup "data" responseObj, errors) of
-- According to spec, If the data entry in the response is not present, the errors entry in the response must not be
-- empty.
(Nothing, Nothing) -> Left "Missing \"data\" field with no errors in response from remote"
(Nothing, Just nonEmptyErrors) -> Right $ RGROnlyErrors $ JO.fromOrdered <$> nonEmptyErrors
(Just JO.Null, Nothing) -> Left "Received null \"data\" field with no errors in response from remote"
(Just JO.Null, Just nonEmptyErrors) -> Right $ RGROnlyErrors $ JO.fromOrdered <$> nonEmptyErrors
(Just dataVal, Nothing) -> Right $ RGROnlyData dataVal
(Just dataVal, Just nonEmptyErrors) -> Right $ RGRDataAndErrors dataVal $ JO.fromOrdered <$> nonEmptyErrors

extractFieldFromResponse ::
forall m.
(Monad m) =>
RemoteSchemaResponsePriority ->
RootFieldAlias ->
ResultCustomizer ->
LBS.ByteString ->
ExceptT (Either GQExecError QErr) m JO.Value
extractFieldFromResponse fieldName resultCustomizer resp = do
extractFieldFromResponse remoteSchemaResponsePriority fieldName resultCustomizer resp = do
let fieldName' = G.unName $ _rfaAlias fieldName
dataVal <-
applyResultCustomizer resultCustomizer
<$> do
graphQLResponse <- decodeGraphQLResponse resp `onLeft` do400
graphQLResponse <- decodeGraphQLResponse remoteSchemaResponsePriority resp `onLeft` do400
case graphQLResponse of
GraphQLResponseErrors errs -> doGQExecError errs
GraphQLResponseData d -> pure d
Expand Down Expand Up @@ -747,17 +793,18 @@ runGQBatched ::
Maybe (CredentialCache AgentLicenseKey) ->
RequestId ->
ResponseInternalErrorsConfig ->
RemoteSchemaResponsePriority ->
UserInfo ->
Wai.IpAddress ->
[HTTP.Header] ->
E.GraphQLQueryType ->
-- | the batched request with unparsed GraphQL query
GQLBatchedReqs (GQLReq GQLQueryText) ->
m (HttpLogGraphQLInfo, HttpResponse EncJSON)
runGQBatched env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId responseErrorsConfig userInfo ipAddress reqHdrs queryType query =
runGQBatched env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId responseErrorsConfig remoteSchemaResponsePriority userInfo ipAddress reqHdrs queryType query =
case query of
GQLSingleRequest req -> do
(gqlQueryOperationLog, httpResp) <- runGQ env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHdrs queryType req responseErrorsConfig
(gqlQueryOperationLog, httpResp) <- runGQ env sqlGenCtx sc enableAL readOnlyMode remoteSchemaResponsePriority prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHdrs queryType req responseErrorsConfig
let httpLoggingGQInfo = (CommonHttpLogMetadata L.RequestModeSingle (Just (GQLSingleRequest (GQLQueryOperationSuccess gqlQueryOperationLog))), (PQHSetSingleton (gqolParameterizedQueryHash gqlQueryOperationLog)))
pure (httpLoggingGQInfo, snd <$> httpResp)
GQLBatchedReqs reqs -> do
Expand All @@ -770,7 +817,7 @@ runGQBatched env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger age
flip HttpResponse []
. encJFromList
. map (either (encJFromJEncoding . encodeGQErr includeInternal) _hrBody)
responses <- for reqs \req -> fmap (req,) $ try $ (fmap . fmap . fmap) snd $ runGQ env sqlGenCtx sc enableAL readOnlyMode prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHdrs queryType req responseErrorsConfig
responses <- for reqs \req -> fmap (req,) $ try $ (fmap . fmap . fmap) snd $ runGQ env sqlGenCtx sc enableAL readOnlyMode remoteSchemaResponsePriority prometheusMetrics logger agentLicenseKey reqId userInfo ipAddress reqHdrs queryType req responseErrorsConfig
let requestsOperationLogs = map fst $ rights $ map snd responses
batchOperationLogs =
map
Expand Down
12 changes: 7 additions & 5 deletions server/src-lib/Hasura/GraphQL/Transport/WebSocket.hs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ import Hasura.Server.Prometheus
PrometheusMetrics (..),
)
import Hasura.Server.Telemetry.Counters qualified as Telem
import Hasura.Server.Types (GranularPrometheusMetricsState (..), ModelInfoLogState (..), MonadGetPolicies (..), RequestId, getRequestId)
import Hasura.Server.Types (GranularPrometheusMetricsState (..), ModelInfoLogState (..), MonadGetPolicies (..), RemoteSchemaResponsePriority, RequestId, getRequestId)
import Hasura.Services.Network
import Hasura.Session
import Hasura.Tracing qualified as Tracing
Expand Down Expand Up @@ -483,6 +483,7 @@ onStart enabledLogTypes agentLicenseKey serverEnv wsConn shouldCaptureVariables
env <- liftIO $ acEnvironment <$> getAppContext appStateRef
sqlGenCtx <- liftIO $ acSQLGenCtx <$> getAppContext appStateRef
enableAL <- liftIO $ acEnableAllowlist <$> getAppContext appStateRef
remoteSchemaResponsePriority <- liftIO $ acRemoteSchemaResponsePriority <$> getAppContext appStateRef

(reqParsed, queryParts) <- Tracing.newSpan "Parse GraphQL" $ do
reqParsedE <- lift $ E.checkGQLExecution userInfo (reqHdrs, ipAddress) enableAL sc q requestId
Expand Down Expand Up @@ -562,7 +563,7 @@ onStart enabledLogTypes agentLicenseKey serverEnv wsConn shouldCaptureVariables
pure $ (AnnotatedResponsePart telemTimeIO_DT Telem.Local finalResponse [], modelInfo)
E.ExecStepRemote rsi resultCustomizer gqlReq remoteJoins -> do
logQueryLog logger $ QueryLog q Nothing requestId QueryLogKindRemoteSchema
runRemoteGQ requestId q fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator
runRemoteGQ requestId q fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator remoteSchemaResponsePriority
E.ExecStepAction actionExecPlan _ remoteJoins -> do
logQueryLog logger $ QueryLog q Nothing requestId QueryLogKindAction
(time, (resp, _), modelInfo) <- doQErr $ do
Expand Down Expand Up @@ -654,7 +655,7 @@ onStart enabledLogTypes agentLicenseKey serverEnv wsConn shouldCaptureVariables
pure $ (AnnotatedResponsePart time Telem.Empty resp $ fromMaybe [] hdrs, modelInfo)
E.ExecStepRemote rsi resultCustomizer gqlReq remoteJoins -> do
logQueryLog logger $ QueryLog q Nothing requestId QueryLogKindRemoteSchema
runRemoteGQ requestId q fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator
runRemoteGQ requestId q fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator remoteSchemaResponsePriority
E.ExecStepRaw json -> do
logQueryLog logger $ QueryLog q Nothing requestId QueryLogKindIntrospection
(,[]) <$> buildRaw json
Expand Down Expand Up @@ -824,13 +825,14 @@ onStart enabledLogTypes agentLicenseKey serverEnv wsConn shouldCaptureVariables
GQLReqOutgoing ->
Maybe RJ.RemoteJoins ->
Tracing.HttpPropagator ->
RemoteSchemaResponsePriority ->
ExceptT (Either GQExecError QErr) (ExceptT () m) (AnnotatedResponsePart, [ModelInfoPart])
runRemoteGQ requestId reqUnparsed fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator = Tracing.newSpan ("Remote schema query for root field " <>> fieldName) $ do
runRemoteGQ requestId reqUnparsed fieldName userInfo reqHdrs rsi resultCustomizer gqlReq remoteJoins tracesPropagator remoteSchemaResponsePriority = Tracing.newSpan ("Remote schema query for root field " <>> fieldName) $ do
env <- liftIO $ acEnvironment <$> getAppContext appStateRef
(telemTimeIO_DT, _respHdrs, resp) <-
doQErr
$ E.execRemoteGQ env tracesPropagator userInfo reqHdrs (rsDef rsi) gqlReq
value <- hoist lift $ extractFieldFromResponse fieldName resultCustomizer resp
value <- hoist lift $ extractFieldFromResponse remoteSchemaResponsePriority fieldName resultCustomizer resp
(finalResponse, modelInfo) <-
doQErr
$ RJ.processRemoteJoins
Expand Down
4 changes: 2 additions & 2 deletions server/src-lib/Hasura/Server/App.hs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ v1Alpha1GQHandler queryType query = do
reqHeaders <- asks hcReqHeaders
ipAddress <- asks hcSourceIpAddress
requestId <- asks hcRequestId
GH.runGQBatched acEnvironment acSQLGenCtx schemaCache acEnableAllowlist appEnvEnableReadOnlyMode appEnvPrometheusMetrics (_lsLogger appEnvLoggers) appEnvLicenseKeyCache requestId acResponseInternalErrorsConfig userInfo ipAddress reqHeaders queryType query
GH.runGQBatched acEnvironment acSQLGenCtx schemaCache acEnableAllowlist appEnvEnableReadOnlyMode appEnvPrometheusMetrics (_lsLogger appEnvLoggers) appEnvLicenseKeyCache requestId acResponseInternalErrorsConfig acRemoteSchemaResponsePriority userInfo ipAddress reqHeaders queryType query

v1GQHandler ::
( MonadIO m,
Expand Down Expand Up @@ -954,7 +954,7 @@ httpApp setupHook appStateRef AppEnv {..} consoleType ekgStore closeWebsocketsOn
Spock.PATCH -> pure EP.PATCH
other -> throw400 BadRequest $ "Method " <> tshow other <> " not supported."
_ -> throw400 BadRequest $ "Nonstandard method not allowed for REST endpoints"
fmap JSONResp <$> runCustomEndpoint acEnvironment acSQLGenCtx schemaCache acEnableAllowlist appEnvEnableReadOnlyMode appEnvPrometheusMetrics (_lsLogger appEnvLoggers) appEnvLicenseKeyCache requestId userInfo reqHeaders ipAddress req endpoints responseErrorsConfig
fmap JSONResp <$> runCustomEndpoint acEnvironment acSQLGenCtx schemaCache acEnableAllowlist appEnvEnableReadOnlyMode acRemoteSchemaResponsePriority appEnvPrometheusMetrics (_lsLogger appEnvLoggers) appEnvLicenseKeyCache requestId userInfo reqHeaders ipAddress req endpoints responseErrorsConfig

-- See Issue #291 for discussion around restified feature
Spock.hookRouteAll ("api" <//> "rest" <//> Spock.wildcard) $ \wildcard -> do
Expand Down
1 change: 1 addition & 0 deletions server/src-lib/Hasura/Server/Init.hs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ mkServeOptions sor@ServeOptionsRaw {..} = do
soAsyncActionsFetchBatchSize <- withOptionDefault rsoAsyncActionsFetchBatchSize asyncActionsFetchBatchSizeOption
soPersistedQueries <- withOptionDefault rsoPersistedQueries persistedQueriesOption
soPersistedQueriesTtl <- withOptionDefault rsoPersistedQueriesTtl persistedQueriesTtlOption
soRemoteSchemaResponsePriority <- withOptionDefault rsoRemoteSchemaResponsePriority remoteSchemaResponsePriorityOption
pure ServeOptions {..}

-- | Fetch Postgres 'Query.ConnParams' components from the environment
Expand Down
Loading

0 comments on commit 7f1f060

Please sign in to comment.