Conversation
Every attribute is read off browser_config, which defaults to None, so the
documented all-arguments form raised on construction:
ManagedBrowser(browser_type='chromium', user_data_dir=..., headless=True,
debugging_port=9222)
AttributeError: 'NoneType' object has no attribute 'browser_type'
That is the exact call BrowserProfiler.launch_builtin_browser makes, so
'crwl browser start' and 'crwl browser restart' fail before a browser is
launched. Where a config is passed as well, the arguments beside it are
ignored instead: browser_type='firefox', debugging_port=9333 came back as
chromium on 9222. browser_profiler.py shows both, one call site having
commented its arguments out and another still passing them.
Build a BrowserConfig from the arguments when none is given. A supplied
config still wins, so every existing call site behaves exactly as before, and
the docstring now says which of the two is the source of truth.
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.
Summary
ManagedBrowser.__init__documents eight arguments and then reads every attribute offbrowser_config, which defaults toNone:Two consequences, both reproducible:
Constructing it the documented way raises.
That is the exact call
BrowserProfiler.launch_builtin_browsermakes(
browser_profiler.py:1205), and it sits outside thetry:below it, so it propagates.crwl browser start(cli.py:717) andcrwl browser restart(cli.py:913) both go throughit, so neither can launch the builtin browser.
With a config, the arguments beside it are silently ignored.
browser_profiler.pyshows the confusion in the repository itself: the call at line 481 hasits arguments commented out next to
browser_config=, while the one at line 1020 stillpasses
user_data_dir,headlessanddebugging_portalongside a config — they do nothingthere, and only happen to agree with it.
List of files changed and why
crawl4ai/browser_manager.py— whenbrowser_configisNone, build one from thearguments. Nothing else changes: a supplied config is still used as-is, so every existing
call site (
browser_manager.py:777, the three inbrowser_profiler.py, and the tests intests/browser/) behaves exactly as before. The docstring now says which of the two wins.tests/unit/test_managed_browser_arguments.py— new.How Has This Been Tested?
tests/unit/test_managed_browser_arguments.py, 9 cases:launch_builtin_browseruses constructs, with its values intactbrowser_config— the control thatpins the existing call sites
8 of the 9 fail on
main; the one that passes is that control.Checklist: