Simplify screen source selection to fix Wayland compatibility#2207
Simplify screen source selection to fix Wayland compatibility#2207IsmaelMartinez wants to merge 7 commits intomainfrom
Conversation
…humbnail on Wayland (#2204) The handleScreenSourceSelection function was calling desktopCapturer.getSources() a second time to re-verify the source already selected from the picker. On Wayland, this redundant call can fail or return mismatched source IDs, preventing both the thumbnail preview window from being created and screen sharing from starting. The source from the picker already has a valid ID from desktopCapturer.getSources() and can be used directly for both the thumbnail setup and the Electron callback. https://claude.ai/code/session_013t3ThsGo83tvcRQzc38dq8
Summary of ChangesHello @IsmaelMartinez, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness of screen sharing, particularly for users on Wayland, by refactoring the screen source selection process. The core change involves trusting the source object directly from the picker, thereby eliminating an unreliable Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
✅ Changelog entry generated and committed to this PR: The file You can edit it directly in this PR if needed. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the screen source selection logic to improve reliability on Wayland by removing a redundant desktopCapturer.getSources() call. The changes are clear and effectively address the described issue. However, I've identified a potential issue where error handling was removed during the simplification, which could lead to an unresponsive UI if an error occurs while setting up the screen share. I've provided a suggestion to re-introduce the error handling in a way that is consistent with the rest of the codebase.
Wrap setupScreenSharing/callback in try-catch to prevent hung UI if an error occurs during preview window creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📦 PR Snap Build Artifacts✅ Snap builds successful! Download artifacts: 🐧 Linux Snap Packagesx86_64 (107.26 MB) arm64 (104.18 MB) armv7l (98.41 MB) 📝 Note: Other package formats (.deb, .rpm, .AppImage, .dmg, .exe) are built in the main workflow |
📦 PR Build Artifacts✅ Build successful! Download artifacts: 🐧 Linuxx86_64 (433.99 MB) - Contains: .deb, .rpm, .tar.gz, .AppImage arm64 (425.23 MB) - Contains: .deb, .rpm, .tar.gz, .AppImage armv7l (403.28 MB) - Contains: .deb, .rpm, .tar.gz, .AppImage 🍎 macOSx86_64 (124.81 MB) - Contains: .dmg 🪟 Windowsx86_64 (105.41 MB) - Contains: .exe installer 📝 Note: Snap packages (.snap) are built in a separate workflow 🕐 Last updated: 2026-03-09 16:49 UTC |
* fix: prevent app crash when proxy connection fails (#2205) The top-level uncaughtException/unhandledRejection handlers added in v2.7.6 (security hardening phase 1) call process.exit(1) on any unhandled error. When a proxy is configured via config.json and the tunnel fails, window.loadURL() rejects with ERR_TUNNEL_CONNECTION_FAILED. This promise was not awaited/caught in ConnectionManager.refresh(), causing an unhandled rejection that triggered the fatal handler. Three changes fix this: - Await and catch the loadURL() promise in ConnectionManager.refresh() so proxy failures trigger a retry instead of an unhandled rejection - Expand the did-fail-load handler to recover from proxy/tunnel errors (ERR_TUNNEL_CONNECTION_FAILED, ERR_PROXY_CONNECTION_FAILED, etc.) in addition to the existing ERR_INTERNET_DISCONNECTED handling - Make the top-level error handlers skip process.exit(1) for known network error patterns as a safety net Closes #2205 https://claude.ai/code/session_01UDNMHvEP2TnazmHUU7uusz * chore: add changelog entry for PR #2206 * fix: address PR review comments for error handler robustness Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Key corrections from multi-agent review: - Confirm root cause via Chromium source: DesktopCapturer::IsRunningUnderWayland() checks XDG_SESSION_TYPE/WAYLAND_DISPLAY independently of --ozone-platform - Add Option F: --disable-features=WebRTCPipeWireCapturer when isX11Forced as the most targeted fix (short-circuits PipeWire before env var check) - Fix investigation step 1 location: getSources() failure is in service.js #handleGetDesktopCapturerSources, not handleScreenSourceSelection - Document sendScreenSharingStarted(null) preload guard bug dropping IPC events - Fix audit Near-Term Priority: setDisplayMediaRequestHandler already exists; --disable-features=WebRTCPipeWireCapturer is the correct next step - Soften #2222 characterization to require debug logs rather than speculation - Note PR #2207 (redundant getSources() removal) as correct complementary fix - Add Chromium source and Electron upstream issue references Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Took the incoming main version which adds handling for 'Object has been destroyed' and 'Script failed to execute' transient errors. https://claude.ai/code/session_013t3ThsGo83tvcRQzc38dq8
…window-issue-G9geO
This changelog was pulled in from a previous main merge and doesn't belong on this feature branch. https://claude.ai/code/session_013t3ThsGo83tvcRQzc38dq8
|
2 participants
Summary
Simplified the screen source selection flow by removing the redundant
desktopCapturer.getSources()call and using the source directly from the picker. This resolves reliability issues on Wayland wheredesktopCaptureris unreliable.Key Changes
desktopCapturerimport from electron dependenciesfindSelectedSource()helper functionhandleScreenSourceSelection()to directly use the source passed from the picker instead of re-querying all sourcesdesktopCapturerfailures (no longer needed)setImmediateretry logic that was working around the unreliable source lookupImplementation Details
The previous implementation would call
desktopCapturer.getSources()to retrieve all available sources, then search through them to find the one matching the user's selection. On Wayland, this call is unreliable and can fail, preventing the thumbnail window from being created.The new implementation trusts that the source object passed from the picker is already valid and uses it directly. This eliminates the problematic
desktopCapturercall while maintaining the same functionality.Fixes issue #2204.
https://claude.ai/code/session_013t3ThsGo83tvcRQzc38dq8