daemon: skip malformed/empty DevToolsActivePort instead of crashing#456
Open
thomwolf wants to merge 4 commits into
Open
daemon: skip malformed/empty DevToolsActivePort instead of crashing#456thomwolf wants to merge 4 commits into
thomwolf wants to merge 4 commits into
Conversation
Chrome 148+ silently ignores --remote-debugging-port when the user-data-dir is the default Chrome profile. Workaround is a dedicated profile dir (cloned from the user's main profile to preserve logins) launched headless. This adds that dir to PROFILES so the harness can discover the live port without needing BU_CDP_WS set explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Walks through the three non-obvious things needed to run the harness unattended against a real (logged-in) profile: the Chrome 148+ --user-data-dir requirement, that --headless=new doesn't write DevToolsActivePort, and the ditto-clone-then-strip-Singleton trick to preserve logins. Includes the launcher-script + launchd plist used in production. Two new gotchas in SKILL.md point at the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
get_ws_url() iterates PROFILES and parses each DevToolsActivePort with
`.split("\n", 1)`. An empty or single-line file (e.g. a stale Chrome-CDP
profile left by a prior run) returns <2 values, raising ValueError. That
error wasn't caught, so a single malformed file aborted the whole
discovery loop before reaching a valid profile — surfacing as the cryptic
"fatal: not enough values to unpack (expected 2, got 1)".
Catch ValueError alongside FileNotFoundError/NotADirectoryError so a bad
profile is skipped and discovery falls through to the next one (and
ultimately to the actionable "DevToolsActivePort not found" message).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Skill review passedReviewed 5 file(s) — no findings. |
Contributor
There was a problem hiding this comment.
1 issue found across 6 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="daemon.py">
<violation number="1" location="daemon.py:66">
P2: The `except ValueError` guard is incomplete: a two-line `DevToolsActivePort` with a non-numeric port (e.g. `abc\n/devtools/browser/...`) passes the `split()` step, then later crashes at `int(port.strip())` inside the inner `try` that only catches `OSError`. This still aborts profile fallback, defeating the PR's goal of skipping all malformed files.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| port, path = (base / "DevToolsActivePort").read_text().strip().split("\n", 1) | ||
| except (FileNotFoundError, NotADirectoryError): | ||
| continue | ||
| except ValueError: |
Contributor
There was a problem hiding this comment.
P2: The except ValueError guard is incomplete: a two-line DevToolsActivePort with a non-numeric port (e.g. abc\n/devtools/browser/...) passes the split() step, then later crashes at int(port.strip()) inside the inner try that only catches OSError. This still aborts profile fallback, defeating the PR's goal of skipping all malformed files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon.py, line 66:
<comment>The `except ValueError` guard is incomplete: a two-line `DevToolsActivePort` with a non-numeric port (e.g. `abc\n/devtools/browser/...`) passes the `split()` step, then later crashes at `int(port.strip())` inside the inner `try` that only catches `OSError`. This still aborts profile fallback, defeating the PR's goal of skipping all malformed files.</comment>
<file context>
@@ -62,6 +63,9 @@ def get_ws_url():
port, path = (base / "DevToolsActivePort").read_text().strip().split("\n", 1)
except (FileNotFoundError, NotADirectoryError):
continue
+ except ValueError:
+ # malformed/empty DevToolsActivePort (stale profile) — skip, try next
+ continue
</file context>
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
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.
Problem
get_ws_url()iteratesPROFILESand parses eachDevToolsActivePortwith.split("\n", 1). An empty or single-line file — e.g. a staleChrome-CDPprofile left behind by a prior run — returns fewer than 2 values, raisingValueError.That exception wasn't caught, so a single malformed file aborted the entire discovery loop before reaching a valid profile. It surfaced as the cryptic daemon-fatal:
This is easy to hit because
Chrome-CDPis the first entry inPROFILES(added in a9ba3b9), so a leftover empty port file there shadows the user's real, working Chrome profile.Fix
Catch
ValueErroralongsideFileNotFoundError/NotADirectoryErrorso a malformed profile is skipped and discovery falls through to the next one — and ultimately to the actionableDevToolsActivePort not found — enable chrome://inspect...message.Test