-
Notifications
You must be signed in to change notification settings - Fork 191
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.
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
browserbaseResumeSessionID to connect to an running bb session #172
browserbaseResumeSessionID to connect to an running bb session #172
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I think this is almost there. I left a few comments on project ID validation, but I'd also request that while you're here, let's assign browserbase_project_id
and browserbase_api_key
to constants at the beginning of getBrowser
and refer to those. That way, we don't have to repeatedly refer to process.env
variables that may or may not exist.
@@ -31,44 +32,67 @@ async function getBrowser( | |||
env = "LOCAL"; | |||
} | |||
|
|||
if (env === "BROWSERBASE" && !process.env.BROWSERBASE_PROJECT_ID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this was removed bc you don't need project ID to connect to a running session, but I don't see this checking for project ID before creating session (see my other comment)
}); | ||
|
||
const session = await browserbase.sessions.create({ | ||
projectId: process.env.BROWSERBASE_PROJECT_ID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not enforcing project ID to exist anywhere anymore
Merging this into |
* browserbaseResumeSessionID to connect to an running bb session (#172) * enforce project ID * api key and project id as params --------- Co-authored-by: Marko Kraemer <[email protected]>
Should´ve caught that I deleted the validation. Happily : ) Great work on Stagehand <3 |
Now you are obligated to add
It wont get it automatically from .env at all I believe. So I had to update my code. |
If intentional behaviour, make sure to update readme. Else I think its more convenient that it tries to get from .env if none are provided in the constructor |
Great catch, I submitted #177 to fix this. Should be released soon :) |
why
For my use case I am using keepAlive for browser base sessions. I have an Web-Browser control API I am creating for softgen.ai´s web interaction agent that gets bb session id as param. Stagehand only created new sessions, but I have to reconnect to the existing one.
what changed
added browserbaseResumeSessionID?: string, to getBrowser() as an optional param to be able to resume an existing running session. Validation with browserbase sdk to check whether the session is running, throws error if not.