Fix issues around error handling from side effect steps

Plan that's hard to make sense of:

function plan($parent) {
  sideEffect($parent, parent => {
    if (parent.something) { throw new Error("Nope!"); }
  });
  return $parent;
}

This is something you might want to do for access control. But the issue is you're returning a pre-existing step, so this step cannot be made dependent on the side effect (that would cause a loop).

Still; when we output the field for this plan, the error from the side effect should have taken effect.

Currently you can accomplish this by making the return value explicitly dependent:

function plan($parent) {
  return sideEffect($parent, parent => {
    if (parent.something) { throw new Error("Nope!"); }
    else { return parent };
  });
}

but this changes the type of the return value, so it's not always desired.

Took a while to figure out what to do here, but the solution is that the OutputPlan should be dependent on the side effects.

After further thought, I realised that you only need to track the latest side effect (since each side effect is implicitly dependent on previous side effects).

We also have the situation where you might trap a side effect; if you do this then the explicit (but indirect) dependency on the side effect should win, so we should remove the implicit dependency (directly) on the side effect. This won't affect later steps which remain implicitly dependent on the side effect (unless dependent on the trap, of course).

So, we track the latest side effect, and every future step is implicitly (if not explicitly) dependent on it, and also the OutputPlan is dependent on it. This final part needs to be handled carefully, essentially if the OutputPlan's step already has a dependency change on the sideEffect (which might be trapped) then no worries; but if not then we need an implicit dependency.

TODO:

[x] Need to ensure we don’t execute steps after latestSideEffectStep fails
[x] Can remove previous "side effect" error handling code with this much simpler solution; also saves us from another loop.
[x] Need to make it so when you add a dependency we remove latestSideEffectStep if it’s now in the chain
[x] Need to factor into deduplication
[x] Need to prevent hoisting past side effect
[x] Should this impact pushing down?
[x] Need to factor into phase calculations
[x] Need to consider when a layer plan inherits - that layer plan implicitly becomes dependent on the side effect too.
[x] Need to make side effects implicitly dependent on previous side effects (already handled?)
[ ] Should also render these "side effect" dependencies into the plan diagram to make debugging easier.

🦋 Changeset detected

Latest commit: 7692c01e02ceaace08bb73cbbb316ead6e5bff72

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

| Name | Type | | -------------------------- | ----- | | grafast | Patch | | @localrepo/grafast-bench | Patch | | @dataplan/json | Patch | | @dataplan/pg | Patch | | @grafserv/persisted | Patch | | grafserv | Patch | | ruru-components | Patch | | @localrepo/grafast-website | Patch | | graphile-build-pg | Patch | | graphile-build | Patch | | graphile-utils | Patch | | pgl | Patch | | postgraphile | Patch | | graphile-export | Patch | | graphile | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

graphile / crystal

Fix issues around error handling from side effect steps #2114

🦋 Changeset detected