-
-
Notifications
You must be signed in to change notification settings - Fork 978
Fix Replay Data Corruption for Non-JSON Payloads (#2813) #2959
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.
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
base: main
Are you sure you want to change the base?
Fix Replay Data Corruption for Non-JSON Payloads (#2813) #2959
Conversation
This fix removes the unintended Zod transformation and JSON enforcement in the ReplayRunData schema. It also refines the UI editability rules to be storage-based (not MIME-based) and adds a 512KB safety threshold for browser performance.
|
WalkthroughThe PR modifies replay functionality across two files. In ReplayRunDialog.tsx, the payload editability logic is enhanced to differentiate between payloads with large types (application/store) and those exceeding size limits (512KB), with conditional UI tooltips and tab defaulting behavior. In replayTask.ts, the validation schema is simplified by removing JSON validation from the payload field while maintaining and refining metadata validation with updated error messaging. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-12-08T15:19:56.823ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2025-11-27T16:26:37.432ZApplied to files:
⏰ 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). (22)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (1)
38-38: Remove unused import.The
divideimport fromeffect/Durationis not used anywhere in this file.🧹 Suggested fix
-import { divide } from "effect/Duration";
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.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/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein 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 webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/v3/replayTask.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/replayTask.ts
🧠 Learnings (6)
📓 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 `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
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} : Generate example payloads for tasks when possible
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx
📚 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:
apps/webapp/app/v3/replayTask.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} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
apps/webapp/app/v3/replayTask.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 metadata methods (set, del, replace, append, remove, increment, decrement, stream, flush) to update metadata during task execution
Applied to files:
apps/webapp/app/v3/replayTask.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/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
apps/webapp/app/v3/replayTask.ts
⏰ 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). (22)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 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 (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (3)
apps/webapp/app/v3/replayTask.ts (1)
4-28: LGTM! Schema change correctly addresses the payload corruption bug.The removal of the JSON-parsing transformation on
payloadensures non-JSON payloads are treated as opaque strings and passed through without mutation. The metadata transform retains its existing behavior with improved error messaging.apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (2)
165-169: Well-structured editability logic.The separation between storage-based (
isLargePayload) and size-based (isTooLargeToEdit) restrictions provides clear semantics and enables appropriate UI messaging. The 512 KB limit is reasonable for in-browser editing, and defaulting to the metadata tab when payload isn't editable is good UX.
273-290: LGTM! Clear conditional messaging for non-editable payloads.The differentiated tooltip text correctly distinguishes between storage-based large payloads (with documentation link) and payloads that simply exceed the in-browser edit limit. This aligns with the PR objective to show the "large payload" documentation link only for runs using external storage.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Summary
This PR fixes a data corruption bug in the run replay flow where non-JSON payloads (e.g., text/plain strings) were being mutated into {}. It ensures that payloads are handled as opaque strings throughout the replay path and improves UI editability rules based on storage semantics.
Problems Corrected
Empty Payload on Replay: Replaying a run with a non-JSON payload (e.g., text/plain strings) without overrides resulted in data corruption, defaulting the payload to {}.
Incorrect UI Label: The Replay modal misclassified non-JSON payloads as "large payloads" and blocked editing.
Technical Changes
Payload-Agnostic Replay Schema
Modified
apps/webapp/app/v3/replayTask.ts
to remove an unintended Zod transformation that forced JSON parsing and defaulted missing payloads to {}. The schema now treats payloads as opaque strings passed through without transformation, preserving data integrity when replaying without overrides.
Robust UI Editability Rules
Updated
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx
to determine editability based on system constraints rather than MIME type assumptions:
Storage-based: Payloads are editable unless stored externally (application/store).
Size-limited: Added a 512KB limit (safe threshold for in-browser editing) to prevent accidental rendering of multi-MB payloads.
Improved Classification: The "large payload" tooltip and documentation link now only appear for runs that actually use external storage.
Verification
Data Integrity Invariants
Replay Symmetry: Replaying a run without modifying the payload produces a semantically identical run input to the original execution.
No-Override Preservation: Verified that replaying a run without specifying a payload override correctly preserves the original payload instead of defaulting to {}.
Opaque String Handling: Replaying a string payload that "looks like JSON" (e.g., "{not: 'json'}") is preserved as a string throughout the path, proving no accidental re-parsing occurs.
Automated Test Results
Validated via reproduction script:
Missing Payload: Preserved as undefined (Correct)
String Payload: Accepted verbatim (Correct)
Looks-like-JSON: Preserved as string (Correct)
prettyPrintPacket
: Returns verbatim for text/plain types.