-
-
Notifications
You must be signed in to change notification settings - Fork 671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v4: New lifecycle hooks #1817
v4: New lifecycle hooks #1817
Conversation
🦋 Changeset detectedLatest commit: 6d5d10a The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
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 |
WalkthroughThis pull request introduces a wide range of updates across documentation, UI components, build processes, task execution, and core API functionality. Key modifications include updating command examples with a new flag, enhancing icon rendering logic, refining error handling, and extending lifecycle and local management APIs. Additionally, new lifecycle hook functions and middleware support have been integrated into worker and task executor modules, and example projects now include database middleware and task lifecycle handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Executor as TaskExecutor
participant Hooks as LifecycleHooksManager
participant Middleware
Client->>Executor: Initiate Task Execution
Executor->>Hooks: Call onWait hook listeners (pre-execution)
Executor->>Middleware: Execute middleware chain
Middleware-->>Executor: Proceed with task logic
alt Error Occurs
Executor->>Hooks: Call onCatchError hook listeners
Executor->>Executor: Process error handling result
end
Executor->>Hooks: Call onResume hook listeners (post-execution)
Executor->>Hooks: Call onComplete hook listeners
Executor-->>Client: Return task result
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (24)
apps/webapp/app/components/runs/v3/RunIcon.tsx (2)
31-32
: Remove console.log statementThis appears to be debugging code that shouldn't be committed to production.
- console.log("spanName", spanName, name); -
81-102
: Consider refactoring repetitive task hook icon definitionsAll the new task hook cases return identical code. Consider refactoring to reduce repetition.
- case "task-middleware": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-fn-run": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-init": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onStart": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onSuccess": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onFailure": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onComplete": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onWait": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onResume": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-catchError": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-cleanup": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; + case "task-middleware": + case "task-fn-run": + case "task-hook-init": + case "task-hook-onStart": + case "task-hook-onSuccess": + case "task-hook-onFailure": + case "task-hook-onComplete": + case "task-hook-onWait": + case "task-hook-onResume": + case "task-hook-catchError": + case "task-hook-cleanup": + return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;packages/core/src/v3/locals/index.ts (2)
9-20
: Singleton pattern implementation for LocalsAPIThe LocalsAPI class follows the singleton pattern correctly, but there's a minor issue with static method implementation.
Replace
this
with explicit class name in static methods to improve clarity:public static getInstance(): LocalsAPI { - if (!this._instance) { - this._instance = new LocalsAPI(); + if (!LocalsAPI._instance) { + LocalsAPI._instance = new LocalsAPI(); } - return this._instance; + return LocalsAPI._instance; }🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 16-16: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 19-19: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
30-40
: Implementation of LocalsManager interface methodsThe interface methods are properly implemented, but there's a minor issue with the return type of
setLocal
.Remove the return statement in
setLocal
since it has a void return type:public setLocal<T>(key: LocalsKey<T>, value: T): void { - return this.#getManager().setLocal(key, value); + this.#getManager().setLocal(key, value); }🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/core/src/v3/lifecycleHooks/index.ts (2)
29-35
: Use the class name instead ofthis
in static context.Static analysis indicates that referencing
this
within a static method can be confusing. You can avoid potential confusion (and lint issues) by referencingLifecycleHooksAPI._instance
directly. Consider using:- if (!this._instance) { - this._instance = new LifecycleHooksAPI(); - } - return this._instance; + if (!LifecycleHooksAPI._instance) { + LifecycleHooksAPI._instance = new LifecycleHooksAPI(); + } + return LifecycleHooksAPI._instance;Additionally, if multiple threads or asynchronous calls invoke
getInstance()
simultaneously, you may encounter race conditions. You might want to add locking or ensure this initialization code runs prior to concurrent access.🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 31-31: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 34-34: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
37-44
: Consider concurrency checks when registering/unregistering global state.
setGlobalLifecycleHooksManager()
anddisable()
manage a global reference. If these operations can be triggered concurrently, consider adding synchronization to avoid subtle race conditions.packages/core/src/v3/locals/manager.ts (1)
37-37
: Remove stray characterThere's a stray "0;" at the end of the file that should be removed.
-0;
packages/core/src/v3/runtime/managedRuntimeManager.ts (3)
56-59
: Audit error handling before resuming.Similar to the wait process, the resume process uses
await Promise.all(...)
for listeners. Consider whether a single listener rejection should fail the entirewaitForTask
flow or if you’d like to isolate errors.
92-96
: Double-check resilience during batch resume.Just like the wait logic, confirm whether a single hook's rejection in
callOnResumeHookListeners
should block resuming the batch. Consider capturing errors in a logging mechanism to avoid unexplained failures in one hook blocking the entire flow.Would you like me to propose a small code snippet to handle partial failures during batch resume?
131-141
: Consider uniform error handling for resume logic.The conditional usage of
callOnResumeHookListeners
parallels the wait logic, but the same caution aboutPromise.all
rejections applies here. You might consider a try/catch approach or capturing errors individually if graceful degradation is required.packages/core/src/v3/lifecycleHooks/manager.ts (2)
65-300
: Extensive duplication of register/get methods.Each hook type (start, init, failure, success, complete, etc.) uses near-identical logic. Consider extracting common logic into helper functions or using a single map with a composite key for the hook type and task ID to reduce duplication.
601-603
: Potential collisions ingenerateHookId
.Using
hook.fn.toString()
might cause large string IDs and unforeseen collisions for complex or transcompiled functions. Consider generating a short hash of the function instead to optimize memory usage and reduce risk of collisions.packages/core/src/v3/lifecycle-hooks-api.ts (1)
1-5
: Centralized lifecycle API instance.Exporting a singleton (
lifecycleHooks
) is a convenient global entry point. However, if testability or multi-tenant usage is needed in the future, you may consider a factory-based or dependency-injection approach instead.packages/cli-v3/src/build/bundle.ts (1)
239-249
: Enhance flexibility for isInitEntryPoint checks
This helper method cleanly ensuresinit.ts
is located at the root of configured dirs. As a future improvement, consider allowing a user-defined init file name if needed or handling subdirectories.packages/core/src/v3/workers/taskExecutor.ts (5)
23-24
: Minor clarity nitpick:
Consider grouping these imports together with the rest of the lifecycle-related imports for easier discovery.
49-52
: Configuration for retry logic is well defined.
This new optionalretries
property inTaskExecutorOptions
clarifies retry behavior. Ensure documentation is updated to reflect theenabledInDev
property usage.
138-198
: Design feedback on local functionexecuteTask
Grouping run initialization, error handling, and final result creation in a single async function simplifies the readability. However, it's quite large—consider extracting repetitive code blocks (e.g., calls to#callOnCompleteFunctions
) into smaller helper functions, if needed, for maintainability.
355-392
:#callRun
function
The usage of an abort signal viaPromise.race
elegantly stops long-running tasks. Ensure side effects (like partial DB writes) are compensated for when aborted.
789-851
:#callOnStartFunctions
Great approach to measure hook performance withrunTimelineMetrics
. Keep these metrics to help diagnose slow start hooks.packages/trigger-sdk/src/v3/shared.ts (3)
1-1
: Removed or replaced imports
UsingSpanKind
from@opentelemetry/api
might be helpful if instrumentation is needed. Confirm it's still required.
20-24
: Export blocks
Explicit re-export of types from@trigger.dev/core/v3
clarifies the public API surface. Keep these well-documented.
354-358
: Unified hooks registration
AnotherregisterTaskLifecycleHooks
call. Maintaining parallel calls in bothcreateTask
andcreateSchemaTask
is consistent, though watch for any duplication.packages/trigger-sdk/src/v3/hooks.ts (1)
16-28
: Type re-exports
Re-exporting these hook types fosters easier consumption for end users. Keep an eye on version-sync to avoid mismatches.packages/core/src/v3/lifecycleHooks/types.ts (1)
4-4
: Refine 'void' in union return types
Static analysis flags the usage ofundefined | void
as potentially confusing. Consider omittingvoid
to reduce ambiguity, for example merging it into a singleundefined
type.- ) => TInitOutput | undefined | void | Promise<TInitOutput | undefined | void>; + ) => TInitOutput | undefined | Promise<TInitOutput | undefined>;Also applies to: 15-15, 32-32, 69-69, 87-87, 105-105, 128-128, 165-165, 229-229
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.cursor/rules/executing-commands.mdc
(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx
(3 hunks)internal-packages/run-engine/src/engine/errors.ts
(1 hunks)packages/cli-v3/src/build/bundle.ts
(5 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts
(1 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts
(8 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts
(1 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts
(7 hunks)packages/core/src/v3/config.ts
(2 hunks)packages/core/src/v3/errors.ts
(1 hunks)packages/core/src/v3/index.ts
(1 hunks)packages/core/src/v3/lifecycle-hooks-api.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/index.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/manager.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/types.ts
(1 hunks)packages/core/src/v3/locals-api.ts
(1 hunks)packages/core/src/v3/locals/index.ts
(1 hunks)packages/core/src/v3/locals/manager.ts
(1 hunks)packages/core/src/v3/locals/types.ts
(1 hunks)packages/core/src/v3/runtime/managedRuntimeManager.ts
(4 hunks)packages/core/src/v3/schemas/build.ts
(2 hunks)packages/core/src/v3/schemas/common.ts
(1 hunks)packages/core/src/v3/semanticInternalAttributes.ts
(1 hunks)packages/core/src/v3/tracer.ts
(1 hunks)packages/core/src/v3/types/tasks.ts
(6 hunks)packages/core/src/v3/utils/globals.ts
(2 hunks)packages/core/src/v3/workers/index.ts
(1 hunks)packages/core/src/v3/workers/taskExecutor.ts
(11 hunks)packages/core/test/taskExecutor.test.ts
(1 hunks)packages/trigger-sdk/src/v3/hooks.ts
(1 hunks)packages/trigger-sdk/src/v3/index.ts
(1 hunks)packages/trigger-sdk/src/v3/locals.ts
(1 hunks)packages/trigger-sdk/src/v3/shared.ts
(6 hunks)packages/trigger-sdk/src/v3/tasks.ts
(3 hunks)references/hello-world/src/db.ts
(1 hunks)references/hello-world/src/trigger/example.ts
(2 hunks)references/hello-world/src/trigger/init.ts
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (12)
packages/core/src/v3/lifecycleHooks/index.ts (2)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
NoopLifecycleHooksManager
(399-599)packages/core/src/v3/lifecycleHooks/types.ts (14)
LifecycleHooksManager
(233-310)RegisterHookFunctionParams
(169-172)AnyOnInitHookFunction
(17-17)RegisteredHookFunction
(174-178)AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)AnyOnCleanupHookFunction
(231-231)TaskWait
(36-53)
packages/core/src/v3/locals/index.ts (3)
packages/core/src/v3/locals/manager.ts (1)
NoopLocalsManager
(3-16)packages/core/src/v3/locals/types.ts (2)
LocalsManager
(10-14)LocalsKey
(5-8)packages/core/src/v3/utils/globals.ts (3)
registerGlobal
(19-35)unregisterGlobal
(43-49)getGlobal
(37-41)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (14)
LifecycleHooksManager
(233-310)RegisteredHookFunction
(174-178)AnyOnInitHookFunction
(17-17)AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)AnyOnCleanupHookFunction
(231-231)TaskWait
(36-53)RegisterHookFunctionParams
(169-172)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
packages/core/src/v3/lifecycleHooks/index.ts (1)
LifecycleHooksAPI
(24-266)
packages/core/test/taskExecutor.test.ts (5)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
lifecycleHooks
(5-5)packages/core/src/v3/lifecycleHooks/manager.ts (1)
StandardLifecycleHooksManager
(18-397)packages/core/src/v3/workers/taskExecutor.ts (12)
payload
(302-353)payload
(355-393)payload
(535-629)payload
(631-702)payload
(704-775)payload
(777-787)payload
(789-851)payload
(853-861)payload
(863-931)payload
(1119-1190)execution
(952-1067)execution
(1192-1208)packages/core/src/v3/types/tasks.ts (2)
RunFnParams
(86-93)TaskMetadataWithFunctions
(871-887)packages/core/src/v3/schemas/common.ts (4)
TaskRunErrorCodes
(195-195)TaskRunErrorCodes
(196-196)TaskRunExecution
(302-316)TaskRunExecution
(318-318)
packages/trigger-sdk/src/v3/tasks.ts (1)
packages/trigger-sdk/src/v3/hooks.ts (2)
onHandleError
(104-109)onCatchError
(113-121)
packages/core/src/v3/config.ts (3)
packages/core/src/v3/lifecycle-hooks-api.ts (4)
AnyOnInitHookFunction
(9-9)AnyOnSuccessHookFunction
(18-18)AnyOnFailureHookFunction
(16-16)AnyOnStartHookFunction
(14-14)packages/core/src/v3/lifecycleHooks/types.ts (4)
AnyOnInitHookFunction
(17-17)AnyOnSuccessHookFunction
(130-130)AnyOnFailureHookFunction
(107-107)AnyOnStartHookFunction
(34-34)packages/trigger-sdk/src/v3/hooks.ts (3)
AnyOnSuccessHookFunction
(21-21)AnyOnFailureHookFunction
(20-20)AnyOnStartHookFunction
(17-17)
packages/core/src/v3/workers/taskExecutor.ts (4)
packages/core/src/v3/schemas/schemas.ts (2)
RetryOptions
(99-126)RetryOptions
(128-128)packages/core/src/v3/semanticInternalAttributes.ts (1)
SemanticInternalAttributes
(1-60)packages/core/src/v3/errors.ts (1)
InternalError
(18-44)packages/core/src/v3/types/tasks.ts (1)
HandleErrorModificationOptions
(128-134)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycleHooks/types.ts (8)
AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)packages/core/src/v3/lifecycle-hooks-api.ts (9)
AnyOnStartHookFunction
(14-14)lifecycleHooks
(5-5)AnyOnFailureHookFunction
(16-16)AnyOnSuccessHookFunction
(18-18)AnyOnCompleteHookFunction
(20-20)AnyOnWaitHookFunction
(22-22)AnyOnResumeHookFunction
(24-24)AnyOnCatchErrorHookFunction
(26-26)AnyOnMiddlewareHookFunction
(29-29)
packages/cli-v3/src/entryPoints/dev-run-worker.ts (6)
packages/core/src/v3/workers/index.ts (3)
StandardLocalsManager
(24-24)StandardRunTimelineMetricsManager
(21-21)StandardLifecycleHooksManager
(23-23)packages/core/src/v3/locals/manager.ts (1)
StandardLocalsManager
(18-36)packages/core/src/v3/runTimelineMetrics/runTimelineMetricsManager.ts (1)
StandardRunTimelineMetricsManager
(5-47)packages/core/src/v3/lifecycleHooks/manager.ts (1)
StandardLifecycleHooksManager
(18-397)packages/core/src/v3/lifecycle-hooks-api.ts (6)
lifecycleHooks
(5-5)AnyOnInitHookFunction
(9-9)AnyOnStartHookFunction
(14-14)AnyOnSuccessHookFunction
(18-18)AnyOnFailureHookFunction
(16-16)AnyOnCatchErrorHookFunction
(26-26)packages/core/src/v3/lifecycleHooks/types.ts (5)
AnyOnInitHookFunction
(17-17)AnyOnStartHookFunction
(34-34)AnyOnSuccessHookFunction
(130-130)AnyOnFailureHookFunction
(107-107)AnyOnCatchErrorHookFunction
(200-200)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (10)
OnInitHookFunction
(13-15)OnCleanupHookFunction
(227-229)OnCatchErrorHookFunction
(195-198)OnResumeHookFunction
(85-87)OnWaitHookFunction
(67-69)OnCompleteHookFunction
(159-165)OnMiddlewareHookFunction
(210-212)OnStartHookFunction
(30-32)OnSuccessHookFunction
(122-128)OnFailureHookFunction
(103-105)
packages/core/src/v3/lifecycleHooks/types.ts (4)
packages/core/src/v3/lifecycle-hooks-api.ts (27)
TaskInitHookParams
(11-11)OnInitHookFunction
(8-8)AnyOnInitHookFunction
(9-9)TaskStartHookParams
(12-12)OnStartHookFunction
(13-13)AnyOnStartHookFunction
(14-14)TaskWait
(34-34)TaskWaitHookParams
(21-21)AnyOnWaitHookFunction
(22-22)TaskResumeHookParams
(23-23)AnyOnResumeHookFunction
(24-24)TaskFailureHookParams
(15-15)AnyOnFailureHookFunction
(16-16)TaskSuccessHookParams
(17-17)AnyOnSuccessHookFunction
(18-18)TaskCompleteResult
(27-27)TaskCompleteHookParams
(19-19)AnyOnCompleteHookFunction
(20-20)RegisteredHookFunction
(10-10)TaskCatchErrorHookParams
(25-25)AnyOnCatchErrorHookFunction
(26-26)TaskMiddlewareHookParams
(28-28)OnMiddlewareHookFunction
(30-30)AnyOnMiddlewareHookFunction
(29-29)TaskCleanupHookParams
(33-33)OnCleanupHookFunction
(31-31)AnyOnCleanupHookFunction
(32-32)packages/core/src/v3/types/tasks.ts (1)
HandleErrorResult
(136-140)packages/core/src/v3/lifecycleHooks/index.ts (1)
LifecycleHooksManager
(263-265)packages/core/src/v3/workers/taskExecutor.ts (2)
wait
(395-463)wait
(465-533)
🪛 Biome (1.9.4)
packages/core/src/v3/lifecycleHooks/index.ts
[error] 30-30: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 31-31: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 34-34: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/core/src/v3/locals/index.ts
[error] 15-15: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 16-16: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 19-19: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 39-39: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/core/test/taskExecutor.test.ts
[error] 914-915: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
packages/core/src/v3/lifecycleHooks/types.ts
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 15-15: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 32-32: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 69-69: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 87-87: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 105-105: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 128-128: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 165-165: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 229-229: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (148)
.cursor/rules/executing-commands.mdc (2)
16-16
: Documentation update looks good!The addition of the
--run
flag to the test command aligns with the current best practices for executing tests in the monorepo.
23-23
: Command update is consistent!The
--run
flag has been correctly added here as well, maintaining consistency across all test command examples.apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
49-51
: Good enhancement for icon flexibilityAdding support for direct rendering of icons from the tablerIcons set improves the component's flexibility.
packages/core/src/v3/semanticInternalAttributes.ts (1)
36-36
: New attribute addition looks goodThe
COLLAPSED
attribute follows the established naming convention and integrates well with the existing attributes.packages/trigger-sdk/src/v3/locals.ts (1)
1-5
: Clean re-export implementationThis file effectively re-exports the local variable management API from the core package, making it accessible through the SDK. Good practice for API design.
packages/trigger-sdk/src/v3/index.ts (1)
15-15
: LGTM: Local variables support added through exportThe addition of this export statement appropriately exposes the functionality from the locals module, enabling access to local variable management features through the SDK.
packages/core/src/v3/index.ts (1)
18-19
: LGTM: New lifecycle hooks and locals API exportsGood addition of exports for the lifecycle hooks and locals APIs. This makes these new features accessible to consumers of the core package.
packages/core/src/v3/errors.ts (1)
308-308
: LGTM: Added middleware error to retry logicThe addition of "TASK_MIDDLEWARE_ERROR" as a retryable error code is appropriate. This ensures middleware-related errors can be properly handled through the retry mechanism.
packages/core/src/v3/workers/index.ts (1)
23-24
: LGTM: Exported lifecycle and locals managersThese exports make the StandardLifecycleHooksManager and StandardLocalsManager available from the workers package, allowing for consistent management of lifecycle hooks and local variables across the worker context.
packages/core/src/v3/schemas/common.ts (1)
168-168
: New error code added for task middleware.The new
"TASK_MIDDLEWARE_ERROR"
error code has been added to the TaskRunInternalError schema. This is consistent with the PR objective of introducing new lifecycle hooks and middleware support for tasks.references/hello-world/src/trigger/init.ts (2)
1-7
: Middleware implementation looks good.The task middleware implementation follows a proper middleware pattern with the use of
next()
to continue execution flow. The logging statement is appropriate for demonstration purposes.
9-41
: Commented out lifecycle hooks provide useful reference.These commented-out lifecycle hook implementations provide good examples of the different hook types available in the new API. Having them commented out is appropriate for a reference implementation.
Note that if you intend to activate these hooks later, you might want to fix the copy-paste error in the logger messages - several hooks use the same message "Hello, world from the start" or "Hello, world from the success" despite being different hook types.
references/hello-world/src/trigger/example.ts (2)
7-8
: Added logging provides better visibility into the task execution.This logging statement helps to trace the task lifecycle and adds consistency with the new hooks pattern.
151-194
: Well-structured implementation of lifecycle hooks.The
hooksTask
implementation thoroughly demonstrates all available lifecycle hooks with clear logging statements. This serves as an excellent reference example for users implementing the new hooks API.The various hooks (
init
,onWait
,onResume
,onStart
,onSuccess
,onFailure
,onComplete
,handleError
,catchError
, andcleanup
) are implemented in a logical order that follows the task lifecycle.internal-packages/run-engine/src/engine/errors.ts (1)
16-18
: Error handling updated to support middleware errors.The error handling logic now properly categorizes middleware errors along with input and output errors as "COMPLETED_WITH_ERRORS". This provides a consistent experience when middleware encounters issues.
packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
144-144
: Feature addition: initEntryPoint property added to manifestThe addition of
initEntryPoint
property to the manifest object ensures that initialization entry point information is properly passed to the worker. This is consistent with the architectural pattern used throughout the codebase for other entry points.packages/core/src/v3/utils/globals.ts (2)
3-4
: New lifecycle and locals management features addedThe imports for the new lifecycle hooks and locals management functionality have been properly added.
67-68
: Global API extended with lifecycle hooks and locals managementThe TriggerDotDevGlobalAPI type has been appropriately extended with two new optional properties for lifecycle hooks and locals management. This follows the established pattern for global API registration.
packages/core/src/v3/locals/index.ts (2)
1-8
: New LocalsAPI implementation with proper importsThe file correctly imports necessary dependencies and establishes a NOOP manager as a fallback.
42-44
: Private helper method for manager retrievalThe private helper method correctly retrieves the registered manager or falls back to the NOOP manager.
packages/cli-v3/src/entryPoints/managed-index-worker.ts (1)
144-144
: Feature addition: initEntryPoint property added to manifestThe addition of
initEntryPoint
property to the manifest object ensures that initialization entry point information is properly passed to the worker. This is consistent with the change made in the dev-index-worker.ts file and maintains parity between development and managed environments.packages/core/src/v3/lifecycleHooks/index.ts (1)
247-266
: Global wait/resume hook approach looks consistent.The implementation of
callOnWaitHookListeners
,callOnResumeHookListeners
,registerOnWaitHookListener
, andregisterOnResumeHookListener
is well-structured and aligns with the rest of the lifecycle logic. No issues flagged here.packages/core/src/v3/tracer.ts (1)
171-186
: Confirm necessity of creating an additional partial span.Lines 171–186 create a “partialSpan” and end it immediately. While this can be useful for more granular logging or instrumentation, it does incur overhead. If the extra span is not strictly needed, removing or making it conditional could optimize performance. Otherwise, this approach appears logically sound and correct relative to the updated
parentContext
.Do you want to verify usage of this “partialSpan” across the codebase to ensure it’s fully utilized? I can generate a script to search for references and confirm its purpose.
packages/core/src/v3/schemas/build.ts (1)
41-41
: Validate optionalinitEntryPoint
usage.Lines 41 and 89 add an optional
initEntryPoint
property to the schema. This is consistent with your existing optional entries. Ensure that any code referencinginitEntryPoint
handles its absence gracefully (e.g., fallback to default or skip initialization). Otherwise, this addition looks good.Also applies to: 89-89
packages/core/src/v3/locals/types.ts (2)
1-8
: Well-designed type-safe local storage mechanismThe implementation uses TypeScript's type branding pattern with a unique symbol to create a type-safe key system, preventing accidental access with incorrect types. The
LocalsKey<T>
type effectively combines the brand with useful metadata (id and type symbol).
10-14
: Clean interface design for locals managementThe
LocalsManager
interface provides a well-structured API with the necessary operations for managing local variables. The generic type parameters ensure type safety throughout the system.packages/core/src/v3/locals/manager.ts (2)
3-16
: Appropriate no-op implementationThe
NoopLocalsManager
provides a useful null object pattern implementation that satisfies the interface without storing any data. This is helpful for testing or environments where locals storage isn't needed.
18-36
: Well-implemented locals storageThe
StandardLocalsManager
correctly implements theLocalsManager
interface with appropriate storage. UsingSymbol.for(id)
ensures consistent symbol creation across multiple calls with the same id, which is important for reliable key lookup.references/hello-world/src/db.ts (3)
4-14
: Good pattern for local database managementThe implementation creates a typed local storage key for database operations and provides helper functions that encapsulate the locals API. Using
getOrThrow
ensures you'll get proper error messages if the database isn't initialized.
16-34
: Well-structured database middlewareThe middleware properly manages the database lifecycle around task execution:
- Initializes the database connection
- Logs before and after task execution
- Properly cleans up by disconnecting
This is a good pattern for resource management in serverless environments.
36-48
: Proper handling of task lifecycle eventsThe
onWait
andonResume
handlers appropriately manage database connections during task pauses. Disconnecting during waiting periods can help reduce resource usage and costs, while reconnecting ensures the database is available when needed.packages/trigger-sdk/src/v3/tasks.ts (3)
1-11
: Comprehensive lifecycle hook importsThe imports bring in a complete set of lifecycle hooks that cover the entire task execution flow, from start to completion, including error handling and wait/resume states.
60-60
: Proper type exportsExporting all types from the hooks module makes them available to consumers of the tasks module, which improves developer experience by reducing the need to import from multiple modules.
92-101
: Good lifecycle hook integration with clear deprecationThe lifecycle hooks are well-integrated into the tasks object, making them easily accessible. The deprecation notice for
handleError
is clear and directs users to the preferredcatchError
method.From the context, I can see that
onHandleError
is an alias foronCatchError
to maintain backward compatibility while encouraging users to migrate to the new API.packages/core/src/v3/runtime/managedRuntimeManager.ts (3)
48-51
: Consider handling rejections from wait-hook listeners.Currently, if any listener rejects, the
await
statement will reject the entire function call. Confirm whether this behavior is desirable, and if not, consider adding individual error-handling logic or a fallback to ensure a single failed hook does not block the rest.Do you want me to check other parts of this codebase to see if there's a universal error handling pattern for rejection from hook listeners?
84-88
: Batch wait logic is well structured.The call to
callOnWaitHookListeners
for the batch scenario looks consistent with the single-task approach. However, verify that any potential errors from one hook in a batch do not block the entire batch processing unless it’s your intended behavior.
117-127
: Conditional wait logic clarity.Splitting wait logic based on
finishDate
for 'duration' vs. 'token' is clear and maintainable. Ensure you have adequate tests for each branch to confirm correct behavior when the hook listeners reject or throw exceptions.packages/core/src/v3/lifecycleHooks/manager.ts (3)
1-16
: Imported types and interfaces are consistent.The import statements properly reference hook function types from “./types.js.” This organization is transparent for future maintainability and helps with tree-shaking. No apparent issues.
18-64
: Parallel execution of wait/resume hook listeners.Here,
callOnWaitHookListeners
andcallOnResumeHookListeners
effectively run all hooks in parallel. Ensure that you want a single rejected promise to short-circuit the rest. If partial or tolerant execution is desired, handle each promise individually.
399-599
: Noop lifecycle manager is straightforward.All methods do nothing, making it a suitable fallback for scenarios that do not require lifecycle management. Confirm that calling these methods inadvertently will not mislead the code flow by silently discarding callbacks.
packages/core/src/v3/lifecycle-hooks-api.ts (1)
7-35
: Clean type re-exports.Re-exporting all hook-related types from a single file is a best practice for discoverability and reduces import clutter. This approach is well structured, and no issues are found here.
packages/cli-v3/src/build/bundle.ts (5)
50-50
: Add initEntryPoint property for clarity
IntroducinginitEntryPoint
in theBundleResult
helps track the initialization file distinctly.
233-233
: Initialization of initEntryPoint variable
DeclaringinitEntryPoint
provides a convenient placeholder that will be set when aninit.ts
is discovered.
299-299
: Include initEntryPoint in the bundle result
ReturninginitEntryPoint
in the build result is consistent and ensures consumers have the path to the initialization file.
377-377
: Extend build manifest with initEntryPoint
PropagatinginitEntryPoint
into the build manifest ensures any deployment or runtime environment can locate and use the initialization file seamlessly.
271-272
:❓ Verification inconclusive
Verify multiple init.ts files scenario
When assigninginitEntryPoint
, make sure there are no collisions if multipleinit.ts
files exist. Confirm only one is expected or handle multiple gracefully.Would you like to scan for additional
init.ts
matches to ensure you only pick up the first occurrence?
🏁 Script executed:
#!/bin/bash rg -A2 $'init.ts'Length of output: 1257
Attention: Confirm Handling of Multiple
init.ts
FilesThe search results reveal that
init.ts
references exist in several parts of the repository (e.g., in the core schemas and documentation). Inpackages/cli-v3/src/build/bundle.ts
, the bundler checks if an entry point is aninit.ts
but immediately assigns that file toinitEntryPoint
without guarding against multiple occurrences.
- Verify that the bundler’s current logic is intentional—i.e., that it picks the first (or last) encountered
init.ts
file—and that any collision is either impossible by design or correctly managed.- If only one
init.ts
is truly expected per trigger directory, consider adding a guard to warn or error when a second instance is found.- If multiple instances are valid, ensure the downstream logic can gracefully handle them without unintended side effects.
Please review and adjust the logic accordingly if necessary.
packages/core/test/taskExecutor.test.ts (19)
1-24
: Test suite initialization
Imports and setup appear standard. Using Vitest is a good choice; the initial lifecycle hooks re-instantiation ensures consistent test isolation.
25-66
: onComplete hook test
Thorough check ensures final output is correct, including the aggregatedinit
data. Looks well-structured for success scenarios.
68-172
: onSuccess hooks ordering
This test effectively confirms multiple success hooks (global + task) are called in registration order, capturing essential coverage.
174-268
: onStart hooks ordering
Verifies that hooks run in sequence before therun
function. Good approach ensuring the correct payload and initialization data.
270-375
: onFailure hooks
Ensures the error is propagated to all hooks in correct order. This covers essential error handling flows.
377-484
: onComplete sequence for successful run
Demonstrates final completion ordering with correct data. Confirms that completion hooks see accurate success results.
486-574
: onComplete when task fails
Validates hooking into the post-run phase even when there’s an error. This ensures reliable cleanup logic.
576-674
: catchError hooks in order
Tests that the error cascades until a handler returns a modification. Correctly stops after an effective catch.
675-730
: skipRetrying logic
This test ensures that a catch error hook can skip further retries, adjusting the final reported error if necessary.
732-785
: Custom retry timing
Checks for specific scheduling of re-execution. The test thoroughly validatesretryAt
usage.
787-901
: Middleware ordering
Great test ensuring global and task-specific middlewares wrap the run step in the intended layered sequence.
959-1035
: Handling errors from init hooks
Verifies that a failing init stops the run and triggers onFailure. Good coverage for early errors.
1037-1113
: Handling errors from task init hooks
Ensures initial global hooks succeed, but failing task-specific hooks block the run function. Nicely tested.
1115-1204
: Propagation of start hook errors
Correctly tests partial initialization success, with start hook failure preventingrun
.
1205-1328
: Cleanup hooks after success
Verifies the order in which cleanup runs and ensures it occurs before middleware’s final exit.
1330-1416
: Cleanup hooks on error
Checks that all cleanup hooks still execute even though the task fails. This ensures robust resource cleanup.
1418-1503
: Handling max duration abort
Simulates manual abort, verifying partial steps and correct error injection. Good coverage of timeouts.
1505-1623
: onWait and onResume hooks
Ensures pause/resume logic is validated. The hooking for wait states is precisely tested, including data re-check.
1626-1716
: executeTask helper
Functionally orchestrates the entire run within tests. Clear to read and maintain.packages/core/src/v3/locals-api.ts (1)
1-30
: Comprehensive locals API
This new module provides a straightforward API for creating, retrieving, and setting local variables. The typedLocalsKey
ensures type safety. Overall, it finely encapsulates local state management.packages/core/src/v3/config.ts (3)
5-11
: Imports are consistent with the new lifecycle hooks.
These imported types align seamlessly with the updated lifecycle architecture. Please ensure all relevant call sites are updated accordingly.
12-12
: New import ofLogLevel
.
This appears to be a straightforward addition. No concerns.
219-222
:❓ Verification inconclusive
Deprecation of
init
in favor oftasks.init
.
Good callout. Confirm that any references toinit
in your project are replaced or accounted for in tests.Run the following script to locate potential references:
🏁 Script executed:
#!/bin/bash ast-grep --pattern $'config.init'Length of output: 2291
Action Required: Verify/Update Deprecated
init
UsageThe
init
property inpackages/core/src/v3/config.ts
is deprecated in favor oftasks.init
. However, I located references toconfig.init
in the following files:
packages/cli-v3/src/entryPoints/managed-run-worker.ts
(lines 198 & 201)packages/cli-v3/src/entryPoints/dev-run-worker.ts
(lines 188 & 191)Please review these occurrences to determine if they should be updated to use
tasks.init
or if they are being maintained for backward compatibility. In either case, ensure that these paths are covered by tests.packages/cli-v3/src/entryPoints/managed-run-worker.ts (14)
4-8
: Addition of lifecycle hook function imports.
These newly imported types reflect the expanded hook coverage. Looks good.
9-9
: Refined import ofapiClientManager
.
No issues.
11-11
: Import ofExecutorToWorkerMessageCatalog
.
This aligns with the message passing schema. No concerns.
13-14
: Imports for lifecycle and locals API.
Both references appear consistent with the new architecture.
18-20
: Added references torunMetadata
,runtime
, andrunTimelineMetrics
.
These imports integrate the new runtime monitoring approach. Looks correct.
24-24
: Import ofTriggerConfig
.
No issues.
26-27
: Import ofWorkerManifest
andWorkerToExecutorMessageCatalog
.
Helps parse worker configuration and handle inter-process messages.
105-106
: Global locals manager registration.
This correctly sets up a shared, standard locals manager.
107-110
: Global lifecycle hooks manager registration.
Centralizing hooks instandardLifecycleHooksManager
is consistent with the new design.
198-203
: Registering globalinit
hook.
config.init
is now hooked globally. Deprecation is clearly indicated in config but used conditionally here. Confirm usage is tested.
205-210
: Registering globalonStart
hook.
Ensures consistent pre-run logic across all tasks. All good.
212-217
: Registering globalonSuccess
hook.
Properly extends the success path with a global callback.
219-224
: Registering globalonFailure
hook.
Ensures universal handling of run failures. No issues.
226-231
: Registering globalhandleError
as a catch error hook.
Allows for a single place to handle thrown errors. Implementation looks good.packages/cli-v3/src/entryPoints/dev-run-worker.ts (12)
4-8
: New lifecycle hook function imports.
These new imports align with the hook-based design introduced incore/v3
.
13-14
: Added lifecycle and locals API imports.
They’re essential for the updated hooks and local data registration.
17-17
: Import ofresourceCatalog
.
Complements the new approach to track tasks. No issues.
20-20
: Bringing inrunTimelineMetrics
.
Supports advanced timeline tracking for tasks.
24-24
: Import ofTriggerConfig
.
No concerns.
26-27
: Imports for worker manifest and message catalogs.
Ensures correct dev-run message handling.
39-40
: AddedStandardLifecycleHooksManager
andStandardLocalsManager
.
Further consolidates the new lifecycle and local usage approach in dev-mode.
43-43
: AddedStandardRunTimelineMetricsManager
.
Allows dev-run worker to track run metrics consistently.
269-269
: Extracted bootstrap results.
Unpacks the new lifecycle logic objects frombootstrap()
. Implementation looks fine.
309-320
: Optional loading ofinitEntryPoint
.
Including this logic supports extra user-defined initialization. The try-catch with error logging is a good safeguard.
385-385
: Configuring theretries
option inTaskExecutor
.
Usingconfig.retries
ensures the dev-run consistently applies global retry settings.
403-403
: Direct call toexecutor.execute
.
Cleaner approach for handling the maxDuration signal. Properly leverages the new design pattern.packages/core/src/v3/workers/taskExecutor.ts (20)
5-19
: Imports for error and hook management look appropriate.
They centralize error parsing and lifecycle handling, which aids in structured error reporting and consistent hook usage.
34-35
: Dependency consolidation observation:
UsingtryCatch
from a central utility is a good approach to standardize error handling across the codebase.
59-65
: Initialization of_retries
property
Storing the retry configuration paves the way for adjustable retry logic. This is helpful for local vs. production runs.
73-73
: Constructor assignment for_retries
Assigningthis._retries
in the constructor is consistent with other fields. Make sure it's tested for edge cases like undefined values.
112-129
: Payload parse block
UsingtryCatch
around payload parsing is a solid approach. It properly captures and records exceptions and aligns with your structured error handling.
130-136
: Registration of wait/resume hooks
Hooking intoonWait
andonResume
here promotes better event-driven control. Good design choice to integrate these at the earliest stage of the task.
200-261
: Output serialization and completion logic
The sequence of stringifying output, exporting it, and updating span attributes is consistent. Nicely ensures trace data captures final output details. If called frequently in short intervals, watch for any performance overhead from repeated exports.
303-353
:#executeTaskWithMiddlewareHooks
This layered approach (reduceRight + next) is effective for stackable middleware. The race condition between the runner andexecuteError
is handled well. Just ensure each hook guards against unexpected side effects in shared state.
395-463
:#callOnWaitFunctions
Running each global hook in a separate try-catch withtryCatch
is correct. If any hook fails, we abort the entire flow, which is consistent.
465-533
:#callOnResumeFunctions
Same pattern asonWait
. Ensuring symmetrical logic fosters clear reentry points for paused or scheduled tasks.
535-629
:#callInitFunctions
Merging multiple global hook results into a single object is practical. Watch for conflicts where multiple hooks return the same property name.
631-702
:#callOnSuccessFunctions
Sequentially running success hooks is standard. No concerns here except verifying it does not mask upstream errors.
704-775
:#callOnFailureFunctions
Registers meaningful spans for each global and task-level failure hook, enabling in-depth debugging. Looks good.
777-787
:#parsePayload
RaisingTaskPayloadParsedError
on parse failure is consistent with your error model. Clear separation of concerns.
853-861
:#cleanupAndWaitUntil
Running cleanup hooks before forcing the wait is well ordered. This ensures leftover resources are released promptly.
863-931
:#callCleanupFunctions
Implementation matches the onStart/onWait pattern. This is consistent with your overall design, no issues spotted.
952-1067
:#handleError
Logic for skipping retries in certain error conditions is thorough. The fallback to default retry settings is clearly documented. Good separation of local vs. global hooks.
1069-1117
:#processHandleErrorResult
Consolidating skip/retry/noop statuses is neatly handled. This makes it easy to maintain or add new statuses.
1119-1190
:#callOnCompleteFunctions
Invoking completion hooks after success/failure ensures finalization logic runs consistently. Good design.
1192-1209
:#internalErrorResult
Wrapping internal errors under a known shape helps unify error responses. This is crucial for debugging.packages/trigger-sdk/src/v3/shared.ts (7)
11-16
: Utility imports
flattenIdempotencyKey
,getEnvVar
, andlifecycleHooks
usage shows a cohesive approach for user environment, keys, and hooking. No immediate issues.
49-52
: Reintroduced or updated batch references
These lines appear to re-export batch-related types. Confirm they align with your new batch logic introduced in the codebase.
62-63
: Extended types
BatchTriggerAndWaitOptions
andBatchTriggerTaskV2RequestBody
are reintroduced or extended. Double-check internal references for correctness.
131-135
: Refined generics increateTask()
The typed usage ofTIdentifier
,TInput
,TOutput
,TInitOutput
helps maintain strict type safety. Good improvement.
198-199
:registerTaskLifecycleHooks
usage
CallingregisterTaskLifecycleHooks
ensures consistent hooking for tasks. Great integration point.
325-326
: Refined generics increateSchemaTask()
Similar tocreateTask()
, providing typed usage for schema tasks helps ensure typed payload parsing.
1564-1637
: New functionregisterTaskLifecycleHooks
This function systematically registers all possible lifecycle hooks (init, start, failure, etc.). It is well-structured and future-proof for additional hooks. Ensure that each branch has test coverage to avoid silent no-ops if a parameter is misconfigured.packages/trigger-sdk/src/v3/hooks.ts (10)
1-14
: Imports from@trigger.dev/core/v3
All relevant hook types are consolidated from core. This is a neat separation of concerns, ensuring the SDK reuses the core hooking logic.
30-40
:onStart
Provides both named and unnamed overloads. This dual signature approach is helpful for hooking with or without IDs.
42-52
:onFailure
Uses the same pattern asonStart
. Consistency helps devs quickly grasp usage.
54-64
:onSuccess
The same design pattern for lifecycle hooks. LGTM.
66-76
:onComplete
Completes the standard set of global event hooks. The fallback naming logic (fn.name) is convenient.
78-85
:onWait
Same approach as other hooks—straightforward. No concerns.
87-97
:onResume
Parallel structure toonWait
. Easy to follow.
99-110
:onHandleError
(deprecated)
Deprecation strategy is well handled here. Delegating toonCatchError
is a clean approach that avoids code duplication.
111-121
:onCatchError
Registration inlifecycleHooks.registerGlobalCatchErrorHook
clarifies global error handling. Ensure usage is well documented for advanced error scenarios.
123-133
:middleware
Provides layering for hooking into the execution pipeline. This is highly extensible.packages/core/src/v3/types/tasks.ts (15)
3-15
: New lifecycle hooks imports
All referenced imports appear consistent with usage in this file. This integration of hook types looks well structured and aligns with the broader lifecycle enhancements.
26-26
: No issues with new import
This import statement forQueueOptions
is straightforward and doesn't introduce any obvious conflicts.
29-29
: Prettify import
ImportingPrettify
as a helper type is a neat approach to simplifying complex TypeScript type definitions.
110-110
: Optional init property
Addinginit?: InitOutput
toStartFnParams
seamlessly integrates the initialization output into the start parameters.
142-142
: Revising 'init' to a structured type
UsingRecord<string, unknown>
clarifies what shape theinit
data can take, improving maintainability and type safety.
275-275
: Deprecation note for init
Documenting the move frominit
to newer mechanisms ensures developers know to favor the more modern lifecycle or middleware approach.
278-278
: Switching to OnInitHookFunction
Replacing a generic function callback withOnInitHookFunction<TPayload, TInitOutput>
enhances type clarity for the init phase.
282-285
: Deprecation note for cleanup
By suggesting middleware for cleanup operations, you're helping isolate cross-cutting concerns. This is a simpler, more compositional approach.
289-293
: Deprecating handleError
Recommending thecatchError
hook clarifies intent and standardizes error-handling terminology across the codebase.
297-297
: New catchError property
A dedicatedcatchError
property distinctly separates error handling from the normal run flow, improving code readability.
299-301
: Introducing onResume, onWait, onComplete
These hooks are great additions for controlling lifecycle transitions and final states, providing fine-grained flexibility.
320-320
: middleware property
Defining a separatemiddleware
function is an excellent way to address cross-cutting concerns without cluttering the primary run logic.
325-325
: onStart hook
Allowing custom logic to run at the earliest stage unlocks more sophisticated control over initialization or startup actions.
330-330
: onSuccess hook
Provides a clean place to perform follow-up actions and ensures the mainrun
code path stays focused on business logic.
335-335
: onFailure hook
Centralizing final failure handling at the lifecycle level avoids scattering error logic and helps maintain consistent cleanup strategies.packages/core/src/v3/lifecycleHooks/types.ts (1)
1-311
: Comprehensive lifecycle hooks introduction
This file provides a robust, well-typed framework for managing every stage of a task's lifecycle, from initialization and middleware to wait/resume and final cleanup. Each hook type cleanly separates concerns, and references toTaskRunContext
and typed parameters help ensure correctness. Overall, this design is both flexible and maintainable.🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 15-15: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 32-32: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 69-69: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 87-87: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 105-105: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 128-128: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 165-165: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 229-229: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
1c36b9c
to
268c9f6
Compare
console.log("spanName", spanName, name); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/cli-v3/src/build/bundle.ts
Outdated
// Check if the entry point is an init.ts file at the root of a trigger directory | ||
function isInitEntryPoint(entryPoint: string): boolean { | ||
const normalizedEntryPoint = entryPoint.replace(/\\/g, "/"); // Normalize path separators | ||
const initFileName = "init.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we support js
jsx
and tsx
files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/v3/lifecycleHooks/index.ts (1)
45-245
: 🛠️ Refactor suggestionReduce repetitive hook registration code.
The implementation contains significant duplication with nearly identical patterns for different hook types. This could make maintenance challenging if behavior needs to change.
Consider introducing a generic helper function to reduce repetition:
private createHookMethods<T>(hookType: string): { registerGlobal: (hook: RegisterHookFunctionParams<T>) => void; registerTask: (taskId: string, hook: RegisterHookFunctionParams<T>) => void; getTask: (taskId: string) => T | undefined; getGlobal: () => RegisteredHookFunction<T>[]; } { return { registerGlobal: (hook: RegisterHookFunctionParams<T>) => { this.#getManager()[`registerGlobal${hookType}Hook`](hook); }, registerTask: (taskId: string, hook: RegisterHookFunctionParams<T>) => { this.#getManager()[`registerTask${hookType}Hook`](taskId, hook); }, getTask: (taskId: string) => { return this.#getManager()[`getTask${hookType}Hook`](taskId); }, getGlobal: () => { return this.#getManager()[`getGlobal${hookType}Hooks`](); } }; }This would allow you to set up all hook methods more concisely in the constructor.
packages/core/test/taskExecutor.test.ts (1)
913-915
:⚠️ Potential issueRemove unreachable code after throw statement
The code at lines 914-915 is unreachable because execution will be halted by the throw statement at line 912.
throw expectedError; - // Should never get here - await next(); - executionOrder.push("middleware-after");🧰 Tools
🪛 Biome (1.9.4)
[error] 914-915: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
🧹 Nitpick comments (22)
references/hello-world/src/trigger/example.ts (1)
151-194
: New task successfully demonstrates lifecycle hooks functionality.The
hooksTask
effectively showcases the new lifecycle hooks system with implementations of all available hooks. This serves as good documentation for developers learning how to use the new features.Note that according to the PR objectives,
init
andcleanup
hooks are marked as deprecated and should be replaced with middleware and locals. Consider adding a comment indicating these are deprecated to avoid misleading new users.init: async () => { + // @deprecated Use middleware instead return { foobar: "baz", }; }, ... cleanup: async ({ ctx, payload }) => { + // @deprecated Use middleware instead logger.info("Hello, world from the cleanup hook", { payload }); },packages/core/src/v3/locals/index.ts (1)
1-45
: Well-structured locals management implementation.The
LocalsAPI
class provides a clean singleton implementation for managing local variables across the application. The approach using the global registry pattern with fallback to a no-operation manager is robust.However, there are a few issues flagged by static analysis:
- Using
this
in static methods can be confusing. Replace with the class name:public static getInstance(): LocalsAPI { - if (!this._instance) { - this._instance = new LocalsAPI(); - } - return this._instance; + if (!LocalsAPI._instance) { + LocalsAPI._instance = new LocalsAPI(); + } + return LocalsAPI._instance; }
- Void function returning a value:
public setLocal<T>(key: LocalsKey<T>, value: T): void { - return this.#getManager().setLocal(key, value); + this.#getManager().setLocal(key, value); }🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 16-16: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 19-19: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 39-39: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/core/src/v3/lifecycleHooks/index.ts (1)
30-35
: Fix static method reference.Using
this
in static methods can be confusing. Consider replacing with the class name for clarity.public static getInstance(): LifecycleHooksAPI { - if (!this._instance) { - this._instance = new LifecycleHooksAPI(); - } - return this._instance; + if (!LifecycleHooksAPI._instance) { + LifecycleHooksAPI._instance = new LifecycleHooksAPI(); + } + return LifecycleHooksAPI._instance; }🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 31-31: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 34-34: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/core/src/v3/locals/types.ts (2)
1-2
: Consider adding documentation for your type branding approach.
Declaring__local
as a unique symbol is a clever way to create a brand type, but adding a brief comment explaining its purpose can improve clarity for future maintainers.
10-14
: Consider supporting a method to remove or clear locals.
TheLocalsManager
interface currently allows creation, retrieval, and setting of local values. In some scenarios, it may be useful to remove or reset a local key.packages/core/src/v3/locals/manager.ts (3)
3-16
: Maintain consistent symbol usage.
While using plainSymbol()
is fine for a no-op manager, consider using the same approach (Symbol.for
) as in the standard manager to ensure consistent global registration or to clarify the reason behind using ephemeral symbols here.
18-36
: Avoid potential collisions withSymbol.for(id)
.
Reuse of the same identifier inSymbol.for
from different parts of the codebase could lead to unexpected overlaps. Adding a namespace prefix toid
or a more descriptive approach could mitigate collisions.
37-37
: Remove the extraneous literal.
The0;
line at the end of the file appears to be unused. Removing it can declutter the code.-0;
packages/trigger-sdk/src/v3/hooks.ts (4)
30-40
: Consider removingvoid
from the return type.
The return typeundefined | void | Promise<undefined | void>
can be simplified, as usingvoid
inside a union type is flagged by static analysis.Here’s a possible diff:
-export function onStart(name: string, fn: AnyOnStartHookFunction): void; -export function onStart(fn: AnyOnStartHookFunction): void; -export function onStart( - fnOrName: string | AnyOnStartHookFunction, - fn?: AnyOnStartHookFunction -): void { +export function onStart(name: string, fn: AnyOnStartHookFunction): undefined; +export function onStart(fn: AnyOnStartHookFunction): undefined; +export function onStart( + fnOrName: string | AnyOnStartHookFunction, + fn?: AnyOnStartHookFunction +): undefined { lifecycleHooks.registerGlobalStartHook({ id: typeof fnOrName === "string" ? fnOrName : fnOrName.name ? fnOrName.name : undefined, fn: typeof fnOrName === "function" ? fnOrName : fn!, }); }
42-52
: onFailure hook consistency.
Function overloads look consistent. Consider similarly removingvoid
from the return type to align with the static analysis recommendation.
54-64
: onSuccess hook pattern.
You’re correctly applying the same overload pattern as onStart and onFailure. Removingvoid
in union types would further align with best practices.
66-133
: Consolidate return type usage and handle ID collisions.
- Return type: Several hooks (
onComplete
,onWait
,onResume
,onHandleError
,onCatchError
,middleware
) also featurevoid
in union types, which is discouraged by static analysis.- ID collisions: If multiple hooks have no explicit name and blank function names, the hook
id
can end up undefined, potentially overwriting or failing to register properly. Consider providing a fallback or requiring a non-empty string for the name in user-facing docs.packages/core/src/v3/lifecycleHooks/types.ts (10)
4-5
: TaskInitOutput usage ofvoid
in union types.
Static analysis flagsvoid
as confusing in union types. Switching toundefined
alone is typically clearer.-export type TaskInitOutput = Record<string, any> | void | undefined; +export type TaskInitOutput = Record<string, any> | undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
13-15
: OnInitHookFunction return type.
The union type includesvoid
. As above, consider droppingvoid
from the union.🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
19-35
: TaskStartHookParams and OnStartHookFunction.
Similar pattern of union return withvoid
. Consider removing it to match best practices.🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
55-71
: OnWaitHookFunction.
Again, returningundefined | void
is flagged by the linter. Removingvoid
clarifies your API.🧰 Tools
🪛 Biome (1.9.4)
[error] 69-69: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
73-90
: OnResumeHookFunction.
Identical pattern regarding union withvoid
. Otherwise, the function definition is consistent.🧰 Tools
🪛 Biome (1.9.4)
[error] 87-87: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
91-108
: OnFailureHookFunction.
Logic for passing the error is appropriate. Consider the same improvement for the return type.🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
109-131
: OnSuccessHookFunction.
The design is consistent. Watch forvoid
usage in union types.🧰 Tools
🪛 Biome (1.9.4)
[error] 128-128: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
146-168
: OnCompleteHookFunction return type.
Static analysis likewise flagsvoid
in unions.🧰 Tools
🪛 Biome (1.9.4)
[error] 165-165: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
180-201
: OnCatchErrorHookFunction definition.
This is a neat way to unify error modification and retry logic. All good, aside from the recurring “void” union.
216-232
: OnCleanupHookFunction.
Deprecation or not, it is consistent with the rest. Watch for union usage ofvoid
.🧰 Tools
🪛 Biome (1.9.4)
[error] 229-229: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
.changeset/weak-jobs-hide.md
(1 hunks).cursor/rules/executing-commands.mdc
(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx
(3 hunks)internal-packages/run-engine/src/engine/errors.ts
(1 hunks)packages/cli-v3/src/build/bundle.ts
(5 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts
(1 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts
(8 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts
(1 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts
(7 hunks)packages/core/src/v3/config.ts
(2 hunks)packages/core/src/v3/errors.ts
(1 hunks)packages/core/src/v3/index.ts
(1 hunks)packages/core/src/v3/lifecycle-hooks-api.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/index.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/manager.ts
(1 hunks)packages/core/src/v3/lifecycleHooks/types.ts
(1 hunks)packages/core/src/v3/locals-api.ts
(1 hunks)packages/core/src/v3/locals/index.ts
(1 hunks)packages/core/src/v3/locals/manager.ts
(1 hunks)packages/core/src/v3/locals/types.ts
(1 hunks)packages/core/src/v3/runtime/managedRuntimeManager.ts
(4 hunks)packages/core/src/v3/schemas/build.ts
(2 hunks)packages/core/src/v3/schemas/common.ts
(1 hunks)packages/core/src/v3/semanticInternalAttributes.ts
(1 hunks)packages/core/src/v3/tracer.ts
(1 hunks)packages/core/src/v3/types/tasks.ts
(6 hunks)packages/core/src/v3/utils/globals.ts
(2 hunks)packages/core/src/v3/workers/index.ts
(1 hunks)packages/core/src/v3/workers/taskExecutor.ts
(11 hunks)packages/core/test/taskExecutor.test.ts
(1 hunks)packages/trigger-sdk/src/v3/hooks.ts
(1 hunks)packages/trigger-sdk/src/v3/index.ts
(1 hunks)packages/trigger-sdk/src/v3/locals.ts
(1 hunks)packages/trigger-sdk/src/v3/shared.ts
(6 hunks)packages/trigger-sdk/src/v3/tasks.ts
(3 hunks)references/hello-world/src/db.ts
(1 hunks)references/hello-world/src/trigger/example.ts
(2 hunks)references/hello-world/src/trigger/init.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/weak-jobs-hide.md
🚧 Files skipped from review as they are similar to previous changes (23)
- packages/core/src/v3/semanticInternalAttributes.ts
- packages/cli-v3/src/entryPoints/managed-index-worker.ts
- packages/core/src/v3/errors.ts
- .cursor/rules/executing-commands.mdc
- packages/trigger-sdk/src/v3/locals.ts
- packages/core/src/v3/utils/globals.ts
- packages/trigger-sdk/src/v3/index.ts
- packages/core/src/v3/schemas/common.ts
- packages/core/src/v3/index.ts
- internal-packages/run-engine/src/engine/errors.ts
- references/hello-world/src/trigger/init.ts
- packages/core/src/v3/workers/index.ts
- packages/core/src/v3/tracer.ts
- apps/webapp/app/components/runs/v3/RunIcon.tsx
- packages/core/src/v3/schemas/build.ts
- packages/core/src/v3/lifecycleHooks/manager.ts
- packages/cli-v3/src/entryPoints/dev-index-worker.ts
- packages/core/src/v3/config.ts
- packages/cli-v3/src/build/bundle.ts
- references/hello-world/src/db.ts
- packages/core/src/v3/lifecycle-hooks-api.ts
- packages/cli-v3/src/entryPoints/dev-run-worker.ts
- packages/trigger-sdk/src/v3/shared.ts
🧰 Additional context used
🧬 Code Definitions (10)
references/hello-world/src/trigger/example.ts (4)
packages/trigger-sdk/src/v3/index.ts (1)
logger
(36-36)packages/trigger-sdk/src/v3/tasks.ts (1)
task
(80-80)packages/core/src/v3/workers/taskExecutor.ts (12)
payload
(302-353)payload
(355-393)payload
(535-629)payload
(631-702)payload
(704-775)payload
(777-787)payload
(789-851)payload
(853-861)payload
(863-931)payload
(1119-1190)wait
(395-463)wait
(465-533)packages/trigger-sdk/src/v3/wait.ts (1)
wait
(138-293)
packages/core/src/v3/locals/types.ts (3)
packages/core/src/v3/locals-api.ts (1)
LocalsKey
(29-29)packages/trigger-sdk/src/v3/locals.ts (1)
LocalsKey
(3-3)packages/core/src/v3/locals/index.ts (1)
LocalsManager
(42-44)
packages/trigger-sdk/src/v3/tasks.ts (1)
packages/trigger-sdk/src/v3/hooks.ts (2)
onHandleError
(104-109)onCatchError
(113-121)
packages/core/src/v3/workers/taskExecutor.ts (4)
packages/core/src/v3/schemas/schemas.ts (2)
RetryOptions
(99-126)RetryOptions
(128-128)packages/core/src/v3/lifecycleHooks/types.ts (3)
RegisteredHookFunction
(174-178)AnyOnMiddlewareHookFunction
(214-214)TaskWait
(36-53)packages/core/src/v3/errors.ts (1)
InternalError
(18-44)packages/core/src/v3/types/tasks.ts (1)
HandleErrorModificationOptions
(128-134)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycleHooks/types.ts (8)
AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)packages/core/src/v3/lifecycle-hooks-api.ts (9)
AnyOnStartHookFunction
(14-14)lifecycleHooks
(5-5)AnyOnFailureHookFunction
(16-16)AnyOnSuccessHookFunction
(18-18)AnyOnCompleteHookFunction
(20-20)AnyOnWaitHookFunction
(22-22)AnyOnResumeHookFunction
(24-24)AnyOnCatchErrorHookFunction
(26-26)AnyOnMiddlewareHookFunction
(29-29)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (10)
OnInitHookFunction
(13-15)OnCleanupHookFunction
(227-229)OnCatchErrorHookFunction
(195-198)OnResumeHookFunction
(85-87)OnWaitHookFunction
(67-69)OnCompleteHookFunction
(159-165)OnMiddlewareHookFunction
(210-212)OnStartHookFunction
(30-32)OnSuccessHookFunction
(122-128)OnFailureHookFunction
(103-105)
packages/core/src/v3/lifecycleHooks/types.ts (4)
packages/core/src/v3/lifecycle-hooks-api.ts (4)
TaskInitHookParams
(11-11)OnInitHookFunction
(8-8)AnyOnInitHookFunction
(9-9)TaskWait
(34-34)packages/core/src/v3/types/tasks.ts (1)
HandleErrorResult
(136-140)packages/core/src/v3/lifecycleHooks/index.ts (1)
LifecycleHooksManager
(263-265)packages/core/src/v3/workers/taskExecutor.ts (2)
wait
(395-463)wait
(465-533)
packages/core/src/v3/lifecycleHooks/index.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (14)
LifecycleHooksManager
(233-310)RegisterHookFunctionParams
(169-172)AnyOnInitHookFunction
(17-17)RegisteredHookFunction
(174-178)AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)AnyOnCleanupHookFunction
(231-231)TaskWait
(36-53)
packages/core/src/v3/locals/index.ts (3)
packages/core/src/v3/locals/manager.ts (1)
NoopLocalsManager
(3-16)packages/core/src/v3/locals/types.ts (2)
LocalsManager
(10-14)LocalsKey
(5-8)packages/core/src/v3/utils/globals.ts (3)
registerGlobal
(19-35)unregisterGlobal
(43-49)getGlobal
(37-41)
packages/core/test/taskExecutor.test.ts (5)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
lifecycleHooks
(5-5)packages/core/src/v3/lifecycleHooks/manager.ts (1)
StandardLifecycleHooksManager
(18-397)packages/core/src/v3/workers/taskExecutor.ts (13)
payload
(302-353)payload
(355-393)payload
(535-629)payload
(631-702)payload
(704-775)payload
(777-787)payload
(789-851)payload
(853-861)payload
(863-931)payload
(1119-1190)result
(1070-1117)execution
(952-1067)execution
(1192-1208)packages/core/src/v3/types/tasks.ts (2)
RunFnParams
(86-93)TaskMetadataWithFunctions
(871-887)packages/core/src/v3/schemas/common.ts (4)
TaskRunErrorCodes
(195-195)TaskRunErrorCodes
(196-196)TaskRunExecution
(302-316)TaskRunExecution
(318-318)
🪛 Biome (1.9.4)
packages/core/src/v3/lifecycleHooks/types.ts
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 15-15: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 32-32: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 69-69: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 87-87: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 105-105: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 128-128: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 165-165: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 229-229: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
packages/core/src/v3/lifecycleHooks/index.ts
[error] 30-30: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 31-31: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 34-34: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/core/src/v3/locals/index.ts
[error] 15-15: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 16-16: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 19-19: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 39-39: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/core/test/taskExecutor.test.ts
[error] 914-915: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (42)
references/hello-world/src/trigger/example.ts (1)
7-8
: Appropriate logging added.Additional logging at the beginning of the task execution is helpful for debugging. This provides visibility into the context and payload being received.
packages/core/src/v3/lifecycleHooks/index.ts (1)
247-261
: Consistent hook listener management.The wait and resume hook listener methods are well-implemented and provide a clean way to register callback functions for these events.
packages/trigger-sdk/src/v3/tasks.ts (3)
1-11
: New hooks imported correctly.The addition of the new hook imports aligns with the PR objectives to enhance the lifecycle hooks system.
60-60
: Type export enhances API discoverability.Re-exporting all types from the hooks module is a good practice that improves API discoverability and makes it easier for developers to access the necessary types.
92-101
: Updated tasks object with clear deprecation notice.The tasks object has been properly updated with the new hook methods. The deprecation notice for
handleError
is well-documented, making it clear that developers should usecatchError
instead, which aligns with the PR objectives.packages/core/src/v3/runtime/managedRuntimeManager.ts (5)
56-59
: Same error handling concern applies here.
84-88
: Same error handling concern applies here.
92-96
: Same error handling concern applies here.
117-127
: Same error handling concern applies here.
131-141
: Same error handling concern applies here.packages/core/test/taskExecutor.test.ts (2)
16-43
: Comprehensive test suite for TaskExecutor lifecycle hooksThis test file provides excellent coverage of the new lifecycle hooks system, testing all the hook types in various scenarios. The setup with
StandardLifecycleHooksManager
ensures tests run in isolation.
1626-1716
: Well-structured test helper functionThe
executeTask
helper function encapsulates all the necessary setup for testing, making the individual test cases easier to read and maintain. It properly initializes the tracing, console interception, and execution context.packages/core/src/v3/locals-api.ts (1)
1-29
: Well-designed locals API with tree-shaking supportThis new API provides a clean, type-safe interface for managing local variables with complete CRUD operations. The separation of the API instance and the interface allows for better tree-shaking as mentioned in the comments.
The
getOrThrow
method provides a convenient way to retrieve values with proper error handling, including the variable ID in the error message for easier debugging.packages/cli-v3/src/entryPoints/managed-run-worker.ts (4)
4-14
: Well-structured imports for enhanced lifecycle hooks systemThe addition of these new hook types aligns with the PR objectives to enhance the lifecycle hooks system. These imports enable type-safe registration of various hook functions.
105-109
: Good initialization of the new managersInitializing
StandardLocalsManager
and setting it as the global manager ensures consistent state management across the application. This is a key part of the new locals system described in the PR objectives.
197-231
: Clear implementation of global hook registrationThis section properly registers global hooks from the configuration, maintaining backward compatibility while implementing the new hook system. The type assertions ensure type safety when registering hooks.
401-401
: Simplified task execution with direct executor callThe execution logic has been streamlined by directly calling
executor.execute
with the necessary parameters, which aligns with the changes to theTaskExecutor
class.packages/core/src/v3/workers/taskExecutor.ts (7)
45-53
: Enhanced TaskExecutorOptions with configurable retriesAdding the
retries
configuration toTaskExecutorOptions
allows for more flexible retry behavior, including enabling retries in development mode. This is a valuable addition that improves task execution reliability.
112-126
: Improved error handling with tryCatch utilityThe use of the
tryCatch
utility creates a more structured approach to error handling. It simplifies error management by capturing errors in a standardized way, making the code more readable and maintainable.
130-136
: Support for onWait and onResume hooksThe addition of registering listeners for
onWait
andonResume
events enhances the task execution flow by allowing for better resource management during wait periods, as specified in the PR objectives.
138-261
: Well-structured executeTask functionThis refactoring encapsulates the task execution logic in a dedicated function with clear error handling paths. The structured approach to handling various error scenarios and success cases improves code maintainability.
302-353
: Excellent middleware implementation with reducer patternThe use of the reducer pattern to compose middleware functions is elegant and effective. It ensures middleware executes in the correct order, with proper nesting and error handling.
1119-1190
: Comprehensive onComplete hook implementationThe implementation of the
onComplete
hooks follows the established pattern for lifecycle hooks, ensuring consistency across the codebase. This new hook provides a reliable way to execute code after task completion regardless of success or failure.
1070-1117
: Clear error result processingThe
processHandleErrorResult
method centralizes the logic for interpreting error handler results, making the code more maintainable. It handles various retry options consistently, improving the reliability of error recovery.packages/trigger-sdk/src/v3/hooks.ts (2)
1-14
: Imports and re-exports look good.
No issues with the import statements or exported types.
16-28
: Re-exporting types.
These lines concisely re-export the needed lifecycle hook types. No concerns.packages/core/src/v3/types/tasks.ts (9)
3-15
: Newly added lifecycle hooks in imports.
The introduction of lifecycle hook types aligns well with the new v4 hooking system. No immediate concerns.
26-29
: Additional imports.
ImportingQueueOptions
andPrettify
appears routine. No issues.
110-110
: Optional init property.
Allowinginit?: InitOutput
inStartFnParams
is consistent with the new approach. Confirm that all call sites handle the possibility ofundefined
.
275-277
: init marked as deprecated.
The deprecation note is clear. This change is consistent with the shift toward locals and middleware.
285-285
: cleanup is also deprecated.
As indicated, shifting to middleware ensures more flexible resource management.
289-293
: handleError property is deprecated.
ReplacinghandleError
withcatchError
is straightforward. Good job keeping the legacy property for backward compatibility.
294-297
: New catchError property.
Provides a clear, consistent error-handling hook. Well aligned with the new lifecycle approach.
299-301
: onResume, onWait, and onComplete hooks.
These additions give more granular control over tasks. Looks clear and type-safe.
320-335
: middleware, onStart, onSuccess, onFailure hooks.
The introduction of these hook properties inCommonTaskOptions
is consistent with the rest of the architecture.packages/core/src/v3/lifecycleHooks/types.ts (7)
1-3
: Import statements.
All relevant types are correctly imported from sibling modules.
6-11
: TaskInitHookParams signature looks good.
Includingpayload
,ctx
, and asignal
is aligned with typical initialization hooks.
36-54
: TaskWait union.
The typed “wait” definitions are clear. This block is a good approach allowing multiple wait mechanisms.
132-145
: TaskCompleteResult.
Returning either success or error is well-defined. No functional issues.
169-179
: RegisterHookFunctionParams and RegisteredHookFunction.
Straightforward approach for storing metadata and function references.
202-215
: TaskMiddlewareHookParams.
Including anext()
function is standard practice for layered middleware. Good approach.
233-310
: LifecycleHooksManager interface.
Comprehensive coverage for all hook registration and retrieval methods. The organization is clear and modular.
await lifecycleHooks.callOnWaitHookListeners({ | ||
type: "task", | ||
runId: params.id, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling around hook calls.
If callOnWaitHookListeners
throws an error, it could block task processing. Consider wrapping these asynchronous hook calls in a try/catch block and decide whether to proceed, retry, or fail gracefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
43-43
:⚠️ Potential issueFix unreachable code in spanNameIcon handling
There's a JSX element on line 43 that's created but never returned, making it unreachable code. This isn't related to your current changes but should be fixed.
- <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; + return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
🧹 Nitpick comments (6)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
79-100
: Consider refactoring repetitive task hook casesAll these new hook cases return the same
InformationCircleIcon
with identical styling. Consider refactoring this repetitive pattern to improve maintainability.case "task-middleware": return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; case "task-fn-run": return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-init": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onStart": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onSuccess": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onFailure": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onComplete": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onWait": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-onResume": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-catchError": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-cleanup": - return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; + case "task-hook-init": + case "task-hook-onStart": + case "task-hook-onSuccess": + case "task-hook-onFailure": + case "task-hook-onComplete": + case "task-hook-onWait": + case "task-hook-onResume": + case "task-hook-catchError": + case "task-hook-cleanup": + return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;Alternatively, consider a more scalable approach using a pattern matching technique:
+ // Handle all task hook cases + if (name?.startsWith("task-hook-")) { + return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; + } switch (name) { case "task": return <TaskIcon className={cn(className, "text-blue-500")} />; // ... other cases case "task-middleware": return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; case "task-fn-run": return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />; - case "task-hook-init": - case "task-hook-onStart": - // ... other task-hook cases removedpackages/core/src/v3/lifecycleHooks/manager.ts (5)
69-79
: Consider adding error handling for hook listeners.The
callOnWaitHookListeners
andcallOnResumeHookListeners
methods usePromise.allSettled
which prevents failures from propagating, but there's no logging or handling of rejected promises.async callOnWaitHookListeners(wait: TaskWait): Promise<void> { - await Promise.allSettled(this.onWaitHookListeners.map((listener) => listener(wait))); + const results = await Promise.allSettled(this.onWaitHookListeners.map((listener) => listener(wait))); + const rejections = results.filter((result): result is PromiseRejectedResult => result.status === 'rejected'); + if (rejections.length > 0) { + console.warn(`${rejections.length} onWait hook listeners failed:`, rejections.map(r => r.reason)); + } } async callOnResumeHookListeners(wait: TaskWait): Promise<void> { - await Promise.allSettled(this.onResumeHookListeners.map((listener) => listener(wait))); + const results = await Promise.allSettled(this.onResumeHookListeners.map((listener) => listener(wait))); + const rejections = results.filter((result): result is PromiseRejectedResult => result.status === 'rejected'); + if (rejections.length > 0) { + console.warn(`${rejections.length} onResume hook listeners failed:`, rejections.map(r => r.reason)); + } }
112-123
: Consider documenting hook replacement behavior.When registering hooks with the same ID multiple times, the newer hook will silently replace the older one due to the Map behavior. This might cause confusion if not documented.
registerGlobalInitHook(hook: RegisterHookFunctionParams<AnyOnInitHookFunction>): void { // if there is no id, lets generate one based on the contents of the function const id = generateHookId(hook); + // If a hook with this ID already exists, it will be replaced const registeredHook = { id, name: hook.id, fn: hook.fn, }; this.globalInitHooks.set(id, registeredHook); }
125-136
: Inconsistent naming between registerTaskInitHook and other methods.In
registerTaskInitHook
, the registered hook uses the taskId as the name property, while other hook registration methods usehook.id
as the name. This inconsistency might lead to confusion.registerTaskInitHook( taskId: string, hook: RegisterHookFunctionParams<AnyOnInitHookFunction> ): void { const registeredHook = { id: generateHookId(hook), - name: taskId, + name: hook.id, fn: hook.fn, }; this.taskInitHooks.set(taskId, registeredHook); }
18-397
: Comprehensive lifecycle hooks management implementation.The
StandardLifecycleHooksManager
class is well-structured and provides a complete implementation of all the lifecycle hooks required by the system. The consistent pattern used for each hook type enhances maintainability.Consider adding a clear comment documenting the purpose of this class and how it fits into the overall architecture.
+/** + * StandardLifecycleHooksManager implements the LifecycleHooksManager interface + * providing a comprehensive solution for managing task lifecycle hooks. + * + * It supports both global hooks (applied to all tasks) and task-specific hooks, + * along with special listener patterns for wait and resume events. + */ export class StandardLifecycleHooksManager implements LifecycleHooksManager { // existing implementation... }
399-599
: NoopLifecycleHooksManager provides a good fallback implementation.The no-op implementation is useful for testing or as a fallback, but has inconsistent method signatures compared to the main implementation.
In addition to fixing the parameter types mentioned in previous comments, consider adding a class-level comment explaining when this implementation should be used.
+/** + * NoopLifecycleHooksManager provides a no-operation implementation of the + * LifecycleHooksManager interface. This can be used as a fallback when no + * lifecycle hook management is needed or for testing purposes. + */ export class NoopLifecycleHooksManager implements LifecycleHooksManager { // existing implementation with fixed parameter types... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/components/runs/v3/RunIcon.tsx
(2 hunks)packages/cli-v3/src/build/bundle.ts
(5 hunks)packages/core/src/v3/lifecycleHooks/manager.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli-v3/src/build/bundle.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (12)
AnyOnInitHookFunction
(17-17)AnyOnStartHookFunction
(34-34)AnyOnFailureHookFunction
(107-107)AnyOnSuccessHookFunction
(130-130)AnyOnCompleteHookFunction
(167-167)AnyOnWaitHookFunction
(71-71)AnyOnResumeHookFunction
(89-89)AnyOnCatchErrorHookFunction
(200-200)AnyOnMiddlewareHookFunction
(214-214)AnyOnCleanupHookFunction
(231-231)TaskWait
(36-53)RegisterHookFunctionParams
(169-172)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
47-49
: Great enhancement for dynamic icon rendering!Adding support for dynamic icon lookup in the
tablerIcons
set makes the component more flexible and future-proof. This allows for rendering any icon that exists in the icon set without modifying the switch statement.packages/core/src/v3/lifecycleHooks/manager.ts (3)
1-16
: Well-structured imports with descriptive type names.The imports are comprehensive and clearly named, providing good type safety for the lifecycle hooks implementation.
18-64
: Excellent design with clear separation of concerns.The
StandardLifecycleHooksManager
class implements a robust lifecycle hooks system with:
- Separate maps for global and task-specific hooks
- Dedicated storage for each hook type
- Well-designed listeners for wait and resume events
This architecture allows for flexible hook registration and efficient lookup.
601-603
: Simple and effective utility function for generating hook IDs.The
generateHookId
function provides a convenient way to generate unique IDs for hooks, either using the provided ID or falling back to the function's string representation.
registerGlobalMiddlewareHook(): void { | ||
// Noop | ||
} | ||
|
||
registerTaskMiddlewareHook(): void { | ||
// Noop | ||
} | ||
|
||
getTaskMiddlewareHook(): undefined { | ||
return undefined; | ||
} | ||
|
||
getGlobalMiddlewareHooks(): [] { | ||
return []; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing parameter types in middleware hook methods.
Similar to the previous issue, the middleware hook methods also lack proper parameter types.
- registerGlobalMiddlewareHook(): void {
+ registerGlobalMiddlewareHook(hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>): void {
// Noop
}
- registerTaskMiddlewareHook(): void {
+ registerTaskMiddlewareHook(
+ taskId: string,
+ hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>
+ ): void {
// Noop
}
- getTaskMiddlewareHook(): undefined {
+ getTaskMiddlewareHook(taskId: string): AnyOnMiddlewareHookFunction | undefined {
return undefined;
}
- getGlobalMiddlewareHooks(): [] {
+ getGlobalMiddlewareHooks(): RegisteredHookFunction<AnyOnMiddlewareHookFunction>[] {
return [];
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
registerGlobalMiddlewareHook(): void { | |
// Noop | |
} | |
registerTaskMiddlewareHook(): void { | |
// Noop | |
} | |
getTaskMiddlewareHook(): undefined { | |
return undefined; | |
} | |
getGlobalMiddlewareHooks(): [] { | |
return []; | |
} | |
registerGlobalMiddlewareHook(hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>): void { | |
// Noop | |
} | |
registerTaskMiddlewareHook( | |
taskId: string, | |
hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction> | |
): void { | |
// Noop | |
} | |
getTaskMiddlewareHook(taskId: string): AnyOnMiddlewareHookFunction | undefined { | |
return undefined; | |
} | |
getGlobalMiddlewareHooks(): RegisteredHookFunction<AnyOnMiddlewareHookFunction>[] { | |
return []; | |
} |
registerGlobalCatchErrorHook(): void { | ||
// Noop | ||
} | ||
|
||
registerTaskCatchErrorHook(): void { | ||
// Noop | ||
} | ||
|
||
getTaskCatchErrorHook(): undefined { | ||
return undefined; | ||
} | ||
|
||
getGlobalCatchErrorHooks(): [] { | ||
return []; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing parameter types in NoopLifecycleHooksManager methods.
Several methods in the NoopLifecycleHooksManager
class don't include proper parameter types, which could lead to type safety issues.
- registerGlobalCatchErrorHook(): void {
+ registerGlobalCatchErrorHook(hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>): void {
// Noop
}
- registerTaskCatchErrorHook(): void {
+ registerTaskCatchErrorHook(
+ taskId: string,
+ hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>
+ ): void {
// Noop
}
- getTaskCatchErrorHook(): undefined {
+ getTaskCatchErrorHook(taskId: string): AnyOnCatchErrorHookFunction | undefined {
return undefined;
}
- getGlobalCatchErrorHooks(): [] {
+ getGlobalCatchErrorHooks(): RegisteredHookFunction<AnyOnCatchErrorHookFunction>[] {
return [];
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
registerGlobalCatchErrorHook(): void { | |
// Noop | |
} | |
registerTaskCatchErrorHook(): void { | |
// Noop | |
} | |
getTaskCatchErrorHook(): undefined { | |
return undefined; | |
} | |
getGlobalCatchErrorHooks(): [] { | |
return []; | |
} | |
registerGlobalCatchErrorHook(hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>): void { | |
// Noop | |
} | |
registerTaskCatchErrorHook( | |
taskId: string, | |
hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction> | |
): void { | |
// Noop | |
} | |
getTaskCatchErrorHook(taskId: string): AnyOnCatchErrorHookFunction | undefined { | |
return undefined; | |
} | |
getGlobalCatchErrorHooks(): RegisteredHookFunction<AnyOnCatchErrorHookFunction>[] { | |
return []; | |
} |
This PR revamps the lifecycle hooks system, including a new way of defining global hooks, better middleware with locals support, new hooks such as
onWait
,onResume
, andonComplete
, and a newinit.ts
file that is automatically loaded on every run.Breaking changes
The params passed to the existing hooks have been consolidated into a single param object.
Before:
After:
Deprecations
These are marked as deprecated but will continue to work:
init
is deprecated, in favor of middleware and localscleanup
is deprecated for the same reasonhandleError
is deprecated, usecatchError
insteadNew global hooks support
Previously, you could only define a single global lifecycle hook in the
trigger.config.ts
file, but this meant that you'd have to import any dependencies you needed in the hooks into the config file, which could cause issues because it sometimes broke the config loading.Now you can define one or more global hooks anywhere in your codebase, like so:
Improved middleware and locals
Previously, if you wanted to initialize some resource (e.g. a database client) and provide it to your tasks, you'd have to use the
init
hook and return the client, which you could then access in yourrun
function and other lifecycle hooks:This works okay when passing the init option when creating a task, but the types don't really work if the
init
is defined globally. So, we've improved the middleware and added a type-safe "locals" system to be able to easily provide resources/clients, etc. to all your tasks and lifecycle hooks:New hooks
We've introduced three new hooks
onWait
This hook will be called right before a wait is starting, such as
wait.for({ seconds: 5 })
,wait.forToken(token)
, oryourTask.triggerAndWait
.You can use this hook to cleanup resources should you need to, before starting a possibly long wait.
onResume
This hook is similar to onWait, but will be called after the wait has finished and before the rest of your code resumes running.
onComplete
This hook is called when a run has completed, either as a successful or failing run.
init.ts file
If you create an
init.ts
file in the root of your trigger directory, we'll automatically load this file before loading your task file when executing a task. This is handy as a place to put any global initialization code that you want to run before every task, without having to stick it in thetrigger.config.ts
file.Summary by CodeRabbit