diff --git a/.changeset/ninety-animals-ring.md b/.changeset/ninety-animals-ring.md new file mode 100644 index 0000000000..d5a77e7633 --- /dev/null +++ b/.changeset/ninety-animals-ring.md @@ -0,0 +1,10 @@ +--- +"postgraphile": patch +"ruru-components": patch +"ruru": patch +"grafserv": patch +"grafast": patch +--- + +Add support for `onError` RFC with `PROPAGATE`, `NULL` and `HALT` behaviors +implemented. diff --git a/grafast/grafast/__tests__/onError-test.ts b/grafast/grafast/__tests__/onError-test.ts new file mode 100644 index 0000000000..9db423181e --- /dev/null +++ b/grafast/grafast/__tests__/onError-test.ts @@ -0,0 +1,561 @@ +/* eslint-disable graphile-export/exhaustive-deps, graphile-export/export-methods, graphile-export/export-instances, graphile-export/export-subclasses, graphile-export/no-nested */ +import { expect } from "chai"; +import { resolvePreset } from "graphile-config"; +import { type ExecutionResult } from "graphql"; +import { it } from "mocha"; + +import { + constant, + context, + get, + grafast, + GraphQLSpecifiedErrorBehaviors, + lambda, + list, + makeGrafastSchema, + Step, +} from "../dist/index.js"; + +const resolvedPreset = resolvePreset({}); +const requestContext = {}; + +declare global { + namespace Grafast { + interface Context { + me?: { + id: number; + name: string; + nonNullable: unknown; + nullable: unknown; + nonNullableList: unknown; + nullableList: unknown; + }; + } + } +} + +const schema = makeGrafastSchema({ + typeDefs: /* GraphQL */ ` + type Query { + me: User + } + type User { + id: Int! + name: String! + nonNullable: Int! + nullable: Int + nonNullableList: [Int!]! + nullableList: [Int] + throwNonNullable: Int! + throwNullable: Int + friends: [User!] + } + `, + objects: { + Query: { + plans: { + me() { + return get(context(), "me"); + }, + }, + }, + User: { + plans: { + friends($user) { + return list([ + constant({ + id: 99, + name: "Test", + nonNullable: 99, + nonNullableList: [], + }), + $user, + ]); + }, + throwNonNullable($user: Step) { + return lambda($user, () => { + throw new Error("Threw in non-nullable field"); + }); + }, + throwNullable($user: Step) { + return lambda($user, () => { + throw new Error("Threw in nullable field"); + }); + }, + }, + }, + }, + enableDeferStream: true, +}); + +function throwOnUnhandledRejections(callback: () => Promise) { + return async () => { + let failed: Error | undefined; + function fail(e: Error) { + console.error(`UNHANDLED PROMISE REJECTION: ${e}`); + failed = e; + } + process.on("unhandledRejection", fail); + try { + return await callback(); + } finally { + process.off("unhandledRejection", fail); + if (failed) { + failed = undefined; + // eslint-disable-next-line no-unsafe-finally + throw new Error(`Unhandled promise rejection occurred`); + } + } + }; +} + +GraphQLSpecifiedErrorBehaviors.forEach((onError) => { + describe(`onError=${onError}`, () => { + it( + "handles null in non-nullable position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + nonNullable + nullable + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + nonNullable: null, + nullable: null, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: + "Cannot return null for non-nullable field User.nonNullable.", + path: ["me", "nonNullable"], + }); + + switch (onError) { + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: null, + }); + break; + } + case "NULL": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + nonNullable: null, + nullable: null, + name: "Benjie", + }, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + + it( + "handles null in nullable position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + nonNullable + nullable + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + nonNullable: 27, + nullable: null, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + expect(result.errors).to.be.undefined; + expect(result.data).to.deep.equal({ + me: { + id: 42, + nonNullable: 27, + nullable: null, + name: "Benjie", + }, + }); + }), + ); + + it( + "handles coercion failure in non-nullable position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + nonNullable + nullable + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + nonNullable: "NOT A NUMBER", + nullable: null, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: `Int cannot represent non-integer value: "NOT A NUMBER"`, + path: ["me", "nonNullable"], + }); + + switch (onError) { + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: null, + }); + break; + } + case "NULL": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + nonNullable: null, + nullable: null, + name: "Benjie", + }, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + + it( + "handles thrown error in nullable position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + throwNullable + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: `Threw in nullable field`, + path: ["me", "throwNullable"], + }); + + switch (onError) { + case "NULL": + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + throwNullable: null, + name: "Benjie", + }, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + + it( + "handles thrown error in non-nullable position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + throwNonNullable + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: `Threw in non-nullable field`, + path: ["me", "throwNonNullable"], + }); + + switch (onError) { + case "NULL": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + throwNonNullable: null, + name: "Benjie", + }, + }); + break; + } + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: null, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + + it( + "handles null in non-nullable list position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + nonNullableList + nullableList + name + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + nonNullableList: [1, 1, 2, null, 5], + nullableList: null, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: + "Cannot return null for non-nullable field User.nonNullableList.", + path: ["me", "nonNullableList", 3], + }); + + switch (onError) { + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: null, + }); + break; + } + case "NULL": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + nonNullableList: [1, 1, 2, null, 5], + nullableList: null, + name: "Benjie", + }, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + + it( + "handles null in non-nullable position inside a non-nullable list position", + throwOnUnhandledRejections(async () => { + const source = /* GraphQL */ ` + { + me { + id + name + friends { + id + name + nonNullableList + nullableList + } + } + } + `; + const result = (await grafast({ + schema, + source, + onError, + contextValue: { + me: { + id: 42, + nonNullableList: [1, 1, 2, null, 5], + nullableList: null, + name: "Benjie", + }, + }, + requestContext, + resolvedPreset, + })) as ExecutionResult; + + // All error modes expect the same errors: + expect(result.errors).to.have.length(1); + expect(result.errors![0]).to.deep.include({ + message: + "Cannot return null for non-nullable field User.nonNullableList.", + path: ["me", "friends", 1, "nonNullableList", 3], + }); + + switch (onError) { + case "PROPAGATE": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + name: "Benjie", + friends: null, + }, + }); + break; + } + case "NULL": { + expect(result.data).to.deep.equal({ + me: { + id: 42, + name: "Benjie", + friends: [ + { + id: 99, + name: "Test", + nonNullableList: [], + nullableList: null, + }, + { + id: 42, + name: "Benjie", + nonNullableList: [1, 1, 2, null, 5], + nullableList: null, + }, + ], + }, + }); + break; + } + case "HALT": { + expect(result.data).to.equal(null); + break; + } + default: { + const never: never = onError; + throw new Error(`Unexpected onError: ${never}`); + } + } + }), + ); + }); +}); diff --git a/grafast/grafast/src/bucket.ts b/grafast/grafast/src/bucket.ts index 05ce30238f..dc7fd267e4 100644 --- a/grafast/grafast/src/bucket.ts +++ b/grafast/grafast/src/bucket.ts @@ -1,6 +1,6 @@ // import type { GraphQLScalarType } from "graphql"; -import type { GrafastExecutionArgs, Step } from "."; +import type { ErrorBehavior, GrafastExecutionArgs, Step } from "."; import type { LayerPlan } from "./engine/LayerPlan"; import type { MetaByMetaKey } from "./engine/OperationPlan"; import type { @@ -15,6 +15,8 @@ import type { export interface RequestTools { /** @internal */ args: GrafastExecutionArgs; + /** @internal */ + onError: ErrorBehavior; /** The `timeSource.now()` at which the request started executing */ startTime: number; /** The `timeSource.now()` at which the request should stop executing (if a timeout was configured) */ diff --git a/grafast/grafast/src/engine/OperationPlan.ts b/grafast/grafast/src/engine/OperationPlan.ts index 400d58be9f..64d026aaf5 100644 --- a/grafast/grafast/src/engine/OperationPlan.ts +++ b/grafast/grafast/src/engine/OperationPlan.ts @@ -31,7 +31,11 @@ import { newSelectionSetDigest, } from "../graphqlCollectFields.js"; import { fieldSelectionsForType } from "../graphqlMergeSelectionSets.js"; -import type { GrafastPlanJSON, StepStreamOptions } from "../index.js"; +import type { + ErrorBehavior, + GrafastPlanJSON, + StepStreamOptions, +} from "../index.js"; import { __FlagStep, __ItemStep, @@ -371,6 +375,7 @@ export class OperationPlan { public readonly context: { [key: string]: any }, rootValueConstraints: Constraint[], public readonly rootValue: any, + public readonly errorBehavior: ErrorBehavior, options: GrafastOperationOptions, ) { this.planningTimeout = options?.timeouts?.planning ?? null; diff --git a/grafast/grafast/src/engine/OutputPlan.ts b/grafast/grafast/src/engine/OutputPlan.ts index f8ec3ca644..9cea0fc751 100644 --- a/grafast/grafast/src/engine/OutputPlan.ts +++ b/grafast/grafast/src/engine/OutputPlan.ts @@ -972,137 +972,88 @@ function executeChildPlan( rawBucketRootValue: any, bucketRootFlags: ExecutionEntryFlags, ) { - const $sideEffect = childOutputPlan.layerPlan.parentSideEffectStep; - if ($sideEffect !== null) { - // Check if there's an error - const sideEffectBucketDetails = getChildBucketAndIndex( - $sideEffect.layerPlan, - null, - bucket, - bucketIndex, - ); - if (sideEffectBucketDetails) { - const [sideEffectBucket, sideEffectBucketIndex] = sideEffectBucketDetails; - const ev = sideEffectBucket.store.get($sideEffect.id); - if (!ev) { - throw new Error( - `GrafastInternalError: ${stripAnsi( - String($sideEffect), - )} has no entry in ${bucket}`, - ); - } - const flags = ev._flagsAt(sideEffectBucketIndex); - if (flags & FLAG_ERROR) { - const e = coerceError( - ev.at(sideEffectBucketIndex), - locationDetails, - mutablePath.slice(1), - ); - if (isNonNull) { - throw e; - } else { - const streamCount = root.streams.length; - const queueCount = root.queue.length; - commonErrorHandler( - e, + const streamCount = root.streams.length; + const queueCount = root.queue.length; + try { + const $sideEffect = childOutputPlan.layerPlan.parentSideEffectStep; + if ($sideEffect !== null) { + // Check if there's an error + const sideEffectBucketDetails = getChildBucketAndIndex( + $sideEffect.layerPlan, + null, + bucket, + bucketIndex, + ); + if (sideEffectBucketDetails) { + const [sideEffectBucket, sideEffectBucketIndex] = + sideEffectBucketDetails; + const ev = sideEffectBucket.store.get($sideEffect.id); + if (!ev) { + throw new Error( + `GrafastInternalError: ${stripAnsi( + String($sideEffect), + )} has no entry in ${bucket}`, + ); + } + const flags = ev._flagsAt(sideEffectBucketIndex); + if (flags & FLAG_ERROR) { + throw coerceError( + ev.at(sideEffectBucketIndex), locationDetails, - mutablePath, - mutablePathIndex, - root, - streamCount, - queueCount, + mutablePath.slice(1), ); - return asString ? "null" : null; } } } - } - // This is the code that changes based on if the field is nullable or not - if (isNonNull) { - // No need to catch error - if (childBucket === bucket) { - //noop + if (childBucket !== bucket && childBucket == null) { + if (isNonNull) { + throw nonNullError(locationDetails, mutablePath.slice(1)); + } + return asString ? "null" : null; } else { - if (childBucket == null) { + const fieldResult = childOutputPlan[ + asString ? "executeString" : "execute" + ]( + root, + mutablePath, + childBucket, + childBucketIndex!, + // NOTE: the previous code may have had a bug here, it referenced childBucket.rootStep + childOutputPlan.rootStep === that.rootStep + ? rawBucketRootValue + : undefined, + childOutputPlan.rootStep === that.rootStep + ? bucketRootFlags + : undefined, + ); + if (isNonNull && fieldResult == (asString ? "null" : null)) { throw nonNullError(locationDetails, mutablePath.slice(1)); } + return fieldResult; } - const fieldResult = childOutputPlan[asString ? "executeString" : "execute"]( - root, - mutablePath, - childBucket, - childBucketIndex!, - // NOTE: the previous code may have had a bug here, it referenced childBucket.rootStep - childOutputPlan.rootStep === that.rootStep - ? rawBucketRootValue - : undefined, - childOutputPlan.rootStep === that.rootStep ? bucketRootFlags : undefined, - ); - if (fieldResult == (asString ? "null" : null)) { - throw nonNullError(locationDetails, mutablePath.slice(1)); + } catch (e) { + const error = coerceError(e, locationDetails, mutablePath.slice(1)); + if (root.errorBehavior === "HALT") throw error; + if (isNonNull && root.errorBehavior !== "NULL") throw error; + if (root.streams.length > streamCount) { + root.streams.splice(streamCount); } - return fieldResult; - } else { - // Need to catch error and set null - const streamCount = root.streams.length; - const queueCount = root.queue.length; - try { - if (childBucket !== bucket && childBucket == null) { - return asString ? "null" : null; - } else { - return childOutputPlan[asString ? "executeString" : "execute"]( - root, - mutablePath, - childBucket, - childBucketIndex!, - // NOTE: the previous code may have had a bug here, it referenced childBucket.rootStep - childOutputPlan.rootStep === that.rootStep - ? rawBucketRootValue - : undefined, - childOutputPlan.rootStep === that.rootStep - ? bucketRootFlags - : undefined, - ); - } - } catch (e) { - commonErrorHandler( - e, - locationDetails, - mutablePath, - mutablePathIndex, - root, - streamCount, - queueCount, + if (root.queue.length > queueCount) { + root.queue.splice(queueCount); + } + const pathLengthTarget = mutablePathIndex + 1; + const overSize = mutablePath.length - pathLengthTarget; + if (overSize > 0) { + (mutablePath as Array).splice( + pathLengthTarget, + overSize, ); - return asString ? "null" : null; } + root.errors.push(error); + return asString ? "null" : null; } } -function commonErrorHandler( - e: Error, - locationDetails: LocationDetails, - mutablePath: ReadonlyArray, - mutablePathIndex: number, - root: PayloadRoot, - streamCount: number, - queueCount: number, -) { - if (root.streams.length > streamCount) { - root.streams.splice(streamCount); - } - if (root.queue.length > queueCount) { - root.queue.splice(queueCount); - } - const error = coerceError(e, locationDetails, mutablePath.slice(1)); - const pathLengthTarget = mutablePathIndex + 1; - const overSize = mutablePath.length - pathLengthTarget; - if (overSize > 0) { - (mutablePath as Array).splice(pathLengthTarget, overSize); - } - root.errors.push(error); -} - const nullExecutor = makeExecutor({ inner: () => null, nameExtra: "null", diff --git a/grafast/grafast/src/engine/executeOutputPlan.ts b/grafast/grafast/src/engine/executeOutputPlan.ts index 5125907888..b64521ab56 100644 --- a/grafast/grafast/src/engine/executeOutputPlan.ts +++ b/grafast/grafast/src/engine/executeOutputPlan.ts @@ -4,7 +4,7 @@ import type { GraphQLError } from "graphql"; import * as assert from "../assert.js"; import type { Bucket, RequestTools } from "../bucket.js"; import { isDev } from "../dev.js"; -import type { JSONValue } from "../interfaces.js"; +import type { ErrorBehavior, JSONValue } from "../interfaces.js"; import type { OutputPlan } from "./OutputPlan.js"; const debug = debugFactory("grafast:OutputPlan"); @@ -30,6 +30,8 @@ export interface OutputStream { * @internal */ export interface PayloadRoot { + errorBehavior: ErrorBehavior; + /** * Serialization works differently if we're running inside GraphQL. (Namely: * we don't serialize - that's GraphQL's job.) diff --git a/grafast/grafast/src/establishOperationPlan.ts b/grafast/grafast/src/establishOperationPlan.ts index 550e0c0d81..b34e83cd44 100644 --- a/grafast/grafast/src/establishOperationPlan.ts +++ b/grafast/grafast/src/establishOperationPlan.ts @@ -10,6 +10,7 @@ import { OperationPlan, SafeError } from "./index.js"; import type { BaseGraphQLRootValue, BaseGraphQLVariables, + ErrorBehavior, EstablishOperationPlanResult, Fragments, LinkedList, @@ -75,16 +76,18 @@ function isOperationPlanResultCompatible< TContext extends Grafast.Context = Grafast.Context, TRootValue extends BaseGraphQLRootValue = BaseGraphQLRootValue, >( - operationPlan: EstablishOperationPlanResult, + operationPlanResult: EstablishOperationPlanResult, variableValues: TVariables, context: TContext, rootValue: TRootValue, + errorBehavior: ErrorBehavior, ): boolean { + if (operationPlanResult.errorBehavior !== errorBehavior) return false; const { variableValuesConstraints, contextConstraints, rootValueConstraints, - } = operationPlan; + } = operationPlanResult; if (!matchesConstraints(variableValuesConstraints, variableValues)) { return false; } @@ -115,6 +118,7 @@ export function establishOperationPlan< variableValues: TVariables, context: TContext, rootValue: TRootValue, + onError: ErrorBehavior, options: GrafastOperationOptions, ): OperationPlan { const planningTimeout = options.timeouts?.planning; @@ -141,6 +145,7 @@ export function establishOperationPlan< variableValues, context, rootValue, + onError, ) ) { const { error, operationPlan } = value; @@ -214,6 +219,7 @@ export function establishOperationPlan< context, rootValueConstraints, rootValue, + onError, options, ); } catch (e) { @@ -234,6 +240,7 @@ export function establishOperationPlan< variableValuesConstraints, contextConstraints, rootValueConstraints, + errorBehavior: onError, ...(operationPlan ? { operationPlan } : { error: error! }), }; if (!cache) { diff --git a/grafast/grafast/src/grafastGraphql.ts b/grafast/grafast/src/grafastGraphql.ts index 34552e72d5..0d73c2fbc3 100644 --- a/grafast/grafast/src/grafastGraphql.ts +++ b/grafast/grafast/src/grafastGraphql.ts @@ -160,6 +160,8 @@ export function grafast( contextValue, variableValues, operationName, + onError, + extensions, fieldResolver, typeResolver, resolvedPreset, @@ -207,6 +209,8 @@ export function grafast( contextValue, variableValues, operationName, + onError, + extensions, fieldResolver, typeResolver, middleware, diff --git a/grafast/grafast/src/index.ts b/grafast/grafast/src/index.ts index 09fce5087c..9a0a62ca81 100644 --- a/grafast/grafast/src/index.ts +++ b/grafast/grafast/src/index.ts @@ -77,6 +77,7 @@ import type { CacheByOperationEntry, DataFromStep, EnumValueApplyResolver, + ErrorBehavior, EstablishOperationPlanEvent, EventCallback, EventMapKey, @@ -267,6 +268,7 @@ import { GrafastInputFieldConfigMap, GrafastInputObjectType, GrafastObjectType, + GraphQLSpecifiedErrorBehaviors, inputObjectFieldSpec, InputObjectTypeSpec, isPromiseLike, @@ -359,6 +361,7 @@ export { EnumValueConfig, EnumValueInput, error, + ErrorBehavior, ErrorStep, EventCallback, EventMapKey, @@ -410,6 +413,7 @@ export { GrafastValuesList, graphqlResolver, GraphQLResolverStep, + GraphQLSpecifiedErrorBehaviors, groupBy, GroupByPlanMemo, inhibitOnNull, @@ -704,6 +708,8 @@ declare global { | "rootValue" | "variableValues" | "operationName" + | "onError" + | "extensions" | "resolvedPreset" | "middleware" | "requestContext" diff --git a/grafast/grafast/src/interfaces.ts b/grafast/grafast/src/interfaces.ts index 20a293dd32..6aea9223ec 100644 --- a/grafast/grafast/src/interfaces.ts +++ b/grafast/grafast/src/interfaces.ts @@ -88,6 +88,7 @@ export interface IEstablishOperationPlanResult { variableValuesConstraints: Constraint[]; contextConstraints: Constraint[]; rootValueConstraints: Constraint[]; + errorBehavior: ErrorBehavior; } export interface EstablishOperationPlanResultSuccess extends IEstablishOperationPlanResult { @@ -735,6 +736,11 @@ export type StreamMoreableArray = Array & { }; export interface GrafastArgs extends GraphQLArgs { + // This should ultimately come from graphql + onError?: ErrorBehavior; + + // These are ours + extensions?: Record; resolvedPreset?: GraphileConfig.ResolvedPreset; requestContext?: Partial; middleware?: Middleware | null; @@ -774,6 +780,11 @@ export type DataFromStep = TStep extends Step ? TData : never; export interface GrafastExecutionArgs extends ExecutionArgs { + // This should ultimately come from graphql + onError?: ErrorBehavior; + + // These are ours + extensions?: Record; resolvedPreset?: GraphileConfig.ResolvedPreset; middleware?: Middleware | null; requestContext?: Partial; @@ -805,6 +816,7 @@ export interface EstablishOperationPlanEvent { variableValues: Record; context: any; rootValue: any; + onError: ErrorBehavior; args: GrafastExecutionArgs; options: GrafastOperationOptions; } @@ -847,3 +859,8 @@ export interface AbstractTypePlanner { } export type Thunk = T | (() => T); + +/** + * GraphQL error behavior, as per https://github.com/graphql/graphql-spec/pull/1163 + */ +export type ErrorBehavior = "PROPAGATE" | "NULL" | "HALT"; diff --git a/grafast/grafast/src/prepare.ts b/grafast/grafast/src/prepare.ts index 8bdb9424a6..2bdd61b79a 100644 --- a/grafast/grafast/src/prepare.ts +++ b/grafast/grafast/src/prepare.ts @@ -42,6 +42,7 @@ import { } from "./engine/OutputPlan.js"; import { establishOperationPlan } from "./establishOperationPlan.js"; import type { + ErrorBehavior, EstablishOperationPlanEvent, GrafastExecutionArgs, GrafastTimeouts, @@ -265,6 +266,7 @@ function outputBucket( >*/ { const operationPlan = rootBucket.layerPlan.operationPlan; const root: PayloadRoot = { + errorBehavior: requestContext.args.onError ?? "PROPAGATE", insideGraphQL: false, errors: [], queue: [], @@ -324,6 +326,7 @@ function executePreemptive( variableValues: any, context: any, rootValue: any, + onError: ErrorBehavior, outputDataAsString: boolean, executionTimeout: number | null, abortSignal: AbortSignal, @@ -358,6 +361,7 @@ function executePreemptive( executionTimeout !== null ? startTime + executionTimeout : null; const requestContext: RequestTools = { args, + onError, startTime, stopTime, // toSerialize: [], @@ -567,6 +571,7 @@ function establishOperationPlanFromEvent(event: EstablishOperationPlanEvent) { event.variableValues, event.context as any, event.rootValue, + event.onError, event.options, ); } @@ -599,6 +604,9 @@ export function grafastPrepare( } const { operation, fragments, variableValues } = exeContext; + // TODO: update this when GraphQL.js gets support for onError + const onError = args.onError ?? "PROPAGATE"; + let operationPlan!: OperationPlan; try { if (middleware != null) { @@ -611,6 +619,7 @@ export function grafastPrepare( variableValues, context: context as any, rootValue, + onError, args, options, }, @@ -624,6 +633,7 @@ export function grafastPrepare( variableValues, context as any, rootValue, + onError, options, ); } @@ -667,6 +677,7 @@ export function grafastPrepare( variableValues, context, rootValue, + onError, options.outputDataAsString ?? false, executionTimeout, abortController.signal, diff --git a/grafast/grafast/src/utils.ts b/grafast/grafast/src/utils.ts index a472a6b829..b08f2ed962 100644 --- a/grafast/grafast/src/utils.ts +++ b/grafast/grafast/src/utils.ts @@ -1512,3 +1512,9 @@ export function terminateIterable( iterable.return(); } } + +export const GraphQLSpecifiedErrorBehaviors = Object.freeze([ + "PROPAGATE", + "NULL", + "HALT", +] as const); diff --git a/grafast/grafserv/src/interfaces.ts b/grafast/grafserv/src/interfaces.ts index 8237bd522f..bf77e388c7 100644 --- a/grafast/grafserv/src/interfaces.ts +++ b/grafast/grafserv/src/interfaces.ts @@ -1,6 +1,12 @@ import "graphile-config"; -import type { execute, PromiseOrDirect, SafeError, subscribe } from "grafast"; +import type { + ErrorBehavior, + execute, + PromiseOrDirect, + SafeError, + subscribe, +} from "grafast"; import type { AsyncExecutionResult, FormattedExecutionResult, @@ -41,6 +47,12 @@ export interface ParsedGraphQLBody { query: unknown; operationName: unknown; variableValues: unknown; + /** + * For customizing the error behavior as per https://github.com/graphql/graphql-spec/pull/1163 + * + * @experimental + */ + onError: unknown; extensions: unknown; } @@ -52,6 +64,7 @@ export interface ValidatedGraphQLBody { query: string; operationName: string | undefined; variableValues: Record | undefined; + onError: ErrorBehavior | undefined; extensions: Record | undefined; } diff --git a/grafast/grafserv/src/middleware/graphql.ts b/grafast/grafserv/src/middleware/graphql.ts index bf149d4098..9207361aaf 100644 --- a/grafast/grafserv/src/middleware/graphql.ts +++ b/grafast/grafserv/src/middleware/graphql.ts @@ -2,8 +2,18 @@ import { parse as parseGraphQLQueryString } from "node:querystring"; import { LRU } from "@graphile/lru"; import { createHash } from "crypto"; -import type { GrafastExecutionArgs, PromiseOrDirect } from "grafast"; -import { $$extensions, hookArgs, isAsyncIterable, SafeError } from "grafast"; +import type { + ErrorBehavior, + GrafastExecutionArgs, + PromiseOrDirect, +} from "grafast"; +import { + $$extensions, + GraphQLSpecifiedErrorBehaviors, + hookArgs, + isAsyncIterable, + SafeError, +} from "grafast"; import type { DocumentNode, GraphQLSchema } from "grafast/graphql"; import * as graphql from "grafast/graphql"; @@ -98,6 +108,7 @@ function parseGraphQLQueryParams( typeof variablesString === "string" ? JSON.parse(variablesString) : undefined; + const onError = params.onError ?? undefined; const extensionsString = params.extensions ?? undefined; const extensions = typeof extensionsString === "string" @@ -109,6 +120,7 @@ function parseGraphQLQueryParams( query, operationName, variableValues, + onError, extensions, }; } @@ -207,6 +219,7 @@ function parseGraphQLBody( query: body.text, operationName: undefined, variableValues: undefined, + onError: undefined, extensions: undefined, }; } @@ -217,6 +230,7 @@ function parseGraphQLBody( query: body.buffer.toString("utf8"), operationName: undefined, variableValues: undefined, + onError: undefined, extensions: undefined, }; } @@ -260,7 +274,7 @@ const graphqlOrHTMLAcceptMatcher = makeAcceptMatcher([ export function validateGraphQLBody( parsed: ParsedGraphQLBody, ): ValidatedGraphQLBody { - const { query, operationName, variableValues, extensions } = parsed; + const { query, operationName, variableValues, onError, extensions } = parsed; if (typeof query !== "string") { throw httpError(400, "query must be a string"); @@ -274,6 +288,15 @@ export function validateGraphQLBody( ) { throw httpError(400, "Invalid variables; expected JSON-encoded object"); } + if ( + onError != null && + !GraphQLSpecifiedErrorBehaviors.includes(onError as ErrorBehavior) + ) { + throw httpError( + 400, + `Invalid onError; supported error behaviors are: ${GraphQLSpecifiedErrorBehaviors.join(", ")}`, + ); + } if ( extensions != null && (typeof extensions !== "object" || Array.isArray(extensions)) @@ -414,7 +437,7 @@ const _makeGraphQLHandlerInternal = (instance: GrafservBase) => { const outputDataAsString = dynamicOptions.outputDataAsString; const { maskIterator, maskPayload, maskError } = dynamicOptions; - const { query, operationName, variableValues } = body; + const { query, operationName, variableValues, onError } = body; const { errors, document } = parseAndValidate(query); if (errors !== undefined) { @@ -458,6 +481,7 @@ const _makeGraphQLHandlerInternal = (instance: GrafservBase) => { contextValue, variableValues, operationName, + onError, resolvedPreset, requestContext: grafastCtx, middleware: grafastMiddleware, diff --git a/grafast/grafserv/src/utils.ts b/grafast/grafserv/src/utils.ts index 472a2c056b..653ef3fa57 100644 --- a/grafast/grafserv/src/utils.ts +++ b/grafast/grafserv/src/utils.ts @@ -281,7 +281,7 @@ export function makeGraphQLWSConfig(instance: GrafservBase): ServerOptions { ); } - const { query, operationName, variableValues } = + const { query, operationName, variableValues, onError, extensions } = validateGraphQLBody(parsedBody); const { errors, document } = parseAndValidate(query); if (errors !== undefined) { @@ -296,6 +296,8 @@ export function makeGraphQLWSConfig(instance: GrafservBase): ServerOptions { contextValue, variableValues, operationName, + onError, + extensions, resolvedPreset, requestContext: grafastCtx, middleware: grafastMiddleware, @@ -361,6 +363,7 @@ export function parseGraphQLJSONBody( const query = params.query; const operationName = params.operationName ?? undefined; const variableValues = params.variables ?? undefined; + const onError = (params as any).onError ?? undefined; const extensions = params.extensions ?? undefined; return { id, @@ -368,6 +371,7 @@ export function parseGraphQLJSONBody( query, operationName, variableValues, + onError, extensions, }; } diff --git a/grafast/ruru-components/src/hooks/useFetcher.ts b/grafast/ruru-components/src/hooks/useFetcher.ts index fd89159dd0..edce71613f 100644 --- a/grafast/ruru-components/src/hooks/useFetcher.ts +++ b/grafast/ruru-components/src/hooks/useFetcher.ts @@ -15,6 +15,7 @@ import { getOperationAST, parse } from "graphql"; import { useEffect, useMemo, useState } from "react"; import type { RuruProps } from "../interfaces.js"; +import { useStorage } from "./useStorage.js"; export interface IExplainedOperation { type: string; @@ -102,6 +103,7 @@ export const useFetcher = ( props: RuruProps, options: { explain?: boolean; verbose?: boolean } = {}, ) => { + const storage = useStorage(); const [streamEndpoint, setStreamEndpoint] = useState(null); const endpoint = props.endpoint ?? "/graphql"; const url = endpoint.startsWith("/") @@ -235,8 +237,15 @@ export const useFetcher = ( return result; }; return async function ( - ...args: Parameters + ...inArgs: Parameters ): Promise> { + const [params, ...rest] = inArgs; + const onError = storage.get("onError"); + const args = [ + onError ? { ...params, onError } : params, + ...rest, + ] as const; + const result = await fetcher(...args); // Short circuit the introspection query so as to not confuse people @@ -275,7 +284,7 @@ export const useFetcher = ( return processPayload(result) as any; } }; - }, [fetcher, verbose]); + }, [fetcher, storage, verbose]); return { fetcher: wrappedFetcher, explainResults, streamEndpoint }; }; diff --git a/grafast/ruru-components/src/hooks/useStorage.ts b/grafast/ruru-components/src/hooks/useStorage.ts index 15c708b4aa..71c606337e 100644 --- a/grafast/ruru-components/src/hooks/useStorage.ts +++ b/grafast/ruru-components/src/hooks/useStorage.ts @@ -8,8 +8,13 @@ export interface StoredKeys { explainSize: string; verbose: "true" | ""; condensed: "true" | ""; + onError: "PROPAGATE" | "NULL" | "HALT" | ""; } +const storageDefault: { [key in keyof StoredKeys]?: StoredKeys[key] } = { + onError: "", +}; + const KEYS: { [key in keyof StoredKeys]: string } = { explain: "Ruru:explain", explainSize: "Ruru:explainSize", @@ -18,14 +23,19 @@ const KEYS: { [key in keyof StoredKeys]: string } = { explorerIsOpen: "graphiql:explorerIsOpen", verbose: "Ruru:verbose", condensed: "Ruru:condensed", + onError: "Ruru:onError", }; +type ToggleableKey = { + [K in keyof StoredKeys]: StoredKeys[K] extends "true" | "" ? K : never; +}[keyof StoredKeys]; + const up = (v: number) => v + 1; export interface RuruStorage { get(key: TKey): StoredKeys[TKey] | null; set(key: TKey, value: StoredKeys[TKey]): void; - toggle(key: TKey): void; + toggle(key: ToggleableKey): void; } export const useStorage = (): RuruStorage => { @@ -39,7 +49,7 @@ export const useStorage = (): RuruStorage => { return { _revision: revision, get(key) { - return cache[key] ?? null; + return cache[key] ?? storageDefault[key] ?? null; }, set(key, value) { cache[key] = value; @@ -57,7 +67,7 @@ export const useStorage = (): RuruStorage => { storage.removeItem(KEYS[key]); return null; } - return val ?? null; + return val ?? storageDefault[key] ?? null; }, set(key, value) { storage.setItem(KEYS[key], value); diff --git a/grafast/ruru-components/src/ruru.tsx b/grafast/ruru-components/src/ruru.tsx index af5995f8b7..37a4791894 100644 --- a/grafast/ruru-components/src/ruru.tsx +++ b/grafast/ruru-components/src/ruru.tsx @@ -221,6 +221,39 @@ export const RuruInner: FC<{ Condensed + { + if (storage.get("onError") === "PROPAGATE") { + storage.set("onError", ""); + } else { + storage.set("onError", "PROPAGATE"); + } + }} + > + + {storage.get("onError") === "PROPAGATE" ? check : nocheck} + onError: PROPAGATE + + + storage.set("onError", "NULL")} + > + + {storage.get("onError") === "NULL" ? check : nocheck} + onError: NULL + + + storage.set("onError", "HALT")} + > + + {storage.get("onError") === "HALT" ? check : nocheck} + onError: HALT + + diff --git a/postgraphile/postgraphile/graphile.config.ts b/postgraphile/postgraphile/graphile.config.ts index 55e9b1b0ba..1ad3b10391 100644 --- a/postgraphile/postgraphile/graphile.config.ts +++ b/postgraphile/postgraphile/graphile.config.ts @@ -521,6 +521,84 @@ const preset: GraphileConfig.Preset = { }, }, }), + // Testing errors + extendSchema({ + typeDefs: /* GraphQL */ ` + extend type Query { + errorTests: ErrorTests + } + extend type Subscription { + errorTests: ErrorTests + } + type ErrorTests { + index: Int + errorOnOddNumbers: Int! + fourtyTwo: Int! + nonNullableReturningNull: Int! + coercionFailure: Int + } + `, + objects: { + Query: { + plans: { + errorTests: EXPORTABLE( + (constant) => () => constant({}), + [constant], + ), + }, + }, + Subscription: { + plans: { + errorTests: { + subscribePlan: EXPORTABLE( + (lambda, sleep) => () => { + return lambda(null, () => { + return (async function* () { + for (let index = 0; ; index++) { + yield { index }; + await sleep(1000); + } + })(); + }); + }, + [lambda, sleep], + ), + plan: EXPORTABLE( + () => + function plan($event) { + return $event; + }, + [], + ), + }, + }, + }, + ErrorTests: { + plans: { + errorOnOddNumbers: EXPORTABLE( + (lambda) => ($payload) => + lambda($payload, (p: any) => { + if (p.index % 2 === 1) { + throw new Error(`ODD! ${p.index}`); + } else { + return p.index; + } + }), + [lambda], + ), + fourtyTwo: EXPORTABLE((constant) => () => constant(42), [constant]), + nonNullableReturningNull: EXPORTABLE( + (constant) => () => constant(null), + [constant], + ), + coercionFailure: EXPORTABLE( + (constant) => () => constant("NOT A NUMBER"), + [constant], + ), + }, + }, + }, + }), // PrimaryKeyMutationsOnlyPlugin, PersistedPlugin, ruruTitle(""), diff --git a/postgraphile/website/postgraphile/usage-schema.md b/postgraphile/website/postgraphile/usage-schema.md index f8fb73c8f4..25ab1d5e4a 100644 --- a/postgraphile/website/postgraphile/usage-schema.md +++ b/postgraphile/website/postgraphile/usage-schema.md @@ -103,7 +103,7 @@ Here's a full example: ```ts import { postgraphile } from "postgraphile"; -import { grafast } from "postgraphile/grafast"; +import { grafast, type ErrorBehavior } from "postgraphile/grafast"; import preset from "./graphile.config.js"; // Make a new PostGraphile instance: @@ -121,6 +121,7 @@ export async function executeQuery( source: string, variableValues?: Record | null, operationName?: string, + onError?: ErrorBehavior, ) { const { schema, resolvedPreset } = await pgl.getSchemaResult(); return await grafast({ @@ -128,6 +129,7 @@ export async function executeQuery( source, variableValues, operationName, + onError, resolvedPreset, requestContext, }); @@ -158,7 +160,7 @@ Here's an example using hookArgs: ```ts import { postgraphile } from "postgraphile"; import { parse, validate } from "postgraphile/graphql"; -import { execute, hookArgs } from "postgraphile/grafast"; +import { execute, hookArgs, type ErrorBehavior } from "postgraphile/grafast"; import preset from "./graphile.config.js"; // Build a `pgl` instance, with helpers and schema based on our preset. @@ -174,6 +176,7 @@ export async function executeQuery( source: string, variableValues?: Record | null, operationName?: string, + onError?: ErrorBehavior, ) { // We might get a newer schema in "watch" mode const { schema, resolvedPreset } = await pgl.getSchemaResult(); @@ -193,6 +196,7 @@ export async function executeQuery( document, variableValues, operationName, + onError, resolvedPreset, requestContext, }); @@ -218,7 +222,7 @@ import type { ResultOf, } from "@graphql-typed-document-node/core"; import { postgraphile } from "postgraphile"; -import { execute, hookArgs } from "postgraphile/grafast"; +import { execute, hookArgs, type ErrorBehavior } from "postgraphile/grafast"; import { validate } from "postgraphile/graphql"; import preset from "./graphile.config.js"; @@ -232,6 +236,7 @@ export async function executeDocument< document: TDoc, variableValues?: VariablesOf, operationName?: string, + onError?: ErrorBehavior, ): Promise, TExtensions>> { const { schema, resolvedPreset } = await pgl.getSchemaResult(); @@ -247,6 +252,7 @@ export async function executeDocument< document, variableValues, operationName, + onError, resolvedPreset, requestContext, } as any);