Skip to content

Commit

Permalink
Fix false indeterminate function warning (#757)
Browse files Browse the repository at this point in the history
## Summary
<!-- Succinctly describe your change, providing context, what you've
changed, and why. -->

Fixes a false indeterminate function warning when using edited inputs in
step state.

```
=================================================

⚠️  Function may be indeterminate

This is expected if a function is updated in the middle of a run, but may indicate a bug if not. We found new steps before seeing all previous steps, which may indicate that the function is non-deterministic. This may cause unexpected behaviour as Inngest executes your function.

=================================================
```

We show this warning when finding a brand new step that the Executor
doesn't yet know about. If we've found this and there's a difference
between the number of steps we've previously found and the amount of
steps the Executor has given us, we display the warning.

The value we used for these counts was the `fulfilled` boolean, which
tracks whether a step will resolve or reject based on Executor state.
This, however, doesn't account for known steps when the Executor
provides an `input` instead.

This PR adds a small `hasStepState` boolean to each found step, such
that we know if the Executor has any knowledge of the step without
scoping it down to just resolve/reject.

## Checklist
<!-- Tick these items off as you progress. -->
<!-- If an item isn't applicable, ideally please strikeout the item by
wrapping it in "~~"" and suffix it with "N/A My reason for skipping
this." -->
<!-- e.g. "- [ ] ~~Added tests~~ N/A Only touches docs" -->

- [ ] ~Added a [docs PR](https://github.com/inngest/website) that
references this PR~ N/A Bug fix
- [ ] ~Added unit/integration tests~ N/A Warning log
- [x] Added changesets if applicable

## Related
<!-- A space for any related links, issues, or PRs. -->
<!-- Linear issues are autolinked. -->
<!-- e.g. - INN-123 -->
<!-- GitHub issues/PRs can be linked using shorthand. -->
<!-- e.g. "- inngest/inngest#123" -->
<!-- Feel free to remove this section if there are no applicable related
links.-->
- Introduced in #748
  • Loading branch information
jpwilliams authored Nov 25, 2024
1 parent 8af4c25 commit 36b61f0
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/sharp-donuts-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Fix false indeterminate function warning
22 changes: 22 additions & 0 deletions packages/inngest/src/components/InngestStepTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,29 @@ export interface FoundStep extends HashedOp {
hashedId: string;
fn?: (...args: unknown[]) => unknown;
rawArgs: unknown[];

/**
* A boolean representing whether the step has been fulfilled, either
* resolving or rejecting the `Promise` returned to userland code.
*
* Note that this is distinct from {@link hasStepState}, which instead tracks
* whether the step has been given some state from the Executor. State from
* the Executor could be data other than a resolution or rejection, such as
* inputs.
*/
fulfilled: boolean;

/**
* A boolean representing whether the step has been given some state from the
* Executor. State from the Executor could be data other than a resolution or
* rejection, such as inputs.
*
* This is distinct from {@link fulfilled}, which instead tracks whether the
* step has been fulfilled, either resolving or rejecting the `Promise`
* returned to userland code.
*/
hasStepState: boolean;

handled: boolean;

/**
Expand Down
9 changes: 5 additions & 4 deletions packages/inngest/src/components/execution/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
}

private async filterNewSteps(
steps: FoundStep[]
foundSteps: FoundStep[]
): Promise<[OutgoingOp, ...OutgoingOp[]] | void> {
if (this.options.requestedRunStep) {
return;
Expand All @@ -292,7 +292,7 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
/**
* Gather any steps that aren't memoized and report them.
*/
const newSteps = steps.filter((step) => !step.fulfilled);
const newSteps = foundSteps.filter((step) => !step.fulfilled);

if (!newSteps.length) {
return;
Expand All @@ -303,8 +303,8 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
* steps. This may indicate that step presence isn't determinate.
*/
const stepsToFulfil = Object.keys(this.state.stepState).length;
const fulfilledSteps = steps.filter((step) => step.fulfilled).length;
const foundAllCompletedSteps = stepsToFulfil === fulfilledSteps;
const knownSteps = foundSteps.filter((step) => step.hasStepState).length;
const foundAllCompletedSteps = stepsToFulfil === knownSteps;

if (!foundAllCompletedSteps) {
// TODO Tag
Expand Down Expand Up @@ -908,6 +908,7 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
fn: opts?.fn ? () => opts.fn?.(...fnArgs) : undefined,
promise,
fulfilled: isFulfilled,
hasStepState: Boolean(stepState),
displayName: opId.displayName ?? opId.id,
handled: false,
handle: async () => {
Expand Down

0 comments on commit 36b61f0

Please sign in to comment.