feat: browserToolkit supports async #1886
Conversation
Added a BaseBrowser that supports asynchronous
Separate synchronous and asynchronous
Add new test code
Change both classes to asynchronous
Modified the asynchronous tool function name
To ensure that when determining whether a function is an asynchronous function, the original function is obtained instead of the packaged version
Support async function
examples/toolkits/browser_toolkit.py
Outdated
| print("Initializing browser...") | ||
| await browser.async_init() | ||
|
|
||
| # Test async_visit_page and async_wait_for_load (using GitHub as the test page) |
There was a problem hiding this comment.
The pre-commit failure is due to an E501 error, indicating that some lines are too long. Please shorten the lines to fit within the character limit. Also, run pre-commit run --all-files before committing and pushing your code to automatically fix formatting and import sorting issues.
There was a problem hiding this comment.
OK, I got it, thanks for reminding me!
examples/toolkits/browser_toolkit.py
Outdated
| ========================================================================== | ||
| Initializing browser... | ||
| Testing async_visit_page and async_wait_for_load: | ||
| Current page URL: https://github.com/camel-ai/camel | ||
| Testing async_click_id: | ||
| Click complete | ||
| Testing async_click_blank_area: | ||
| Successfully clicked the blank area | ||
| Testing async_get_screenshot: | ||
| Screenshot taken successfully, saved at: tmp/camel_0317152711.png | ||
| Testing async_capture_full_page_screenshots: | ||
| Full page screenshot file paths: ['tmp/camel_0317152711.png', 'tmp/camel_0317152712.png', 'tmp/camel_0317152713.png', 'tmp/camel_0317152713.png', 'tmp/camel_0317152714.png', 'tmp/camel_0317152715.png', 'tmp/camel_0317152715.png', 'tmp/camel_0317152716.png', 'tmp/camel_0317152716.png', 'tmp/camel_0317152717.png', 'tmp/camel_0317152718.png', 'tmp/camel_0317152718.png', 'tmp/camel_0317152719.png', 'tmp/camel_0317152720.png', 'tmp/camel_0317152720.png', 'tmp/camel_0317152721.png', 'tmp/camel_0317152721.png', 'tmp/camel_0317152722.png', 'tmp/camel_0317152723.png', 'tmp/camel_0317152723.png', 'tmp/camel_0317152724.png', 'tmp/camel_0317152724.png'] |
There was a problem hiding this comment.
are these expected output logs from running the test functions?
Fix "browse url" error when executing asynchronous tools: 'playwright' object has no attribute 'start'
|
hey @subway-jack , thanks for the contribution! I turned the status of this PR to draft, feel free to open this once it's ready for review~ |
Add asynchronous Browser case code
There was a problem hiding this comment.
thanks @subway-jack left some comments below
camel/toolkits/browser_toolkit.py
Outdated
| pages. | ||
| """ | ||
|
|
||
| class AsyncBaseBrowser: |
There was a problem hiding this comment.
maybe we can add an arg named async_mode in BaseBrowser，and no need to add a new class，add asynchronous methods based on the original functions
camel/toolkits/browser_toolkit.py
Outdated
|
|
||
| def init(self) -> None: | ||
| r"""Initialize the browser asynchronously.""" | ||
| return self.async_init() |
There was a problem hiding this comment.
will there be any issues if a synchronous method is used to call an asynchronous function?
There was a problem hiding this comment.
This should be the previous version. Now I just added the @retry_on_error()
camel/toolkits/browser_toolkit.py
Outdated
| fixed_part = f"_{timestamp}.png" | ||
| # Get the absolute path of the cache directory (ensure it ends with a separator) | ||
| base_path = os.path.join(os.path.abspath(self.cache_dir), "") | ||
| file_path = os.path.join(self.cache_dir, f"{url_name}{fixed_part}") | ||
|
|
||
| # If the generated file path exceeds the limit, truncate url_name accordingly | ||
| if len(file_path) > MAX_PATH_LENGTH: | ||
| allowed_name_length = ( | ||
| MAX_PATH_LENGTH - len(base_path) - len(fixed_part) | ||
| ) | ||
| url_name = url_name[:allowed_name_length] | ||
| file_path = os.path.join( | ||
| self.cache_dir, f"{url_name}{fixed_part}" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Is this handled this way because there will be problems on Windows if the path exceeds a certain length？
There was a problem hiding this comment.
thanks @subway-jack 's contribution! Left some comments below
|
|
||
| MAX_PATH_LENGTH = 260 | ||
|
|
||
| AVAILABLE_ACTIONS_PROMPT = """ |
There was a problem hiding this comment.
for the duplicated components involving the browser toolkit file, could we move them all into a browser_toolkit_commons.py file to reduce duplication? We can created another issue & PR to refactor this later
There was a problem hiding this comment.
Thanks for the suggestion! I’ve opened a follow-up issue to extract all duplicated browser-toolkit code into a shared commons module: #2380. We’ll track the refactor there and land a separate PR so this one can stay focused.
Added a BaseBrowser that supports asynchronous
Description
Describe your changes in detail (optional if the linked issue already contains a detailed description of the changes).
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!