From 1136c621672b9037d79dc087d3fc7d12373cc8b7 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Mon, 31 Mar 2025 11:22:31 +0100 Subject: [PATCH] fix: use default retry settings when no catchError handlers are defined --- .../runEngine/services/triggerTask.server.ts | 6 +++ packages/core/src/v3/workers/taskExecutor.ts | 17 +++--- packages/core/test/taskExecutor.test.ts | 52 ++++++++++++++++++- references/hello-world/src/trigger/retry.ts | 37 +++++++++++++ 4 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 references/hello-world/src/trigger/retry.ts diff --git a/apps/webapp/app/runEngine/services/triggerTask.server.ts b/apps/webapp/app/runEngine/services/triggerTask.server.ts index a010b6632a..4ed1d5d58d 100644 --- a/apps/webapp/app/runEngine/services/triggerTask.server.ts +++ b/apps/webapp/app/runEngine/services/triggerTask.server.ts @@ -282,6 +282,12 @@ export class RunEngineTriggerTaskService extends WithRunEngine { runtimeEnvironmentId: environment.id, version: body.options?.lockToVersion, }, + select: { + id: true, + version: true, + sdkVersion: true, + cliVersion: true, + }, }) : undefined; diff --git a/packages/core/src/v3/workers/taskExecutor.ts b/packages/core/src/v3/workers/taskExecutor.ts index 6fbc207dcc..fbeddea14f 100644 --- a/packages/core/src/v3/workers/taskExecutor.ts +++ b/packages/core/src/v3/workers/taskExecutor.ts @@ -1027,6 +1027,14 @@ export class TaskExecutor { } } + const defaultRetryResult = + typeof defaultDelay === "undefined" + ? { status: "noop" as const } + : { + status: "retry" as const, + retry: { timestamp: Date.now() + defaultDelay, delay: defaultDelay }, + }; + // Check if retries are enabled in dev environment if ( execution.environment.type === "DEVELOPMENT" && @@ -1040,7 +1048,7 @@ export class TaskExecutor { const globalCatchErrorHooks = lifecycleHooks.getGlobalCatchErrorHooks(); if (globalCatchErrorHooks.length === 0 && !taskCatchErrorHook) { - return { status: "noop" }; + return defaultRetryResult; } return this._tracer.startActiveSpan( @@ -1085,12 +1093,7 @@ export class TaskExecutor { } // If no hooks handled the error, use default retry behavior - return typeof defaultDelay === "undefined" - ? { status: "noop" as const } - : { - status: "retry" as const, - retry: { timestamp: Date.now() + defaultDelay, delay: defaultDelay }, - }; + return defaultRetryResult; }, { attributes: { diff --git a/packages/core/test/taskExecutor.test.ts b/packages/core/test/taskExecutor.test.ts index 355471297c..4fc97c7229 100644 --- a/packages/core/test/taskExecutor.test.ts +++ b/packages/core/test/taskExecutor.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "vitest"; import { ConsoleInterceptor } from "../src/v3/consoleInterceptor.js"; import { + RetryOptions, RunFnParams, ServerBackgroundWorker, TaskMetadataWithFunctions, @@ -784,6 +785,48 @@ describe("TaskExecutor", () => { expect((result as any).result.retry.delay).toBeLessThan(30100); }); + test("should use the default retry settings if no catch error hook is provided", async () => { + const expectedError = new Error("Task failed intentionally"); + + const task = { + id: "test-task", + fns: { + run: async (payload: any, params: RunFnParams) => { + throw expectedError; + }, + }, + }; + + const result = await executeTask(task, { test: "data" }, undefined, { + maxAttempts: 3, + minTimeoutInMs: 1000, + maxTimeoutInMs: 5000, + factor: 2, + }); + + // Verify the final result contains the specific retry timing + expect(result).toEqual({ + result: { + ok: false, + id: "test-run-id", + error: { + type: "BUILT_IN_ERROR", + message: "Task failed intentionally", + name: "Error", + stackTrace: expect.any(String), + }, + retry: { + timestamp: expect.any(Number), + delay: expect.any(Number), + }, + skippedRetrying: false, + }, + }); + + expect((result as any).result.retry.delay).toBeGreaterThan(1000); + expect((result as any).result.retry.delay).toBeLessThan(3000); + }); + test("should execute middleware hooks in correct order around other hooks", async () => { const executionOrder: string[] = []; @@ -1623,7 +1666,12 @@ describe("TaskExecutor", () => { }); }); -function executeTask(task: TaskMetadataWithFunctions, payload: any, signal?: AbortSignal) { +function executeTask( + task: TaskMetadataWithFunctions, + payload: any, + signal?: AbortSignal, + retrySettings?: RetryOptions +) { const tracingSDK = new TracingSDK({ url: "http://localhost:4318", }); @@ -1643,7 +1691,7 @@ function executeTask(task: TaskMetadataWithFunctions, payload: any, signal?: Abo consoleInterceptor, retries: { enabledInDev: false, - default: { + default: retrySettings ?? { maxAttempts: 1, }, }, diff --git a/references/hello-world/src/trigger/retry.ts b/references/hello-world/src/trigger/retry.ts new file mode 100644 index 0000000000..02ea019b35 --- /dev/null +++ b/references/hello-world/src/trigger/retry.ts @@ -0,0 +1,37 @@ +import { logger, task } from "@trigger.dev/sdk"; + +type RetryPayload = { + failCount: number; +}; + +export const retryTask = task({ + id: "retry-task", + // Configure 5 retries with exponential backoff + retry: { + maxAttempts: 5, + factor: 1.8, + minTimeoutInMs: 20, + maxTimeoutInMs: 100, + randomize: false, + }, + run: async (payload: RetryPayload, { ctx }) => { + const currentAttempt = ctx.attempt.number; + logger.info("Running retry task", { + currentAttempt, + desiredFailCount: payload.failCount, + }); + + // If we haven't reached the desired number of failures yet, throw an error + if (currentAttempt <= payload.failCount) { + const error = new Error(`Intentionally failing attempt ${currentAttempt}`); + logger.warn("Task failing", { error, currentAttempt }); + throw error; + } + + // If we've made it past the desired fail count, return success + logger.info("Task succeeded", { currentAttempt }); + return { + attemptsTaken: currentAttempt, + }; + }, +});