Skip to content

Commit

Permalink
Use StepErrors for unhandled error catching (#474)
Browse files Browse the repository at this point in the history
## Summary
<!-- Succinctly describe your change, providing context, what you've
changed, and why. -->

Aligns with the
[spec](https://github.com/inngest/inngest/blob/main/docs/SDK_SPEC.md)
for throwing errors when memoizing data. This previous solution was for
the initial release of `step.invoke()` pre-per-step errors.

Functionally there is no change, though the UI will no longer show
`NonRetriableError` prefixes and will prefer user error data.

## 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
- [x] Added unit/integration tests
- [x] Added changesets if applicable
  • Loading branch information
jpwilliams committed Jan 31, 2024
1 parent 16f02e9 commit b3a7b39
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/wild-tables-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Improve UI when showing an unhandled `StepError`
3 changes: 0 additions & 3 deletions packages/inngest/src/components/InngestStepTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,6 @@ export const createStepTools = <
*
* A string ID can also be passed to reference functions outside of the
* current app.
*
* If a function isn't found or otherwise errors, the step will fail and
* throw a `NonRetriableError`.
*/
invoke: createTool<
<TFunction extends InvokeTargetFunctionDefinition>(
Expand Down
32 changes: 4 additions & 28 deletions packages/inngest/src/components/execution/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,15 +456,7 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
* we should return a `NonRetriableError` to stop execution.
*/
if (typeof output.error !== "undefined") {
const serializedError = serializeError(output.error);
output.data = serializedError;

if (output.error === this.state.recentlyRejectedStepError) {
output.error = new NonRetriableError(serializedError.message, {
cause: output.error,
});
output.data = serializeError(output.error);
}
output.data = serializeError(output.error);
}

const transformedOutput = await this.state.hooks?.transformOutput?.({
Expand All @@ -479,7 +471,9 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {
* Ensure we give middleware the chance to decide on retriable behaviour
* by looking at the error returned from output transformation.
*/
let retriable: boolean | string = !(error instanceof NonRetriableError);
let retriable: boolean | string = !(
error instanceof NonRetriableError || error instanceof StepError
);
if (retriable && error instanceof RetryAfterError) {
retriable = error.retryAfter;
}
Expand Down Expand Up @@ -761,24 +755,6 @@ class V1InngestExecution extends InngestExecution implements IInngestExecution {

if (typeof stepState.data !== "undefined") {
resolve(stepState.data);
} else if (opId.op === StepOpCode.InvokeFunction) {
const errCheck = z
.object({ message: z.string() })
.safeParse(stepState.error);

const errMessage = [
"Invoking function failed",
errCheck.success ? errCheck.data.message : "",
]
.map((s) => s.trim())
.filter(Boolean)
.join("; ");

const err = new NonRetriableError(errMessage, {
cause: stepState.error,
});

reject(err);
} else {
this.state.recentlyRejectedStepError = new StepError(
opId.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("run", () => {
expect(item).toBeDefined();

const output = await item?.getOutput();
expect(output?.name).toEqual("NonRetriableError");
expect(output?.message).toContain("Invoking function failed");
expect(output?.name).toEqual("Error");
expect(output?.message).toContain("could not find function");
}, 20000);
});

0 comments on commit b3a7b39

Please sign in to comment.