GeezerVison is showing github.com — fix(batch): rate limiting by token bucket no longer incorrectly goes negative by ericallam · Pull Request #2837 · triggerdotdev/trigger.dev
Conversation
ericallam
commented
Jan 6, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Also improves the BatchTriggerError when a result of getting rate limited.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
🦋 Changeset detected
Latest commit: 53f7153
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 26 packages
| Name |
Type |
| @trigger.dev/sdk |
Patch |
| @trigger.dev/python |
Patch |
| d3-chat |
Patch |
| references-d3-openai-agents |
Patch |
| references-nextjs-realtime |
Patch |
| references-realtime-hooks-test |
Patch |
| references-realtime-streams |
Patch |
| references-telemetry |
Patch |
| @trigger.dev/build |
Patch |
| @trigger.dev/core |
Patch |
| @trigger.dev/react-hooks |
Patch |
| @trigger.dev/redis-worker |
Patch |
| @trigger.dev/rsc |
Patch |
| @trigger.dev/schema-to-json |
Patch |
| @trigger.dev/database |
Patch |
| @trigger.dev/otlp-importer |
Patch |
| trigger.dev |
Patch |
| @internal/cache |
Patch |
| @internal/clickhouse |
Patch |
| @internal/redis |
Patch |
| @internal/replication |
Patch |
| @internal/run-engine |
Patch |
| @internal/schedule-engine |
Patch |
| @internal/testcontainers |
Patch |
| @internal/tracing |
Patch |
| @internal/zod-worker |
Patch |
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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Jan 6, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Walkthrough
This pull request enhances error handling for batch trigger operations, particularly in rate-limiting scenarios. It adds a changelog entry for a patch release, adjusts batch creation error response handling to clamp rate limit values to a minimum of 0, simplifies the BatchRateLimitExceededError message, and exposes the BatchTriggerError class as a public export in the SDK with additional diagnostic properties (isRateLimited, retryAfterMs, apiError, cause). The PR also includes a patch to the @upstash/ratelimit dependency that modifies token bucket logic to check if tokens are less than incrementBy rather than equal to zero. Package.json is updated to include the new dependency patch.
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name |
Status |
Explanation |
Resolution |
| Description check |
⚠️ Warning |
No pull request description was provided; required sections like Testing and Changelog are missing entirely. |
Add a pull request description following the template, including Testing steps, Changelog entry, and any relevant issue reference (Closes #). |
| Docstring Coverage |
⚠️ Warning |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. |
You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (1 passed)
| Check name |
Status |
Explanation |
| Title check |
✅ Passed |
The title clearly and specifically describes the main fix: preventing rate limiting from going negative with token bucket implementation. |
✨ Finishing touches
📜 Recent review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between edf5b14 and 53f7153.
⛔ Files ignored due to path filters (2)
pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
references/hello-world/src/trigger/batches.ts is excluded by !references/**
📒 Files selected for processing (7)
.changeset/sharp-ravens-doubt.md
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
package.json
packages/trigger-sdk/src/v3/index.ts
packages/trigger-sdk/src/v3/shared.ts
patches/@upstash__ratelimit.patch
🧰 Additional context used
📓 Path-based instructions (7)
packages/trigger-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Files:
packages/trigger-sdk/src/v3/index.ts
packages/trigger-sdk/src/v3/shared.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/trigger-sdk/src/v3/index.ts
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
packages/trigger-sdk/src/v3/shared.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/trigger-sdk/src/v3/index.ts
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
packages/trigger-sdk/src/v3/shared.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/trigger-sdk/src/v3/index.ts
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
package.json
packages/trigger-sdk/src/v3/shared.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp
Files:
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/routes/api.v3.batches.ts
apps/webapp/app/runEngine/concerns/batchLimits.server.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.trigger()` to trigger multiple different tasks at once from backend code
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
packages/trigger-sdk/src/v3/index.ts
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.trigger()` to trigger multiple different tasks at once from backend code
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The SDK at packages/trigger-sdk is an isomorphic TypeScript SDK
Applied to files:
packages/trigger-sdk/src/v3/index.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
packages/trigger-sdk/src/v3/index.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `tasks.trigger()` with type-only imports to trigger tasks from backend code without importing the task implementation
Applied to files:
packages/trigger-sdk/src/v3/index.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerByTask()` to batch trigger tasks by passing task instances for static task sets
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerAndWait()` to batch trigger multiple different tasks and wait for results
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
packages/trigger-sdk/src/v3/index.ts
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `yourTask.batchTrigger()` to trigger multiple runs of a task from inside another task
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerByTaskAndWait()` to batch trigger tasks by passing task instances and wait for results
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export every task, including subtasks
Applied to files:
packages/trigger-sdk/src/v3/index.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `yourTask.batchTriggerAndWait()` to batch trigger tasks and wait for all results from a parent task
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `tasks.batchTrigger()` to trigger multiple runs of a single task with different payloads
Applied to files:
packages/trigger-sdk/src/v3/index.ts
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `runs.subscribeToBatch()` to subscribe to changes for all runs in a batch
Applied to files:
.changeset/sharp-ravens-doubt.md
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
.changeset/sharp-ravens-doubt.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Control concurrency using the `queue` property with `concurrencyLimit` option
Applied to files:
packages/trigger-sdk/src/v3/shared.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
packages/trigger-sdk/src/v3/shared.ts
🧬 Code graph analysis (1)
packages/trigger-sdk/src/v3/shared.ts (2)
packages/trigger-sdk/src/v3/index.ts (3)
BatchTriggerError (4-4)
ApiError (29-29)
RateLimitError (36-36)
packages/core/src/v3/apiClient/errors.ts (2)
ApiError (3-106)
RateLimitError (143-163)
⏰ 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). (24)
- GitHub Check: Cursor Bugbot
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
.changeset/sharp-ravens-doubt.md (1)
1-5: LGTM! Changelog entry is clear and appropriate.
The changelog format is correct and the message accurately describes the user-facing improvement for batch trigger error handling during rate limiting.
apps/webapp/app/routes/api.v3.batches.ts (1)
148-169: LGTM! Defensive clamping prevents negative rate limit values in API response.
The addition of Math.max(0, error.remaining) on Line 161 is a good defensive practice that ensures the X-RateLimit-Remaining header never shows negative values, even if the underlying rate limiter temporarily returns a negative remainder. This complements the upstream fix in the @upstash/ratelimit patch.
packages/trigger-sdk/src/v3/index.ts (1)
4-4: LGTM! Public API expanded to include BatchTriggerError.
Adding BatchTriggerError to the public exports is a backward-compatible enhancement that allows SDK consumers to catch and inspect batch trigger errors with richer diagnostic information (isRateLimited, retryAfterMs, apiError, cause as mentioned in the PR summary).
apps/webapp/app/runEngine/concerns/batchLimits.server.ts (1)
111-121: LGTM! Error message simplified to focus on actionable information.
The simplified error message on Line 118 now focuses on the reset time rather than including potentially confusing details like remaining (which could be negative due to the bug this PR fixes) and itemCount. The constructor parameters remain unchanged, so the API handler can still access all properties to construct proper response headers with clamping applied.
package.json (1)
82-83: The @upstash/ratelimit patch file exists and contains the correct token bucket fix.
The new patch entry for @upstash/[email protected] is correctly formatted and aligns with the PR's goal of fixing negative token bucket values. The patch modifies the token bucket logic to check if tokens are less than incrementBy rather than equal to zero, applied to both dist/index.js and dist/index.mjs.
patches/@upstash__ratelimit.patch (1)
9-11: LGTM! Correct fix for token bucket boundary condition.
The change from tokens == 0 to tokens < incrementBy properly handles cases where incrementBy > 1. The old check would allow consumption when 0 < tokens < incrementBy, causing negative token counts. This fix ensures the rate limiter returns early when insufficient tokens are available.
Also applies to: 22-24
packages/trigger-sdk/src/v3/shared.ts (3)
5-5: LGTM!
Import additions for ApiError and RateLimitError are correctly placed and necessary for the enhanced error handling in BatchTriggerError.
Also applies to: 21-21
1599-1646: LGTM! Well-structured rate limit detection.
The BatchTriggerError enhancements correctly:
- Prioritize
RateLimitError instanceof check to extract millisecondsUntilReset
- Fall back to status code check for other
ApiError instances
- Expose useful diagnostic properties (
isRateLimited, retryAfterMs, apiError) for consumer retry logic
1649-1678: LGTM!
The buildBatchErrorMessage helper correctly prioritizes error types for message formatting, using Math.ceil appropriately for human-readable retry timing.
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.
❤️ Share
Comment @coderabbitai help to get the list of available commands and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Review Complete
Your review story is ready!
View Story
Comment !reviewfast on this PR to re-generate the story.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|