Enable coercion of trapped values (#2024)

.changeset/old-countries-hear.md

@@ -0,0 +1,5 @@
+---
+"grafast": patch
+---
+
+Planning error paths improved to indicate list positions.

.changeset/poor-sheep-itch.md

@@ -0,0 +1,6 @@
+---
+"grafast": patch
+---
+
+Introduce `trap()` step to trap errors and inhibits, and coerce them to a chosen
+form (e.g. null, empty list).

grafast/grafast/__tests__/trap-test.ts

@@ -0,0 +1,149 @@
+/* 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 type { ExecutionResult } from "graphql";
+import { it } from "mocha";
+import sqlite3 from "sqlite3";
+
+import type { ExecutionDetails, GrafastResultsList } from "../dist/index.js";
+import {
+  access,
+  assertNotNull,
+  context,
+  ExecutableStep,
+  grafast,
+  lambda,
+  list,
+  makeGrafastSchema,
+  trap,
+  TRAP_ERROR,
+} from "../dist/index.js";
+
+const makeSchema = () => {
+  return makeGrafastSchema({
+    typeDefs: /* GraphQL */ `
+      type Error {
+        message: String
+      }
+      type Query {
+        unhandledError(setNullToError: Int): Int
+        errorToNull(setNullToError: Int): Int
+        errorToEmptyList(setNullToError: Int): [Int]
+        errorToError(setNullToError: Int): Error
+      }
+    `,
+    plans: {
+      Query: {
+        unhandledError(_, { $setNullToError }) {
+          const $a = assertNotNull($setNullToError, "Null!");
+          return $a;
+        },
+        errorToNull(_, { $setNullToError }) {
+          const $a = assertNotNull($setNullToError, "Null!");
+          return trap($a, TRAP_ERROR, { valueForError: "NULL" });
+        },
+        errorToEmptyList(_, { $setNullToError }) {
+          const $a = assertNotNull($setNullToError, "Null!");
+          const $list = list([$a]);
+          return trap($list, TRAP_ERROR, { valueForError: "EMPTY_LIST" });
+        },
+        errorToError(_, { $setNullToError }) {
+          const $a = assertNotNull($setNullToError, "Null!");
+          const $derived = lambda($a, () => null, true);
+          return trap($derived, TRAP_ERROR, { valueForError: "PASS_THROUGH" });
+        },
+      },
+    },
+    enableDeferStream: false,
+  });
+};
+
+it("schema works as expected", async () => {
+  const schema = makeSchema();
+  const source = /* GraphQL */ `
+    query Q {
+      nonError: unhandledError(setNullToError: 2)
+      error: unhandledError(setNullToError: null)
+    }
+  `;
+  const variableValues = {};
+  const result = (await grafast({
+    schema,
+    source,
+    variableValues,
+    contextValue: {},
+    resolvedPreset: {},
+    requestContext: {},
+  })) as ExecutionResult;
+  expect(result.errors).to.exist;
+  expect(result.errors).to.have.length(1);
+  expect(result.errors![0].path).to.deep.equal(["error"]);
+  expect(result.errors![0].message).to.equal("Null!");
+  expect(result.data).to.deep.equal({ nonError: 2, error: null });
+});
+it("enables trapping an error to null", async () => {
+  const schema = makeSchema();
+  const source = /* GraphQL */ `
+    query Q {
+      nonError: errorToNull(setNullToError: 2)
+      error: errorToNull(setNullToError: null)
+    }
+  `;
+  const variableValues = {};
+  const result = (await grafast({
+    schema,
+    source,
+    variableValues,
+    contextValue: {},
+    resolvedPreset: {},
+    requestContext: {},
+  })) as ExecutionResult;
+  expect(result.errors).to.not.exist;
+  expect(result.data).to.deep.equal({ nonError: 2, error: null });
+});
+it("enables trapping an error to emptyList", async () => {
+  const schema = makeSchema();
+  const source = /* GraphQL */ `
+    query Q {
+      nonError: errorToEmptyList(setNullToError: 2)
+      error: errorToEmptyList(setNullToError: null)
+    }
+  `;
+  const variableValues = {};
+  const result = (await grafast({
+    schema,
+    source,
+    variableValues,
+    contextValue: {},
+    resolvedPreset: {},
+    requestContext: {},
+  })) as ExecutionResult;
+  expect(result.errors).to.not.exist;
+  expect(result.data).to.deep.equal({ nonError: [2], error: [] });
+});
+it("enables trapping an error to error", async () => {
+  const schema = makeSchema();
+  const source = /* GraphQL */ `
+    query Q {
+      nonError: errorToError(setNullToError: 2) {
+        message
+      }
+      error: errorToError(setNullToError: null) {
+        message
+      }
+    }
+  `;
+  const variableValues = {};
+  const result = (await grafast({
+    schema,
+    source,
+    variableValues,
+    contextValue: {},
+    resolvedPreset: {},
+    requestContext: {},
+  })) as ExecutionResult;
+  expect(result.errors).to.not.exist;
+  expect(result.data).to.deep.equal({
+    nonError: null,
+    error: { message: "Null!" },
+  });
+});

grafast/grafast/src/engine/OperationPlan.ts

@@ -26,6 +26,7 @@ import {
 import { fieldSelectionsForType } from "../graphqlMergeSelectionSets.js";
 import type { GrafastPlanJSON, ModifierStep } from "../index.js";
 import {
+  __FlagStep,
   __ItemStep,
   __TrackedValueStep,
   __ValueStep,
@@ -39,6 +40,7 @@ import {
 import { inputStep } from "../input.js";
 import { inspect } from "../inspect.js";
 import type {
+  AddDependencyOptions,
   FieldPlanResolver,
   GrafastPlanBucketJSONv1,
   GrafastPlanBucketPhaseJSONv1,
@@ -49,7 +51,14 @@ import type {
   StepOptions,
   TrackedArguments,
 } from "../interfaces.js";
-import { $$proxy, $$subroutine, $$timeout, $$ts } from "../interfaces.js";
+import {
+  $$proxy,
+  $$subroutine,
+  $$timeout,
+  $$ts,
+  ALL_FLAGS,
+  DEFAULT_ACCEPT_FLAGS,
+} from "../interfaces.js";
 import type { ApplyAfterModeArg } from "../operationPlan-input.js";
 import { withFieldArgsForArguments } from "../operationPlan-input.js";
 import type { ListCapableStep, PolymorphicStep } from "../step.js";
@@ -66,12 +75,14 @@ import { access } from "../steps/access.js";
 import { constant, ConstantStep } from "../steps/constant.js";
 import { graphqlResolver } from "../steps/graphqlResolver.js";
 import { timeSource } from "../timeSource.js";
+import type { Sudo } from "../utils.js";
 import {
   defaultValueToValueNode,
   findVariableNamesUsed,
   hasItemPlan,
   isTypePlanned,
   sudo,
+  writeableArray,
 } from "../utils.js";
 import type {
   LayerPlanPhase,
@@ -387,6 +398,10 @@ export class OperationPlan {
     this.checkTimeout();
     this.lap("optimizeSteps");
 
+    this.inlineSteps();
+    this.checkTimeout();
+    this.lap("inlineSteps");
+
     this.phase = "finalize";
 
     this.stepTracker.finalizeSteps();
@@ -1368,7 +1383,7 @@ export class OperationPlan {
       const $item = isListCapableStep($step)
         ? withGlobalLayerPlan(
             $__item.layerPlan,
-            polymorphicPaths,
+            $__item.polymorphicPaths,
             ($step as ListCapableStep<any>).listItem,
             $step,
             $__item,
@@ -1464,8 +1479,8 @@ export class OperationPlan {
             }
           } catch (e) {
             throw new Error(
-              `The step returned by '${path.join(
-                ".",
+              `The step returned by '${path.join(".")}${"[]".repeat(
+                listDepth,
               )}' is not compatible with the GraphQL object type '${
                 nullableFieldType.name
               }': ${e.message}`,
@@ -2448,6 +2463,8 @@ export class OperationPlan {
           // have not been able to come up with a counterexample that is
           // unsafe. Should we do so, we should remove this.
           break;
+        } else if (step instanceof __FlagStep) {
+          break;
         } else {
           return;
         }
@@ -2483,6 +2500,8 @@ export class OperationPlan {
           // OPTIMIZE: figure out under which circumstances it is safe to hoist here.
           // break;
           return;
+        } else if (step instanceof __FlagStep) {
+          break;
         } else {
           // Plans that rely on external state shouldn't be hoisted because
           // their results may change after a mutation, so the mutation should
@@ -3064,6 +3083,70 @@ export class OperationPlan {
     );
   }
 
+  private inlineSteps() {
+    flagLoop: for (const $flag of this.stepTracker.activeSteps) {
+      if ($flag instanceof __FlagStep) {
+        // We can only inline it if it's not used by an output plan or layer plan
+        {
+          const usages = this.stepTracker.outputPlansByRootStep.get($flag);
+          if (usages?.size) {
+            continue;
+          }
+        }
+        {
+          const usages = this.stepTracker.layerPlansByRootStep.get($flag);
+          if (usages?.size) {
+            continue;
+          }
+        }
+        {
+          const usages = this.stepTracker.layerPlansByParentStep.get($flag);
+          if (usages?.size) {
+            continue;
+          }
+        }
+
+        // We're only going to inline one if we can inline all.
+        const toInline: Array<{
+          $dependent: Sudo<ExecutableStep>;
+          dependencyIndex: number;
+          inlineDetails: AddDependencyOptions;
+          dependent: ExecutableStep["dependents"][number];
+        }> = [];
+        for (const dependent of $flag.dependents) {
+          const { step, dependencyIndex } = dependent;
+          const $dependent = sudo(step);
+          const inlineDetails = $flag.inline(
+            $dependent.getDepOptions(dependencyIndex),
+          );
+          if (inlineDetails === null) {
+            continue flagLoop;
+          }
+          toInline.push({
+            $dependent,
+            dependencyIndex,
+            inlineDetails,
+            dependent,
+          });
+        }
+        // All of them pass the check, let's inline them
+        for (const todo of toInline) {
+          const {
+            $dependent,
+            dependencyIndex,
+            inlineDetails: { onReject, acceptFlags = DEFAULT_ACCEPT_FLAGS },
+          } = todo;
+          writeableArray($dependent.dependencyOnReject)[dependencyIndex] =
+            onReject;
+          writeableArray($dependent.dependencyForbiddenFlags)[dependencyIndex] =
+            ALL_FLAGS & ~acceptFlags;
+        }
+        const $flagDep = sudo($flag).dependencies[0];
+        this.stepTracker.replaceStep($flag, $flagDep);
+      }
+    }
+  }
+
   /** Finalizes each step */
   private finalizeSteps(): void {
     const initialStepCount = this.stepTracker.stepCount;

grafast/grafast/src/engine/StepTracker.ts

@@ -5,7 +5,7 @@ import type { AddDependencyOptions } from "../interfaces.js";
 import { $$subroutine, ALL_FLAGS, TRAPPABLE_FLAGS } from "../interfaces.js";
 import { ExecutableStep } from "../step.js";
 import { __FlagStep } from "../steps/__flag.js";
-import { sudo } from "../utils.js";
+import { sudo, writeableArray } from "../utils.js";
 import type {
   LayerPlan,
   LayerPlanReasonSubroutine,
@@ -14,17 +14,6 @@ import type {
 import { lock } from "./lock.js";
 import type { OutputPlan } from "./OutputPlan.js";
 
-/**
- * We want everything else to treat things like `dependencies` as read only,
- * however we ourselves want to be able to write to them, so we can use
- * writeable for this.
- *
- * @internal
- */
-function writeableArray<T>(a: ReadonlyArray<T>): Array<T> {
-  return a as any;
-}
-
 /**
  * This class keeps track of all of our steps, and the dependencies between
  * steps and other steps, layer plans and steps, and output plans and steps.
@@ -282,13 +271,6 @@ export class StepTracker {
   ): number {
     const $dependent = sudo(raw$dependent);
     const $dependency = sudo(options.step);
-    if ($dependency instanceof __FlagStep) {
-      // See if we can inline this
-      const inlineDetails = $dependency.inline(options);
-      if (inlineDetails !== null) {
-        return this.addStepDependency($dependent, inlineDetails);
-      }
-    }
     if (!this.activeSteps.has($dependent)) {
       throw new Error(
         `Cannot add ${$dependency} as a dependency of ${$dependent}; the latter is deleted!`,