fix NonRetryableError thrown with an empty error message not stopping workflow retries locally (#10228)

Author: dario-piotrowicz

.changeset/shy-coats-drum.md

@@ -0,0 +1,6 @@
+---
+"@cloudflare/workflows-shared": patch
+"wrangler": patch
+---
+
+fix `NonRetryableError` thrown with an empty error message not stopping workflow retries locally

fixtures/workflow/src/index.ts

@@ -4,6 +4,7 @@ import {
 	WorkflowEvent,
 	WorkflowStep,
 } from "cloudflare:workers";
+import { NonRetryableError } from "cloudflare:workflows";
 
 type Params = {
 	name: string;
@@ -31,13 +32,46 @@ export class Demo extends WorkflowEntrypoint<{}, Params> {
 	}
 }
 
+export class Demo3 extends WorkflowEntrypoint<{}, Params> {
+	async run(
+		event: WorkflowEvent<Params & { doRetry: boolean; errorMessage: string }>,
+		step: WorkflowStep
+	) {
+		let runs = -1;
+		try {
+			await step.do(
+				"First (and only step) step",
+				{
+					retries: {
+						limit: 3,
+						delay: 100,
+					},
+				},
+				async function () {
+					runs++;
+					if (event.payload.doRetry) {
+						throw new Error(event.payload.errorMessage);
+					} else {
+						throw new NonRetryableError(event.payload.errorMessage);
+					}
+				}
+			);
+		} catch {}
+
+		return `The step was retried ${runs} time${runs === 1 ? "" : "s"}`;
+	}
+}
+
 type Env = {
 	WORKFLOW: Workflow;
+	WORKFLOW3: Workflow<{ doRetry: boolean; errorMessage: string }>;
 };
 export default class extends WorkerEntrypoint<Env> {
 	async fetch(req: Request) {
 		const url = new URL(req.url);
 		const id = url.searchParams.get("workflowName");
+		const doRetry = url.searchParams.get("doRetry");
+		const errorMessage = url.searchParams.get("errorMessage");
 
 		if (url.pathname === "/favicon.ico") {
 			return new Response(null, { status: 404 });
@@ -50,6 +84,20 @@ export default class extends WorkerEntrypoint<Env> {
 			} else {
 				handle = await this.env.WORKFLOW.create({ id });
 			}
+		} else if (url.pathname === "/createDemo3") {
+			if (id === null) {
+				handle = await this.env.WORKFLOW3.create();
+			} else {
+				handle = await this.env.WORKFLOW3.create({
+					id,
+					params: {
+						doRetry: doRetry === "false" ? false : true,
+						errorMessage: errorMessage ?? "",
+					},
+				});
+			}
+		} else if (url.pathname === "/get3") {
+			handle = await this.env.WORKFLOW3.get(id);
 		} else {
 			handle = await this.env.WORKFLOW.get(id);
 		}

fixtures/workflow/tests/index.test.ts

@@ -1,7 +1,8 @@
+import { randomUUID } from "crypto";
 import { rm } from "fs/promises";
 import { resolve } from "path";
 import { fetch } from "undici";
-import { afterAll, beforeAll, describe, it, vi } from "vitest";
+import { afterAll, beforeAll, describe, it, test, vi } from "vitest";
 import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived";
 
 describe("Workflows", () => {
@@ -98,4 +99,86 @@ describe("Workflows", () => {
 			name: "Error",
 		});
 	});
+
+	describe("retrying a step", () => {
+		test("should retry a step if a generic Error (with a generic error message) is thrown", async ({
+			expect,
+		}) => {
+			const name = randomUUID();
+			await fetchJson(
+				`http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=true&errorMessage=generic_error_message`
+			);
+
+			await vi.waitFor(
+				async () => {
+					const result = await fetchJson(
+						`http://${ip}:${port}/get3?workflowName=${name}`
+					);
+
+					expect(result["output"]).toEqual("The step was retried 3 times");
+				},
+				{ timeout: 1500 }
+			);
+		});
+
+		test("should retry a step if a generic Error (with an empty error message) is thrown", async ({
+			expect,
+		}) => {
+			const name = randomUUID();
+			await fetchJson(
+				`http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=true&errorMessage=`
+			);
+
+			await vi.waitFor(
+				async () => {
+					const result = await fetchJson(
+						`http://${ip}:${port}/get3?workflowName=${name}`
+					);
+
+					expect(result["output"]).toEqual("The step was retried 3 times");
+				},
+				{ timeout: 1500 }
+			);
+		});
+
+		test("should not retry a step if a NonRetryableError (with a generic error message) is thrown", async ({
+			expect,
+		}) => {
+			const name = randomUUID();
+			await fetchJson(
+				`http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=false&errorMessage=generic_error_message"`
+			);
+
+			await vi.waitFor(
+				async () => {
+					const result = await fetchJson(
+						`http://${ip}:${port}/get3?workflowName=${name}`
+					);
+
+					expect(result["output"]).toEqual("The step was retried 0 times");
+				},
+				{ timeout: 1500 }
+			);
+		});
+
+		test("should not retry a step if a NonRetryableError (with an empty error message) is thrown", async ({
+			expect,
+		}) => {
+			const name = randomUUID();
+			await fetchJson(
+				`http://${ip}:${port}/createDemo3?workflowName=${name}&doRetry=false&errorMessage=`
+			);
+
+			await vi.waitFor(
+				async () => {
+					const result = await fetchJson(
+						`http://${ip}:${port}/get3?workflowName=${name}`
+					);
+
+					expect(result["output"]).toEqual("The step was retried 0 times");
+				},
+				{ timeout: 1500 }
+			);
+		});
+	});
 });

fixtures/workflow/wrangler.jsonc

@@ -8,5 +8,10 @@
 			"name": "my-workflow",
 			"class_name": "Demo",
 		},
+		{
+			"binding": "WORKFLOW3",
+			"name": "my-workflow3",
+			"class_name": "Demo3",
+		},
 	],
 }

packages/workflows-shared/src/context.ts

@@ -343,7 +343,7 @@ export class Context extends RpcTarget {
 				if (
 					e instanceof Error &&
 					(error.name === "NonRetryableError" ||
-						error.message.startsWith("NonRetryableError:"))
+						error.message.startsWith("NonRetryableError"))
 				) {
 					this.#engine.writeLog(
 						InstanceEvent.ATTEMPT_FAILURE,