Skip to content

fix(recording): recover from fullscreen countdown failures#313

Merged
webadderall merged 2 commits intowebadderallorg:mainfrom
meiiie:fix/fullscreen-countdown-recovery
Apr 22, 2026
Merged

fix(recording): recover from fullscreen countdown failures#313
webadderall merged 2 commits intowebadderallorg:mainfrom
meiiie:fix/fullscreen-countdown-recovery

Conversation

@meiiie
Copy link
Copy Markdown
Collaborator

@meiiie meiiie commented Apr 21, 2026

Description

Improves recovery when starting a recording from Windows fullscreen sessions. The patch keeps the countdown window from stealing focus on Windows, explicitly clears the recording-state latch when start-up fails, and restores the HUD through the tray recovery path instead of leaving the app alive but unusable.

Motivation

Issue #241 reports that starting a recording from fullscreen can finish the countdown, fail to begin recording, and then leave the app effectively stuck until it is force-closed.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

Related Issue(s)

Testing Guide

  • node ..\\Recordly\\node_modules\\typescript\\bin\\tsc --noEmit
  • node ..\\Recordly\\node_modules\\@biomejs\\biome\\bin\\biome lint electron\\main.ts electron\\windows.ts src\\hooks\\useScreenRecorder.ts
  • npm run build:win
  • Windows Electron repro for countdown focus behavior:
    • show() path switched focus from the fullscreen target to the countdown overlay
    • showInactive() kept the fullscreen target focused while leaving the overlay visible on top

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

Notes

This stays intentionally narrow and Windows-focused. I validated the countdown focus behavior locally with a minimal Electron fullscreen repro, but I still have not reproduced the exact multi-monitor Chrome setup from the issue report.

Summary by CodeRabbit

  • Bug Fixes
    • Improved window restoration on Windows: avoids unnecessary restores, ensures hidden/minimized windows become visible, and correctly adjusts z-order and activation.
    • Updated countdown window display on Windows to use inactive show mode for proper window management.
    • Enhanced recording failure handling to better reset recording state and ensure cleanup runs reliably.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 29cba2d2-1af8-4921-b0fe-5b73b6238170

📥 Commits

Reviewing files that changed from the base of the PR and between 92ee514 and 1e102b3.

📒 Files selected for processing (1)
  • src/hooks/useScreenRecorder.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/hooks/useScreenRecorder.ts

📝 Walkthrough

Walkthrough

Platform-specific window-restore and show behavior was adjusted for Windows: non-editor windows now trigger the HUD overlay instead of restoring; restore is performed only when minimized, visibility ensured with show(), and z-order/activation use moveTop() and focus(). Countdown window show behavior on Windows uses showInactive() plus moveTop(). Renderer error path now best-effort notifies main process to clear recording state (setRecordingState(false)) before cleanup.

Changes

Cohort / File(s) Summary
Window restore & activation
electron/main.ts
restoreWindowSafely(window): on Windows, non-editor windows trigger showHudOverlayFromTray() and return; otherwise restore only when minimized, call show() if not visible, then moveTop() and focus() to adjust z-order/activation.
Countdown window show behavior
electron/windows.ts
createCountdownWindow did-finish-load handler: on Windows use win.showInactive() then win.moveTop() instead of unconditional win.show(); non-Windows keep win.show().
Recording-state error handling
src/hooks/useScreenRecorder.ts
In startRecording failure path, attempt window.electronAPI?.setRecordingState(false) (try/catch, warn on failure) before performing cleanupCapturedMedia() and await stopWebcamRecorder() in a finally block to propagate state reset to main process.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • webadderall/Recordly#259 — Modifies startRecording in src/hooks/useScreenRecorder.ts; related to renderer-side recording error handling and IPC state reset.
  • webadderall/Recordly#264 — Related changes touching HUD/window visibility and z-order handling for overlay/countdown windows.

Poem

🐰 I hopped through code and peeked inside,
Windows whispers moved the pane aside,
I nudged the recorder's weary state,
Made overlays bloom and timers wait,
A tiny rabbit cheers the tidy tide.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main purpose of the PR: fixing recovery from fullscreen countdown failures when recording on Windows.
Description check ✅ Passed The description covers all major template sections: a clear description of the fix, detailed motivation linking to issue #241, appropriate bug fix categorization, related issues, a comprehensive testing guide, and a completed checklist.
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.

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

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 and usage tips.

@meiiie meiiie marked this pull request as ready for review April 21, 2026 13:56
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/hooks/useScreenRecorder.ts`:
- Around line 1302-1305: The IPC call to
window.electronAPI?.setRecordingState(false) can throw and currently prevents
cleanupCapturedMedia() and stopWebcamRecorder() from running; refactor the block
so setRecording(false) remains (to update UI), then call await
window.electronAPI?.setRecordingState(false) inside a try block and move
cleanupCapturedMedia() and await stopWebcamRecorder() into a finally block so
they always run even if the IPC rejects; refer to setRecording,
setRecordingState, cleanupCapturedMedia, and stopWebcamRecorder to locate and
change the code.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c5c152d3-7cf6-42ff-9ced-c991fccc6723

📥 Commits

Reviewing files that changed from the base of the PR and between 4005687 and 92ee514.

📒 Files selected for processing (3)
  • electron/main.ts
  • electron/windows.ts
  • src/hooks/useScreenRecorder.ts

Comment thread src/hooks/useScreenRecorder.ts Outdated
@webadderall webadderall merged commit 6200113 into webadderallorg:main Apr 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants