fix: wrap JSON.parse in try/catch and fix maxAttempts undefined check#4357
fix: wrap JSON.parse in try/catch and fix maxAttempts undefined check#4357PedroHenrique0713 wants to merge 1 commit into
Conversation
…pts null check fix(ioSerialization): wrap JSON.parse in try/catch for malformed JSON. Malformed or truncated JSON payloads in parsePacket/parsePacketAsJson threw unhandled promise rejections, breaking the stream pipeline. fix(apiClient): guard maxAttempts against undefined in rate-limit check. Non-null assertion bypassed silently when maxAttempts was undefined, allowing infinite retries on HTTP 429 responses.
|
|
Hi @PedroHenrique0713, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // Retry on rate limits with special handling for Retry-After | ||
| if (response.status === 429) { | ||
| if (attempt >= retryOptions.maxAttempts!) { | ||
| if (retryOptions.maxAttempts !== undefined && attempt >= retryOptions.maxAttempts) { |
There was a problem hiding this comment.
🟡 Missing release note entry for a change to a published package
This pull request modifies a published package but does not include the required changeset entry (change in packages/core/src/v3/apiClient/index.ts:2378), so the fix will not appear in the user-visible release notes.
Impact: The bug fixes ship without any changelog/release-note entry and the package version will not be bumped.
Repository changeset rule
AGENTS.md and CONTRIBUTING.md both state that any change to a public package (packages/*) must include a changeset (pnpm run changeset:add). This PR changes packages/core/src/v3/apiClient/index.ts and packages/core/src/v3/utils/ioSerialization.ts but adds no file under .changeset/.
Prompt for agents
This PR modifies the published package @trigger.dev/core (packages/core) but does not include a changeset. Per AGENTS.md and CONTRIBUTING.md, any change to a public package under packages/* requires a changeset. Run pnpm run changeset:add and select a patch bump for @trigger.dev/core with a user-facing description of the two bug fixes (JSON.parse now returns undefined on malformed payloads instead of throwing, and the rate-limit retry guard no longer allows unbounded retries when maxAttempts is undefined).
Was this helpful? React with 👍 or 👎 to provide feedback.
| case "application/super+json": | ||
| return superjson.parse(value.data); |
There was a problem hiding this comment.
🔍 super+json parse path remains unwrapped despite PR claiming both parse functions are fixed
The PR description states malformed/truncated payloads caused unhandled rejections in parsePacket/parsePacketAsJson. The fix only wraps the application/json case in try/catch. The application/super+json case (superjson.parse(value.data) at packages/core/src/v3/utils/ioSerialization.ts:40 and :68) can also throw on malformed data and remains uncaught. If super+json packets can be truncated/corrupted the same way, this fix is incomplete and those paths will still reject. Worth confirming whether super+json payloads share the same corruption risk.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated stream batch retry logic to safely handle an undefined maxAttempts value. Added error handling to application/json parsing in parsePacket and parsePacketAsJson so invalid JSON returns undefined instead of throwing. Other packet data types retain their existing behavior. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Summary
Two bug fixes in Trigger.dev core package.
1. Unhandled JSON.parse rejection in
parsePacketandparsePacketAsJsoncalledJSON.parsewithout try/catch on payloads. Malformed/truncated JSON threw unhandled promise rejections, breaking the stream pipeline. Fix: wrap in try/catch, returnundefinedon parse failure.2. NaN comparison bypasses rate-limit guard in
apiClientretryOptions.maxAttempts!used non-null assertion but the type allowsundefined. When undefined,attempt >= undefinedevaluates toNaN(always falsy), silently allowing infinite retries on HTTP 429 responses. Fix: check for undefined before comparing.