Skip to content

fix(core): fallback to safe string payload in stringifyIO when superjson fails - #4931

Closed
Tyagiquamar wants to merge 3 commits into
triggerdotdev:mainfrom
Tyagiquamar:fix-io-serialization-fallback
Closed

fix(core): fallback to safe string payload in stringifyIO when superjson fails#4931
Tyagiquamar wants to merge 3 commits into
triggerdotdev:mainfrom
Tyagiquamar:fix-io-serialization-fallback

Conversation

@Tyagiquamar

Copy link
Copy Markdown

IOPacket defines data?: string | undefined. When superjson.stringify(value) failed in stringifyIO(), the catch block previously returned { data: value, dataType: 'application/json' }. If value was an object, BigInt, or un-superjson-able data structure, packet.data received a raw object instead of a string, causing downstream Buffer.byteLength and offloading calculations to fail.

This fix updates the catch block of stringifyIO() to safely format value using JSON.stringify(value, makeSafeReplacer()) (falling back to String(value)), preserving the string type contract of IOPacket.data.

@changeset-bot

changeset-bot Bot commented Sep 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 74db548

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages
Name Type
@trigger.dev/sdk Patch
@trigger.dev/python Patch
@internal/dashboard-agent Patch
@trigger.dev/build Patch
trigger.dev 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/rbac Patch
@trigger.dev/sso Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/metrics-pipeline Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/run-store Patch
@internal/schedule-engine Patch
@internal/tracing Patch
@internal/webhook-engine Patch
@internal/webhook-sources Patch
@internal/testcontainers Patch
@internal/cache 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

@github-actions

Copy link
Copy Markdown
Contributor

Hi @Tyagiquamar, 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.

@github-actions github-actions Bot closed this Sep 13, 2026
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 233321e7-90c2-4093-81e8-1832ae1f6304

📥 Commits

Reviewing files that changed from the base of the PR and between 8b72e6c and 74db548.

📒 Files selected for processing (5)
  • .changeset/envvars-update-outside-task.md
  • packages/core/src/v3/utils/ioSerialization.test.ts
  • packages/core/src/v3/utils/ioSerialization.ts
  • packages/trigger-sdk/src/v3/envvars.test.ts
  • packages/trigger-sdk/src/v3/envvars.ts

Walkthrough

The changes fix envvars.update() argument resolution and missing-name validation in task and non-task contexts. They add tests for request payloads, defaults, and validation. The changes also update stringifyIO() to retry failed superjson serialization with a safe JSON replacer, then fall back to text. Tests cover these serialization paths.

Severity of issue fixed: Medium

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.


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.

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +100 to +102
const data = JSON.stringify(value, makeSafeReplacer());

return { data, dataType: "application/json" };

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.

🟡 Undefined fallback drops payloads

When fallback serialization returns undefined, stringifyIO emits an empty JSON packet. Parsing replaces the original value with undefined.

Learn more

JSON.stringify returns undefined rather than throwing for some top-level values. The returned IOPacket therefore has no data, and parsePacket treats it as an absent value. The outer fallback never runs because no exception occurred.

Example: A custom class instance rejected by SuperJSON can define toJSON() to return undefined. The JSON fallback then produces no data, and the receiver gets undefined instead of a textual representation.

Recommended fix: Treat an undefined result as a failed JSON serialization and return the existing text/plain representation.

Suggested change
const data = JSON.stringify(value, makeSafeReplacer());
return { data, dataType: "application/json" };
const data = JSON.stringify(value, makeSafeReplacer());
return data === undefined
? { data: String(value), dataType: "text/plain" }
: { data, dataType: "application/json" };
Devin Review

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

Comment on lines +1 to +5
---
"@trigger.dev/sdk": patch
---

`envvars.update()`: calling it outside a task run no longer throws `ReferenceError: name is not defined`. The variable name is now resolved from the positional arguments, matching the other env var methods, and a missing name raises a descriptive `name is required` error instead.

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.

🔍 Core fix lacks a changeset

The changeset covers only @trigger.dev/sdk. The user-visible @trigger.dev/core serialization fix receives no version bump or release note.

Devin Review

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

Comment on lines +21 to +28
it("fallback returns string data when superjson fails", async () => {
// Create an object where superjson.stringify throws or handles non-standard values
const cyclic: any = { name: "test" };
cyclic.self = cyclic;

const result = await stringifyIO(cyclic);
expect(typeof result.data).toBe("string");
});

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.

🔍 Fallback test misses fallback path

SuperJSON supports the cyclic fixture, so this test can pass through its normal path. The string-only assertion never verifies fallback behavior.

Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant