Gracefully shutdown task run processes #2299
Conversation
🦋 Changeset detectedLatest commit: 7e0f9a1 The changes in this PR will be included in the next version bump. 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 |
WalkthroughThe changes introduce a two-step process termination mechanism for task run processes by first sending a SIGTERM signal and waiting up to a configurable timeout (default 1000 ms) before sending a SIGKILL signal if the process remains alive. This is implemented via a new private method in the Estimated code review effort2 (~15 minutes) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/early-points-jam.md (1)
5-5: Fix typo in changeset descriptionThere's a typo where "Ttask" should be "Task".
-Gracefully shutdown task run processes using SIGTERM followed by SIGKILL after a 1s timeout. This also prevents cancelled or completed runs from leaving orphaned Ttask run processes behind +Gracefully shutdown task run processes using SIGTERM followed by SIGKILL after a 1s timeout. This also prevents cancelled or completed runs from leaving orphaned Task run processes behind
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/early-points-jam.md(1 hunks)packages/cli-v3/src/executions/taskRunProcess.ts(5 hunks)packages/cli-v3/src/indexing/indexWorkerManifest.ts(2 hunks)
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/indexing/indexWorkerManifest.tspackages/cli-v3/src/executions/taskRunProcess.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In `packages/core/src/v3/errors.ts`, within the `taskRunErrorEnhancer` function, `error.message` is always defined, so it's safe to directly call `error.message.includes("SIGTERM")` without additional checks.
packages/cli-v3/src/indexing/indexWorkerManifest.ts (1)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
.changeset/early-points-jam.md (10)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : The run function contains your task logic in Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When triggering a task from backend code, use tasks.trigger, tasks.batchTrigger, or tasks.triggerAndPoll as shown in the examples.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use schedules.task from @trigger.dev/sdk/v3 and follow the shown patterns.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the task function from @trigger.dev/sdk/v3 and export them as shown in the correct pattern.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use yourTask.trigger, yourTask.batchTrigger, yourTask.triggerAndWait, yourTask.batchTriggerAndWait, batch.triggerAndWait, batch.triggerByTask, or batch.triggerByTaskAndWait as shown.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : You MUST use @trigger.dev/sdk/v3 when writing Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using Realtime features, use the runs.subscribeToRun, runs.subscribeToRunsWithTag, and runs.subscribeToBatch APIs as shown.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using metadata in tasks, use the metadata API as shown, and only inside run functions or task lifecycle hooks.
packages/cli-v3/src/executions/taskRunProcess.ts (2)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
🧬 Code Graph Analysis (2)
packages/cli-v3/src/indexing/indexWorkerManifest.ts (1)
packages/core/src/v3/errors.ts (2)
TaskMetadataParseError(458-467)UncaughtExceptionError(447-456)
packages/cli-v3/src/executions/taskRunProcess.ts (1)
packages/cli-v3/src/utilities/logger.ts (1)
logger(113-113)
🪛 LanguageTool
.changeset/early-points-jam.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...or completed runs from leaving orphaned Ttask run processes behind
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/indexing/indexWorkerManifest.tspackages/cli-v3/src/executions/taskRunProcess.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In `packages/core/src/v3/errors.ts`, within the `taskRunErrorEnhancer` function, `error.message` is always defined, so it's safe to directly call `error.message.includes("SIGTERM")` without additional checks.
packages/cli-v3/src/indexing/indexWorkerManifest.ts (1)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
.changeset/early-points-jam.md (10)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : The run function contains your task logic in Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When triggering a task from backend code, use tasks.trigger, tasks.batchTrigger, or tasks.triggerAndPoll as shown in the examples.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use schedules.task from @trigger.dev/sdk/v3 and follow the shown patterns.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the task function from @trigger.dev/sdk/v3 and export them as shown in the correct pattern.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use yourTask.trigger, yourTask.batchTrigger, yourTask.triggerAndWait, yourTask.batchTriggerAndWait, batch.triggerAndWait, batch.triggerByTask, or batch.triggerByTaskAndWait as shown.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : You MUST use @trigger.dev/sdk/v3 when writing Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using Realtime features, use the runs.subscribeToRun, runs.subscribeToRunsWithTag, and runs.subscribeToBatch APIs as shown.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using metadata in tasks, use the metadata API as shown, and only inside run functions or task lifecycle hooks.
packages/cli-v3/src/executions/taskRunProcess.ts (2)
Learnt from: nicktrn
PR: #1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In packages/core/src/v3/errors.ts, within the taskRunErrorEnhancer function, error.message is always defined, so it's safe to directly call error.message.includes("SIGTERM") without additional checks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:24.984Z
Learning: Applies to /trigger//*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
🧬 Code Graph Analysis (2)
packages/cli-v3/src/indexing/indexWorkerManifest.ts (1)
packages/core/src/v3/errors.ts (2)
TaskMetadataParseError(458-467)UncaughtExceptionError(447-456)
packages/cli-v3/src/executions/taskRunProcess.ts (1)
packages/cli-v3/src/utilities/logger.ts (1)
logger(113-113)
🪛 LanguageTool
.changeset/early-points-jam.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...or completed runs from leaving orphaned Ttask run processes behind
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/cli-v3/src/executions/taskRunProcess.ts (4)
54-54: LGTM: Well-designed optional parameterThe new
gracefulTerminationTimeoutInMsparameter is appropriately typed, follows naming conventions, and maintains backward compatibility by being optional.
118-118: LGTM: Graceful termination implementation in cancelThe replacement of direct
kill()with#gracefullyTerminate()properly implements the graceful shutdown mechanism while maintaining existing error handling patterns.
135-135: LGTM: Consistent graceful termination in cleanupThe implementation follows the same pattern as the
cancel()method and properly respects the conditionalkillparameter.
399-409: LGTM: Well-implemented graceful termination logicThe two-step termination approach (SIGTERM → SIGKILL) is correctly implemented with:
- Appropriate default timeout (1s)
- Proper connection state checking before SIGKILL
- Good logging for debugging
- Reuse of existing
kill()method for consistencypackages/cli-v3/src/indexing/indexWorkerManifest.ts (4)
64-64: LGTM: Explicit SIGKILL signal in timeout handlerMaking the signal explicit improves code clarity and ensures consistent termination behavior when the worker times out.
82-82: LGTM: Explicit SIGKILL for successful completion cleanupThe explicit signal specification is consistent with other changes and appropriate for immediate cleanup after successful indexing.
89-89: LGTM: Explicit SIGKILL for parse failure cleanupThe explicit signal specification is appropriate for immediate cleanup after task parsing failures and maintains consistency across error handling paths.
96-96: LGTM: Consistent explicit SIGKILL across all termination pathsThe explicit signal specification completes the pattern of making all process termination signals explicit throughout the file, improving code clarity and ensuring consistent behavior.
When shutting down a task run process (either when it’s cancelled or when the run is finished and the process isn’t being reused) we will now issue a SIGTERM signal to the process, and then after a 1s delay, a SIGKILL if the process hasn't already been terminated. This is especially important for runs that are cancelled and intercept the SIGTERM handler and prevent the process from shutting down (which effectively stops cancelling from working).
One common source of these bugs we've seen is from the usage of Playwright inside a task, and launching a browser without setting the handleSIGTERM option to
false.