-
Notifications
You must be signed in to change notification settings - Fork 957
handle reroute for rollout #786
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
Conversation
🦋 Changeset detectedLatest commit: b0125a0 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
types/api.ts
Outdated
@@ -29,6 +29,7 @@ export interface StartSessionParams { | |||
|
|||
export interface StartSessionResult { | |||
sessionId: string; | |||
unavailable?: boolean; |
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.
this is a little unintuitive -- having a true/false field for apiEnabled
from the API would be cleaner than a true/undefined unavailable
field
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.
Bug: Session ID Inconsistency and Type Violation
The API client sets its internal this.sessionId
property before checking the unavailable
flag in the response data. If unavailable
is true, the sessionId
in the returned data is set to undefined
, but this.sessionId
retains the original value. This creates an inconsistent state where the client instance holds a stale session ID, potentially causing subsequent API calls to use an invalid ID. Additionally, setting sessionResponseBody.data.sessionId
to undefined
violates the StartSessionResult
type contract which expects sessionId
to be a string.
lib/api.ts#L92-L100
Lines 92 to 100 in b408393
this.sessionId = sessionResponseBody.data.sessionId; | |
// Temporary reroute for rollout | |
if (sessionResponseBody.data?.unavailable) { | |
sessionResponseBody.data.sessionId = undefined; | |
} | |
return sessionResponseBody.data; |
BugBot free trial expires on June 9, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
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.
PR Summary
This PR adds session availability handling for a rollout feature, but needs fixes to maintain API client state consistency and type safety.
- The
init()
method inlib/api.ts
should clearthis.sessionId
whenavailable
is false to maintain consistency with returned data - Add type guard or assertion to ensure
sessionResponseBody.data.sessionId
is always a string whenavailable
is true - Consider adding error handling for API calls when
this.sessionId
is undefined - Add documentation for the
available
flag behavior intypes/api.ts
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @browserbasehq/[email protected] ### Patch Changes - [#796](#796) [`12a99b3`](12a99b3) Thanks [@miguelg719](https://github.com/miguelg719)! - Added a experimental flag to enable the newest and most experimental features - [#807](#807) [`2451797`](2451797) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - include version number in StagehandDefaultError message - [#803](#803) [`1d631a5`](1d631a5) Thanks [@miguelg719](https://github.com/miguelg719)! - Enable session affinity for cache optimization - [#804](#804) [`9c398bb`](9c398bb) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - update operatorResponseSchema based on new openai spec - [#786](#786) [`c19ad7f`](c19ad7f) Thanks [@miguelg719](https://github.com/miguelg719)! - Handle reroute to account for rollout ## @browserbasehq/[email protected] ### Minor Changes - [#778](#778) [`df570b6`](df570b6) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - iframe support ### Patch Changes - [#809](#809) [`03ebebc`](03ebebc) Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - log NoObjectGenerated error details - [#801](#801) [`1d4f0ab`](1d4f0ab) Thanks [@miguelg719](https://github.com/miguelg719)! - Default use API to true - [#798](#798) [`d86200b`](d86200b) Thanks [@miguelg719](https://github.com/miguelg719)! - Fix pino logging memory leak by reusing worker ## @browserbasehq/[email protected] ### Patch Changes - Updated dependencies \[[`12a99b3`](12a99b3), [`2451797`](2451797), [`1d631a5`](1d631a5), [`9c398bb`](9c398bb), [`c19ad7f`](c19ad7f)]: - @browserbasehq/[email protected] ## @browserbasehq/[email protected] ### Patch Changes - Updated dependencies \[[`12a99b3`](12a99b3), [`2451797`](2451797), [`1d631a5`](1d631a5), [`9c398bb`](9c398bb), [`c19ad7f`](c19ad7f)]: - @browserbasehq/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
why
what changed
test plan