fix: cleanup ttl expire run graphile jobs

[ericallam]

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced new services for managing task run attempts, including expiration handling.
  • Added new tasks such as immediateReturn and simulateError for enhanced task management.

• Improvements

  • Enhanced error handling and state management for task run attempts.
  • Simplified task triggering process in clientUsage.ts.
  • Updated createJsonHeroDoc task with a concurrency limit and increased wait duration.

• Bug Fixes

  • Updated logic to ensure task runs do not exceed maximum attempts.

• Chores

  • Updated script commands and dependency versions in package.json.

[changeset-bot]

⚠️ No Changeset found

Latest commit: a7895b0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

The changes encompass modifications across several service files related to task run management in a web application. Key updates include the introduction of the ExpireEnqueuedRunService, which facilitates handling task run expirations, and enhancements to error handling and state management in task run attempts. The package.json file has been updated to change the client script command and update dependency versions. Additionally, several tasks in the simple.ts file have been modified to adjust concurrency limits and wait durations, alongside the introduction of new tasks.

Changes

Files	Change Summary
apps/webapp/app/v3/services/createTaskRunAttempt.server.ts	Introduced ExpireEnqueuedRunService and MAX_TASK_RUN_ATTEMPTS constant. Updated task run attempt creation logic to include maximum attempts check and TTL handling. Enhanced error handling and state management.
apps/webapp/app/v3/services/enqueueDelayedRun.server.ts	Removed workerQueue import. Modified job enqueuing logic to use ExpireEnqueuedRunService.enqueue instead of workerQueue.enqueue.
apps/webapp/app/v3/services/expireEnqueuedRun.server.ts	Added dequeue and enqueue static methods for handling run expiration and queuing. Existing call method logic remains unchanged.
apps/webapp/app/v3/services/finalizeTaskRun.server.ts	Added import for ExpireEnqueuedRunService. Updated call method to check for ttl on the run object and invoke ExpireEnqueuedRunService.dequeue if present.
apps/webapp/app/v3/services/triggerTask.server.ts	Added import for ExpireEnqueuedRunService. Updated call method to utilize ExpireEnqueuedRunService.enqueue for task run expiration handling.
references/v3-catalog/package.json	Updated client script command from ts-node to tsx. Updated dependency versions without adding or removing any.
references/v3-catalog/src/clientUsage.ts	Simplified main function by reducing complexity and removing multiple task handling calls, focusing on a single trigger and retrieval process.
references/v3-catalog/src/trigger/simple.ts	Updated createJsonHeroDoc task to include a concurrency limit of 1 and increased wait duration from 5 to 30 seconds. Introduced new tasks: immediateReturn and simulateError.

Possibly related PRs

• fix: cleanup ttl expire run graphile jobs #1373: The changes in this PR directly relate to the ExpireEnqueuedRunService, which is also a key component in the main PR. Both PRs involve modifications to the handling of task runs with a TTL (time-to-live) and the integration of the ExpireEnqueuedRunService for managing task expiration.

🐰 "In the code where the tasks do play,
New services hop in, brightening the way.
With queues and limits, oh what a sight,
Task runs now dance, in the soft moonlight.
Errors are tamed, and runs do expire,
A rabbit's delight, as we build and inspire!" 🐇

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[pkg-pr-new]

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/build@1373

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/core@1373

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev@1373

pnpm add https://pkg.pr.new/triggerdotdev/trigger.dev/@trigger.dev/sdk@1373

commit: 1d2f385

[coderabbitai]

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (5)

references/v3-catalog/src/clientUsage.ts (1)

Line range hint 4-28: Remove redundant tasks.trigger call.

The main function contains two identical tasks.trigger calls. This redundancy is likely unintentional and should be addressed.

Consider removing one of the tasks.trigger calls to eliminate the duplication:

async function main() {
  await tasks.trigger(
    "create-jsonhero-doc",
    {
      title: "Hello World",
      content: {
        message: "Hello, World!",
      },
    },
    {
      ttl: "1m",
    }
  );

-  await tasks.trigger(
-    "create-jsonhero-doc",
-    {
-      title: "Hello World",
-      content: {
-        message: "Hello, World!",
-      },
-    },
-    {
-      ttl: "1m",
-    }
-  );
}

references/v3-catalog/package.json (1)

Line range hint 1-93: Consider regular package.json maintenance

While not directly related to the visible changes, it's good practice to regularly review and update the package.json file. Consider the following maintenance tasks:

1. Regularly update dependencies to their latest compatible versions to ensure security and performance improvements.
2. Review scripts to ensure they're all necessary and functioning as expected.
3. Consider adding scripts for common development tasks if they're not already present (e.g., linting, testing).
4. Ensure all dependencies are properly categorized as either dependencies or devDependencies.

Would you like assistance in setting up automated dependency updates or creating additional development scripts?

references/v3-catalog/src/trigger/simple.ts (1)

Line range hint 101-107: Clarify purpose of immediateReturn task and standardize logging

The newly added immediateReturn task appears to be designed for testing or debugging purposes, as it only logs messages without performing any significant operation or returning a value.

Consider the following suggestions:

1. Add a comment explaining the purpose of this task to improve code clarity.
2. Standardize the logging approach. Currently, both console and logger methods are used. It's generally better to stick to one logging method throughout the codebase for consistency.

Consider refactoring the task to use only the logger methods:

export const immediateReturn = task({
  id: "immediateReturn",
  run: async (payload: any, { ctx }) => {
    logger.info("some");
    logger.warn("random");
    logger.error("logs");
  },
});

This change would align with the logging approach used in other tasks within this file.

apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (2)

9-11: Consider adding unit tests for dequeue method

To ensure the dequeue method functions correctly in all scenarios, consider adding unit tests.

Would you like assistance in generating unit tests for this method, or should I open a GitHub issue to track this task?

---

13-19: Consider adding unit tests for enqueue method

To ensure the enqueue method functions correctly in all scenarios, consider adding unit tests.

Would you like assistance in generating unit tests for this method, or should I open a GitHub issue to track this task?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 27e8f5c and 1d2f385.

📒 Files selected for processing (8)

• apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (2 hunks)
• apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (2 hunks)
• apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (1 hunks)
• apps/webapp/app/v3/services/finalizeTaskRun.server.ts (2 hunks)
• apps/webapp/app/v3/services/triggerTask.server.ts (2 hunks)
• references/v3-catalog/package.json (1 hunks)
• references/v3-catalog/src/clientUsage.ts (2 hunks)
• references/v3-catalog/src/trigger/simple.ts (1 hunks)

🔇 Additional comments (13)

references/v3-catalog/src/clientUsage.ts (2)

1-1: LGTM: Import statement is correct.

The import of tasks from "@trigger.dev/sdk/v3" is appropriate for the usage in the main function.

---

31-31: LGTM: Error handling is appropriate.

The use of .catch(console.error) when calling main() ensures that any unhandled errors in the async function are logged to the console. This is a good practice for catching and reporting errors in the main execution flow.

references/v3-catalog/src/trigger/simple.ts (2)

Line range hint 109-122: Enhance error simulation in simulateError task

The newly added simulateError task is designed to simulate an error scenario after a 1-second delay. This can be useful for testing error handling in the system. However, there are a couple of points to consider:

1. The error message is hardcoded and doesn't utilize the message parameter from the payload.
2. The error is thrown indirectly through a separate function, which might make error tracking more difficult.

Consider refactoring the task to directly throw an error with the provided message:

export const simulateError = task({
  id: "simulateError",
  run: async (payload: { message: string }) => {
    // Sleep for 1 second
    await new Promise((resolve) => setTimeout(resolve, 1000));

    throw new Error(payload.message);
  },
});

This change would make the task more flexible (allowing custom error messages) and easier to track (by removing the indirection through thisFunctionWillThrow).

To ensure this change doesn't affect other parts of the codebase, let's check if thisFunctionWillThrow is used elsewhere:

#!/bin/bash
# Check for other uses of thisFunctionWillThrow
rg --type typescript "thisFunctionWillThrow" --glob '!references/v3-catalog/src/trigger/simple.ts'

If the function isn't used elsewhere, it can be safely removed.

---

Line range hint 1-214: Summary of changes and recommendations

This review covered the following main changes:

1. Added concurrency limit and increased wait duration in createJsonHeroDoc task.
2. Added new immediateReturn task for logging purposes.
3. Added new simulateError task for error simulation.

Key recommendations:

• Verify the necessity of the increased wait duration in createJsonHeroDoc.
• Standardize logging approaches across tasks.
• Enhance the flexibility and traceability of the simulateError task.

These changes appear to be focused on specific tasks and don't seem to have a widespread impact on the overall structure or functionality of the file. However, please ensure that these modifications align with the intended objectives of the PR, which are to "fix: cleanup ttl expire run graphile jobs".

To confirm that these changes align with the PR objectives, it would be helpful to review any related issues or discussions:

#!/bin/bash
# Fetch PR description and comments for context
gh pr view 1373 --json body,comments

This will provide additional context to ensure the changes meet the intended goals of the PR.

apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (1)

5-5: Import statement for ExpireEnqueuedRunService is correctly added

The import of ExpireEnqueuedRunService from "./expireEnqueuedRun.server" is appropriate and aligns with its usage below.

apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (3)

5-6: Imports added appropriately

The imports for workerQueue and PrismaClientOrTransaction are correctly added and necessary for the new methods.

---

9-11: Properly implemented dequeue method

The dequeue method is correctly defined to remove a run from the worker queue using the provided runId and optional transaction.

---

13-19: Properly implemented enqueue method

The enqueue method is correctly defined to add a run to the worker queue with the specified runId, optional runAt date, and optional transaction.

apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (3)

2-3: Imports are properly added and organized.

The necessary modules and constants are correctly imported:

• TaskRun, TaskRunAttempt from @trigger.dev/database
• MAX_TASK_RUN_ATTEMPTS from "~/consts"
• reportInvocationUsage from "~/services/platform.v3.server"
• BaseService, ServiceValidationError from "./baseService.server"
• ExpireEnqueuedRunService from "./expireEnqueuedRun.server"

These imports ensure the required functionalities are available within this service.

Also applies to: 7-7, 10-10, 12-12

---

Line range hint 97-109: Validation of maximum task run attempts is correctly implemented.

The logic to prevent task runs from exceeding the maximum number of allowed attempts is appropriately added. By comparing nextAttemptNumber with MAX_TASK_RUN_ATTEMPTS, the service ensures that no more attempts are made once the limit is reached. Additionally, it gracefully handles the situation by invoking CrashTaskRunService with a relevant reason.

---

142-145: Proper handling of task run TTL and dequeuing expired runs.

The addition of the conditional check for taskRun.ttl and the call to ExpireEnqueuedRunService.dequeue(taskRun.id, tx); within the transaction ensures that any task run with a TTL is correctly dequeued when execution starts. This effectively cleans up TTL-expired run jobs, aligning with the PR objective.

apps/webapp/app/v3/services/triggerTask.server.ts (2)

24-24: Import statement is correct

The ExpireEnqueuedRunService is correctly imported from ./expireEnqueuedRun.server.

---

439-439: ⚠️ Potential issue

Potential misuse of instance method

ExpireEnqueuedRunService.enqueue is being called as if enqueue is a static method. If enqueue is an instance method, you should create an instance of ExpireEnqueuedRunService before invoking enqueue.

To verify whether enqueue is a static method, run the following script:

If enqueue is not a static method, consider modifying the code as follows:

-if (expireAt) {
-  await ExpireEnqueuedRunService.enqueue(taskRun.id, expireAt, tx);
+if (expireAt) {
+  const expireService = new ExpireEnqueuedRunService();
+  await expireService.enqueue(taskRun.id, expireAt, tx);
 }