Don't let an unreadable profile dir abort CDP discovery - #621
Don't let an unreadable profile dir abort CDP discovery#621eddieberklee wants to merge 1 commit into
Conversation
get_ws_url() and _ws_from_devtools_active_port() read DevToolsActivePort
out of each candidate profile and skip the profile on FileNotFoundError or
NotADirectoryError. On macOS the read can also fail with PermissionError:
TCC guards ~/Library/Application Support/Google/Chrome, so any terminal
without Full Disk Access gets EPERM rather than ENOENT.
That escaped as a fatal traceback:
browser-harness: fatal: [Errno 1] Operation not permitted:
'/Users/<user>/Library/Application Support/Google/Chrome/DevToolsActivePort'
The daemon died before reaching its own fallbacks -- the 9222/9223 probe,
the remote_debugging_user_enabled() hint, and the "start Chrome" error --
so the user saw a raw errno instead of an actionable message, even when a
reachable browser was running on another port.
Treat an unreadable profile like a missing one and keep looking.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/browser_harness/daemon.py">
<violation number="1" location="src/browser_harness/daemon.py:205">
P2: In the get_ws_url discovery loop, when every profile is unreadable (the PR's FDA-less scenario) the new PermissionError catch skips each profile, but the immediately-following liveness gate then misreports the browser as not running. `supported_browser_running()` → `browser_running_for_profile()` reads `SingletonLock` and also gets EPERM/OSError under the same TCC protection, returning False, so get_ws_url raises "chrome-not-running" before ever reaching the 9222/9223 probe or the final "enable chrome://inspect" error the PR claims the fix unblocks. So a user whose browser is genuinely running on another port gets a misleading "not running" error instead of the probe. The added comment "Skipping lets the 9222/9223 probe below still run" is therefore inaccurate for the all-profiles-blocked case. Consider gating the liveness check on actually scanning at least one readable profile, or falling through to the probe when every profile is unreadable.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| # PermissionError: macOS TCC guards ~/Library/Application Support/Google/Chrome, | ||
| # so a terminal without Full Disk Access gets EPERM here. Skip the profile and | ||
| # keep looking rather than killing discovery outright. | ||
| except (FileNotFoundError, NotADirectoryError, PermissionError): |
There was a problem hiding this comment.
P2: In the get_ws_url discovery loop, when every profile is unreadable (the PR's FDA-less scenario) the new PermissionError catch skips each profile, but the immediately-following liveness gate then misreports the browser as not running. supported_browser_running() → browser_running_for_profile() reads SingletonLock and also gets EPERM/OSError under the same TCC protection, returning False, so get_ws_url raises "chrome-not-running" before ever reaching the 9222/9223 probe or the final "enable chrome://inspect" error the PR claims the fix unblocks. So a user whose browser is genuinely running on another port gets a misleading "not running" error instead of the probe. The added comment "Skipping lets the 9222/9223 probe below still run" is therefore inaccurate for the all-profiles-blocked case. Consider gating the liveness check on actually scanning at least one readable profile, or falling through to the probe when every profile is unreadable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/daemon.py, line 205:
<comment>In the get_ws_url discovery loop, when every profile is unreadable (the PR's FDA-less scenario) the new PermissionError catch skips each profile, but the immediately-following liveness gate then misreports the browser as not running. `supported_browser_running()` → `browser_running_for_profile()` reads `SingletonLock` and also gets EPERM/OSError under the same TCC protection, returning False, so get_ws_url raises "chrome-not-running" before ever reaching the 9222/9223 probe or the final "enable chrome://inspect" error the PR claims the fix unblocks. So a user whose browser is genuinely running on another port gets a misleading "not running" error instead of the probe. The added comment "Skipping lets the 9222/9223 probe below still run" is therefore inaccurate for the all-profiles-blocked case. Consider gating the liveness check on actually scanning at least one readable profile, or falling through to the probe when every profile is unreadable.</comment>
<file context>
@@ -199,7 +199,10 @@ def _ws_from_devtools_active_port(http_url: str) -> str | None:
+ # PermissionError: macOS TCC guards ~/Library/Application Support/Google/Chrome,
+ # so a terminal without Full Disk Access gets EPERM here. Skip the profile and
+ # keep looking rather than killing discovery outright.
+ except (FileNotFoundError, NotADirectoryError, PermissionError):
continue
port = active[0].strip() if active else ""
</file context>
On macOS,
~/Library/Application Support/Google/Chromeis TCC-protected. A terminal without Full Disk Access getsPermissionError(EPERM) readingDevToolsActivePort— notFileNotFoundError.Both read sites skip a profile on
FileNotFoundError, NotADirectoryErroronly, so EPERM escaped as a fatal traceback:The daemon died before reaching any of its own fallbacks — the 9222/9223 probe, the
remote_debugging_user_enabled()hint, and the final "enable chrome://inspect" error. A user with a perfectly reachable browser on another port just saw a raw errno.Change
Add
PermissionErrorto bothexcepttuples, so an unreadable profile is skipped like a missing one and discovery continues. No behavior change when the read succeeds.Tests
Two regression tests in
tests/unit/test_daemon.py, using a profile stub whoseread_textraisesPermissionError. Both fail onmainwithPermissionErrorand pass with the fix. They clearBU_CDP_WS/BU_CDP_URLso they do not depend on the ambient environment.Repro without the patch, on any macOS terminal lacking Full Disk Access:
Summary by cubic
Skips unreadable Chrome profile directories during CDP discovery so macOS TCC no longer aborts the daemon. Previously, reading
DevToolsActivePortcould raisePermissionErrorand stop discovery; now we treat it like a missing profile and continue. No change when the read succeeds.PermissionErrorin both_ws_from_devtools_active_portandget_ws_url, alongsideFileNotFoundErrorandNotADirectoryError.remote_debugging_user_enabled()hint, and the final "chrome-not-running" error instead of a raw errno.PermissionError; they clearBU_CDP_WS/BU_CDP_URLto avoid environment coupling.Written for commit 055384b. Summary will update on new commits.