Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a method for chromium-attached browser_key #116

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Add a method for chromium-attached browser_key #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0322b66
Add a method for chromium-attached browser_key
illgitthat Mar 18, 2024
d18c2ad
Merge branch 'main' into add-chromium-attached-browser-option
illgitthat Mar 21, 2024
57d7839
Move ws_endpoint to _create_attached_chromium
illgitthat Mar 21, 2024
b341552
Extra line snuck in
illgitthat Mar 21, 2024
00eae87
Extra line fix
illgitthat Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions skyvern/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Settings(BaseSettings):
ADDITIONAL_MODULES: list[str] = []

BROWSER_TYPE: str = "chromium-headful"
WS_ENDPOINT: str | None = None
MAX_SCRAPING_RETRIES: int = 0
VIDEO_PATH: str | None = None
HAR_PATH: str | None = "./har"
Expand Down
24 changes: 23 additions & 1 deletion skyvern/webeye/browser_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,29 @@ async def _create_headful_chromium(playwright: Playwright, **kwargs: dict) -> tu
return browser_context, browser_artifacts


async def _create_attached_chromium(playwright: Playwright, **kwargs: dict) -> tuple[BrowserContext, BrowserArtifacts]:
ws_endpoint = SettingsManager.get_settings().WS_ENDPOINT
browser = await playwright.chromium.connect_over_cdp(ws_endpoint)
browser_args = BrowserContextFactory.build_browser_args()
browser_args.update(
{
"headless": False,
}
)
contexts = browser.contexts
if contexts:
browser_context = contexts[0]
else:
print("No existing contexts found. Creating a new context...")
browser_context = await browser.new_context(**browser_args)

browser_artifacts = BrowserContextFactory.build_browser_artifacts(har_path=browser_args["record_har_path"])
return browser_context, browser_artifacts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the error you get trying this out??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code generally looks correct to me (minus the if contexts part)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resharing here:

2024-03-21T03:26:13.376947Z [info     ] Creating a new page
2024-03-21T03:26:13.917403Z [info     ] A new page is created
2024-03-21T03:26:13.917509Z [info     ] Navigating page to https://www.finditparts.com and waiting for 3 seconds
2024-03-21T03:26:17.559109Z [info     ] Successfully went to https://www.finditparts.com```

Essentially it opens the page and then just stops there.

Is there any log setting to modify to get more verbose logging?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you connecting over CDP locally? If you turn headless=true does a new window actually open up?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debugging playwright has definitely been a nightmare for us

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, connecting locally.

  1. Kill all running chromium processes
  2. Launch a new window by adding the --remote-debugging-port=9222

That pointed me in the right direction. It looks like a lot of the settings needed (like har path) can't be set on an existing browser_context: https://playwright.dev/python/docs/api/class-browsercontext

Although, if I remove the check for using an existing browser context it doesn't launch at all.



BrowserContextFactory.register_type("chromium-headless", _create_headless_chromium)
BrowserContextFactory.register_type("chromium-headful", _create_headful_chromium)
BrowserContextFactory.register_type("chromium-attached", _create_attached_chromium)


class BrowserState:
Expand All @@ -142,7 +163,8 @@ def __init__(
self.browser_artifacts = browser_artifacts

async def _close_all_other_pages(self) -> None:
if not self.browser_context or not self.page:
browser_type = SettingsManager.get_settings().BROWSER_TYPE
if not self.browser_context or not self.page or browser_type == "chromium-attached":
return
pages = self.browser_context.pages
for page in pages:
Expand Down