Skip to content

Improve export failure diagnostics#536

Open
yusufm wants to merge 2 commits intosiddharthvaddem:mainfrom
yusufm:codex/export-diagnostics
Open

Improve export failure diagnostics#536
yusufm wants to merge 2 commits intosiddharthvaddem:mainfrom
yusufm:codex/export-diagnostics

Conversation

@yusufm
Copy link
Copy Markdown
Contributor

@yusufm yusufm commented May 3, 2026

Summary

  • show export failures with source, output size, codec, bitrate, and VideoEncoder availability
  • preserve multiline diagnostics in the export dialog error block
  • improve save-failure messaging for unsaved exports
  • fix the linted hook dependency used by CI

Testing

  • npm run lint
  • npx tsc --noEmit
  • npm run build-vite
  • npm run test:browser

Summary by CodeRabbit

  • Bug Fixes
    • Export error messages now preserve line breaks and whitespace for clearer readability.
    • Export failure flows show richer diagnostic details (codec, bitrate, dimensions, frame rate, and source info) to help identify problems.
    • Countdown/recorder cleanup improved to prevent lingering or stuck countdown overlays after recording.

@yusufm yusufm requested a review from siddharthvaddem as a code owner May 3, 2026 21:25
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5455cfc-8672-4a27-8591-5861fecd1fa4

📥 Commits

Reviewing files that changed from the base of the PR and between 156e9c1 and 559e97d.

📒 Files selected for processing (1)
  • src/components/video-editor/ExportDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/video-editor/ExportDialog.tsx

📝 Walkthrough

Walkthrough

This PR standardizes export/save error diagnostics and surfaces them in the UI, preserves multiline export error formatting, and memoizes a countdown-overlay cleanup callback used by the screen recorder. Changes touch export/error messaging in VideoEditor, ExportDialog rendering, and useScreenRecorder cleanup wiring.

Changes

Export & Recorder Error Diagnostics

Layer / File(s) Summary
Diagnostics Infrastructure
src/components/video-editor/VideoEditor.tsx
Adds ExportDiagnostics and helpers (getFileNameForDiagnostics, buildExportDiagnosticMessage, buildSaveDiagnosticMessage) to produce consistent multi-line diagnostic messages (source, dims/framerate, codec/bitrate, encoder availability).
Export Flow Integration
src/components/video-editor/VideoEditor.tsx
Replaces raw error strings with the structured diagnostic messages in GIF and MP4 export flows, unsaved-export save handling, and the export catch path; also converts thrown errors into the diagnostic format.
Dependency Update
src/components/video-editor/VideoEditor.tsx
handleExport callback deps now include videoSourcePath so diagnostics reference the current source file.
Error Display Formatting
src/components/video-editor/ExportDialog.tsx
Error paragraph now uses whitespace-pre-wrap break-words so multiline diagnostic messages preserve newlines and wrap correctly.
Recorder Cleanup Memoization
src/hooks/useScreenRecorder.ts
Introduces safeHideCountdownOverlay as a memoized useCallback (logs failures) and updates countdown effect cleanup to depend on this stable callback; removes previous inline definition.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • siddharthvaddem

"kinda cursed errors now speak in paragraphs,
preserved and tidy, no more one-line scars.
diagnostics lined up, and cleanup memoized—
lowkey neat, but nit: check encoder edge cases. 🚀"

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers key changes but is missing required template sections like Type of Change, Related Issue(s), and Screenshots/Video. Fill out missing template sections: specify the Type of Change checkbox, link any related issues, and add screenshots or videos if UI changes are involved.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: improving export failure diagnostics with detailed error information and formatting.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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
Review rate limit: 6/8 reviews remaining, refill in 15 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/video-editor/ExportDialog.tsx`:
- Line 164: The error paragraph in ExportDialog.tsx currently uses
"whitespace-pre-wrap" which preserves newlines but allows very long tokens to
overflow; update the <p> that renders the error (the node showing {error}) to
include the Tailwind word-break class (e.g., add "break-words") so long
paths/URLs wrap within the modal — keep existing classes and just append
"break-words" to the className for the error paragraph.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e5d7af31-b76f-43ba-972a-b0459a6b0196

📥 Commits

Reviewing files that changed from the base of the PR and between 7e00cdb and 156e9c1.

📒 Files selected for processing (3)
  • src/components/video-editor/ExportDialog.tsx
  • src/components/video-editor/VideoEditor.tsx
  • src/hooks/useScreenRecorder.ts

Comment thread src/components/video-editor/ExportDialog.tsx Outdated
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