add diagnostics report#552
Merged
siddharthvaddem merged 1 commit intosiddharthvaddem:mainfrom May 8, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds end-to-end diagnostic save capability: IPC handler collects system metadata and writes JSON diagnostics to disk via save dialog; preload bridge exposes the API to renderers; TypeScript types define the contract; SettingsPanel conditionally renders a "Save Diagnostics" button wired through VideoEditor to capture current error state and editor snapshot. ChangesDiagnostic Save Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
electron/ipc/handlers.ts (1)
1328-1332: ⚡ Quick winnit: reuse
buildDialogOptionshere for consistency.Small thing, but using the shared dialog helper keeps parent-window behavior aligned with the rest of the IPC handlers and avoids drift later.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ipc/handlers.ts` around lines 1328 - 1332, The save dialog call uses an inline options object instead of the shared helper; replace the inline options passed to dialog.showSaveDialog in the handler (the block creating defaultPath "openscreen-diagnostic-...") with a call to buildDialogOptions(...) so it returns the same options shape (pass title, defaultPath, and filters into buildDialogOptions) and then pass that result to dialog.showSaveDialog to preserve consistent parent-window behavior across IPC handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 1322-1361: Wrap the entire "save-diagnostic" ipcMain.handle
handler body in a top-level try/catch so any exceptions (e.g. from
dialog.showSaveDialog) are caught and the IPC always returns the expected object
shape; inside the catch, log the error (console.error) and return an object like
{ success: false, error: String(error), canceled: false } (or include canceled
if relevant) instead of letting the promise reject. Keep the existing inner
try/catch for fs.writeFile but move/encapsulate everything
(dialog.showSaveDialog, diagnostic construction, and writeFile call) into the
outer try block in the "save-diagnostic" handler to ensure callers always
receive a consistent response.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1688-1697: The hardcoded button label "Save Diagnostics" in
SettingsPanel should be replaced with a localized string; locate the button
rendered when onSaveDiagnostic is present (the <button> using FileDown and
onSaveDiagnostic) and call the project's i18n helper (e.g., t('...') or
useTranslation/Trans) instead of the literal text, and add a matching locale key
like "settings.saveDiagnostics" to the translation files so the label is
translated consistently across locales.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1733-1744: The hardcoded strings in handleSaveDiagnostic (the
error fallback "Manual diagnostic export" and the toast messages passed to
toast.success/toast.error) must be localized: import or use the project's
translation helper (e.g., useTranslation()/t) and replace the literal strings
passed to window.electronAPI.saveDiagnostic (error), toast.success, and
toast.error with translated keys (e.g., t('diagnostics.manualExport'),
t('diagnostics.saveSuccess'), t('diagnostics.saveFailure')); keep exportError
fallback logic but use t(...) for the default message and update any existing
translation files with the new keys.
---
Nitpick comments:
In `@electron/ipc/handlers.ts`:
- Around line 1328-1332: The save dialog call uses an inline options object
instead of the shared helper; replace the inline options passed to
dialog.showSaveDialog in the handler (the block creating defaultPath
"openscreen-diagnostic-...") with a call to buildDialogOptions(...) so it
returns the same options shape (pass title, defaultPath, and filters into
buildDialogOptions) and then pass that result to dialog.showSaveDialog to
preserve consistent parent-window behavior across IPC handlers.
🪄 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: 704e031e-624e-42e0-9002-cf8103020736
📒 Files selected for processing (5)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/preload.tssrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsx
Comment on lines
+1322
to
+1361
| ipcMain.handle( | ||
| "save-diagnostic", | ||
| async ( | ||
| _, | ||
| payload: { error: string; stack?: string; projectState: unknown; logs: string[] }, | ||
| ) => { | ||
| const { filePath, canceled } = await dialog.showSaveDialog({ | ||
| title: "Save Diagnostic File", | ||
| defaultPath: `openscreen-diagnostic-${Date.now()}.json`, | ||
| filters: [{ name: "JSON", extensions: ["json"] }], | ||
| }); | ||
|
|
||
| if (canceled || !filePath) return { success: false, canceled: true }; | ||
|
|
||
| const diagnostic = { | ||
| timestamp: new Date().toISOString(), | ||
| appVersion: app.getVersion(), | ||
| platform: process.platform, | ||
| arch: process.arch, | ||
| osRelease: os.release(), | ||
| osVersion: os.version(), | ||
| totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024), | ||
| nodeVersion: process.versions.node, | ||
| electronVersion: process.versions.electron, | ||
| chromeVersion: process.versions.chrome, | ||
| error: payload.error, | ||
| stack: payload.stack, | ||
| projectState: payload.projectState, | ||
| recentLogs: payload.logs, | ||
| }; | ||
|
|
||
| try { | ||
| await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8"); | ||
| return { success: true, path: filePath }; | ||
| } catch (error) { | ||
| console.error("Failed to write diagnostic file:", error); | ||
| return { success: false, error: String(error) }; | ||
| } | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Guard the whole diagnostic IPC flow with a top-level try/catch.
Right now, failures from showSaveDialog can reject the IPC call instead of returning the expected object shape. that’s kinda cursed for caller handling paths.
💡 Suggested patch
ipcMain.handle(
"save-diagnostic",
async (
_,
payload: { error: string; stack?: string; projectState: unknown; logs: string[] },
) => {
- const { filePath, canceled } = await dialog.showSaveDialog({
- title: "Save Diagnostic File",
- defaultPath: `openscreen-diagnostic-${Date.now()}.json`,
- filters: [{ name: "JSON", extensions: ["json"] }],
- });
-
- if (canceled || !filePath) return { success: false, canceled: true };
-
- const diagnostic = {
- timestamp: new Date().toISOString(),
- appVersion: app.getVersion(),
- platform: process.platform,
- arch: process.arch,
- osRelease: os.release(),
- osVersion: os.version(),
- totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024),
- nodeVersion: process.versions.node,
- electronVersion: process.versions.electron,
- chromeVersion: process.versions.chrome,
- error: payload.error,
- stack: payload.stack,
- projectState: payload.projectState,
- recentLogs: payload.logs,
- };
-
try {
+ const { filePath, canceled } = await dialog.showSaveDialog({
+ title: "Save Diagnostic File",
+ defaultPath: `openscreen-diagnostic-${Date.now()}.json`,
+ filters: [{ name: "JSON", extensions: ["json"] }],
+ });
+
+ if (canceled || !filePath) return { success: false, canceled: true };
+
+ const diagnostic = {
+ timestamp: new Date().toISOString(),
+ appVersion: app.getVersion(),
+ platform: process.platform,
+ arch: process.arch,
+ osRelease: os.release(),
+ osVersion: os.version(),
+ totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024),
+ nodeVersion: process.versions.node,
+ electronVersion: process.versions.electron,
+ chromeVersion: process.versions.chrome,
+ error: payload.error,
+ stack: payload.stack,
+ projectState: payload.projectState,
+ recentLogs: payload.logs,
+ };
+
await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8");
return { success: true, path: filePath };
} catch (error) {
console.error("Failed to write diagnostic file:", error);
return { success: false, error: String(error) };
}
},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ipcMain.handle( | |
| "save-diagnostic", | |
| async ( | |
| _, | |
| payload: { error: string; stack?: string; projectState: unknown; logs: string[] }, | |
| ) => { | |
| const { filePath, canceled } = await dialog.showSaveDialog({ | |
| title: "Save Diagnostic File", | |
| defaultPath: `openscreen-diagnostic-${Date.now()}.json`, | |
| filters: [{ name: "JSON", extensions: ["json"] }], | |
| }); | |
| if (canceled || !filePath) return { success: false, canceled: true }; | |
| const diagnostic = { | |
| timestamp: new Date().toISOString(), | |
| appVersion: app.getVersion(), | |
| platform: process.platform, | |
| arch: process.arch, | |
| osRelease: os.release(), | |
| osVersion: os.version(), | |
| totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024), | |
| nodeVersion: process.versions.node, | |
| electronVersion: process.versions.electron, | |
| chromeVersion: process.versions.chrome, | |
| error: payload.error, | |
| stack: payload.stack, | |
| projectState: payload.projectState, | |
| recentLogs: payload.logs, | |
| }; | |
| try { | |
| await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8"); | |
| return { success: true, path: filePath }; | |
| } catch (error) { | |
| console.error("Failed to write diagnostic file:", error); | |
| return { success: false, error: String(error) }; | |
| } | |
| }, | |
| ); | |
| ipcMain.handle( | |
| "save-diagnostic", | |
| async ( | |
| _, | |
| payload: { error: string; stack?: string; projectState: unknown; logs: string[] }, | |
| ) => { | |
| try { | |
| const { filePath, canceled } = await dialog.showSaveDialog({ | |
| title: "Save Diagnostic File", | |
| defaultPath: `openscreen-diagnostic-${Date.now()}.json`, | |
| filters: [{ name: "JSON", extensions: ["json"] }], | |
| }); | |
| if (canceled || !filePath) return { success: false, canceled: true }; | |
| const diagnostic = { | |
| timestamp: new Date().toISOString(), | |
| appVersion: app.getVersion(), | |
| platform: process.platform, | |
| arch: process.arch, | |
| osRelease: os.release(), | |
| osVersion: os.version(), | |
| totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024), | |
| nodeVersion: process.versions.node, | |
| electronVersion: process.versions.electron, | |
| chromeVersion: process.versions.chrome, | |
| error: payload.error, | |
| stack: payload.stack, | |
| projectState: payload.projectState, | |
| recentLogs: payload.logs, | |
| }; | |
| await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8"); | |
| return { success: true, path: filePath }; | |
| } catch (error) { | |
| console.error("Failed to write diagnostic file:", error); | |
| return { success: false, error: String(error) }; | |
| } | |
| }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/handlers.ts` around lines 1322 - 1361, Wrap the entire
"save-diagnostic" ipcMain.handle handler body in a top-level try/catch so any
exceptions (e.g. from dialog.showSaveDialog) are caught and the IPC always
returns the expected object shape; inside the catch, log the error
(console.error) and return an object like { success: false, error:
String(error), canceled: false } (or include canceled if relevant) instead of
letting the promise reject. Keep the existing inner try/catch for fs.writeFile
but move/encapsulate everything (dialog.showSaveDialog, diagnostic construction,
and writeFile call) into the outer try block in the "save-diagnostic" handler to
ensure callers always receive a consistent response.
Comment on lines
+1688
to
+1697
| {onSaveDiagnostic && ( | ||
| <button | ||
| type="button" | ||
| onClick={onSaveDiagnostic} | ||
| className="flex-1 flex items-center justify-center gap-1.5 text-[10px] text-slate-500 hover:text-slate-300 py-1.5 transition-colors" | ||
| > | ||
| <FileDown className="w-3 h-3 text-slate-400" /> | ||
| Save Diagnostics | ||
| </button> | ||
| )} |
Contributor
There was a problem hiding this comment.
Button label should go through i18n.
Save Diagnostics is currently hardcoded, so this one string won’t localize with the rest of the panel.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/SettingsPanel.tsx` around lines 1688 - 1697, The
hardcoded button label "Save Diagnostics" in SettingsPanel should be replaced
with a localized string; locate the button rendered when onSaveDiagnostic is
present (the <button> using FileDown and onSaveDiagnostic) and call the
project's i18n helper (e.g., t('...') or useTranslation/Trans) instead of the
literal text, and add a matching locale key like "settings.saveDiagnostics" to
the translation files so the label is translated consistently across locales.
Comment on lines
+1733
to
+1744
| const handleSaveDiagnostic = useCallback(async () => { | ||
| const result = await window.electronAPI.saveDiagnostic({ | ||
| error: exportError ?? "Manual diagnostic export", | ||
| projectState: editorState, | ||
| logs: [], | ||
| }); | ||
| if (result.success) { | ||
| toast.success("Diagnostic file saved"); | ||
| } else if (!result.canceled) { | ||
| toast.error("Failed to save diagnostic file"); | ||
| } | ||
| }, [exportError, editorState]); |
Contributor
There was a problem hiding this comment.
Please localize the new diagnostic strings.
Right now "Manual diagnostic export" and the success/failure toasts are hardcoded English. lowkey breaks localization consistency in an otherwise fully translated screen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/VideoEditor.tsx` around lines 1733 - 1744, The
hardcoded strings in handleSaveDiagnostic (the error fallback "Manual diagnostic
export" and the toast messages passed to toast.success/toast.error) must be
localized: import or use the project's translation helper (e.g.,
useTranslation()/t) and replace the literal strings passed to
window.electronAPI.saveDiagnostic (error), toast.success, and toast.error with
translated keys (e.g., t('diagnostics.manualExport'),
t('diagnostics.saveSuccess'), t('diagnostics.saveFailure')); keep exportError
fallback logic but use t(...) for the default message and update any existing
translation files with the new keys.
siddharthvaddem
approved these changes
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Description
This adds in a a way to send in a diagnostics report (Will add a way to buffer and logger in later pr)
Motivation
Related Issue(s)
Screenshots / Video
Screenshot (if applicable):
Video (if applicable):
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit
New Features