fix: use atomic writes in FileExporter.export() to prevent corrupted trace files#646
fix: use atomic writes in FileExporter.export() to prevent corrupted trace files#646anandgupta42 merged 1 commit intomainfrom
Conversation
…d trace files
`FileExporter.export()` used `fs.writeFile` directly, while the snapshot mechanism
used atomic writes (tmp + rename). When concurrent sessions wrote to the same trace
file, a snapshot rename could race with a non-atomic writeFile, producing truncated
JSON ("Unterminated string" parse error).
Also adds temp file cleanup on failure, matching the existing `snapshot()` pattern.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
What does this PR do?
Fixes
FileExporter.export()to use atomic writes (write to temp file, then rename) instead of directfs.writeFile(). This prevents corrupted trace JSON when concurrent sessions or snapshots write to the same file.Root cause:
snapshot()already used atomic writes (tmp + rename), butexport()used non-atomicfs.writeFile(). When both targeted the same file concurrently, the rename could race with the writeFile, producing truncated JSON.Also adds temp file cleanup on failure (
fs.unlink(tmpPath)) to match the existingsnapshot()error handling pattern.Type of change
Issue for this PR
Closes #644
How did you verify your code works?
concurrent traces to same session ID overwrite cleanly)Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit