Skip to content

fix: wrap JSON.parse in try/catch and fix maxAttempts undefined check #4357

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.

Sign up for GitHub

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

Closed
PedroHenrique0713 wants to merge 1 commit into from
Closed

fix: wrap JSON.parse in try/catch and fix maxAttempts undefined check #4357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/v3/apiClient/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2375,7 +2375,7 @@ function shouldRetryStreamBatchItems(

// 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) {

@devin-ai-integration devin-ai-integration Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

return { retry: false };
}

Expand Down
12 changes: 10 additions & 2 deletions packages/core/src/v3/utils/ioSerialization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ export async function parsePacket(value: IOPacket, options?: ParsePacketOptions)

switch (value.dataType) {
case "application/json":
return JSON.parse(value.data, makeSafeReviver(options));
try {
return JSON.parse(value.data, makeSafeReviver(options));
} catch {
return undefined;
}
case "application/super+json":
return superjson.parse(value.data);
Comment on lines 39 to 40

@devin-ai-integration devin-ai-integration Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

case "text/plain":
Expand All @@ -55,7 +59,11 @@ export async function parsePacketAsJson(

switch (value.dataType) {
case "application/json":
return JSON.parse(value.data, makeSafeReviver(options));
try {
return JSON.parse(value.data, makeSafeReviver(options));
} catch {
return undefined;
}
case "application/super+json":
const superJsonResult = superjson.parse(value.data);

Expand Down