-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Added on.('network' listener) #1382
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
base: main
Are you sure you want to change the base?
Conversation
|
Greptile OverviewGreptile SummaryAdded network event listener capabilities to Stagehand's
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Page
participant CDPSession
participant NetworkManager
participant NetworkMessage
participant Listener
Note over User,Listener: Network Event Registration
User->>Page: page.on("network", listener)
Page->>Page: onNetworkEvent(listener)
Page->>Page: Add listener to networkListeners Set
alt First listener registered
Page->>Page: ensureNetworkTaps()
Page->>CDPSession: send("Network.enable")
Page->>CDPSession: on("Network.requestWillBeSent", handler)
Page->>CDPSession: on("Network.responseReceived", handler)
end
Note over User,Listener: Network Request Flow
CDPSession-->>Page: Network.requestWillBeSent event
Page->>Page: emitNetworkRequest(evt)
Page->>NetworkMessage: new NetworkMessage({type: "request", ...})
Page->>Listener: listener(networkMessage)
Note over User,Listener: Network Response Flow
CDPSession-->>Page: Network.responseReceived event
Page->>Page: emitNetworkResponse(evt)
Page->>NetworkMessage: new NetworkMessage({type: "response", ...})
Page->>Listener: listener(networkMessage)
Note over User,Listener: Listener Removal
User->>Page: page.off("network", listener)
Page->>Page: offNetworkEvent(listener)
Page->>Page: Remove listener from networkListeners Set
alt Last listener removed
Page->>Page: removeAllNetworkTaps()
Page->>CDPSession: off("Network.requestWillBeSent", handler)
Page->>CDPSession: off("Network.responseReceived", handler)
end
|
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.
4 files reviewed, no comments
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.
2 issues found across 4 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/tests/page-network.spec.ts">
<violation number="1" location="packages/core/lib/v3/tests/page-network.spec.ts:246">
P2: The assertions check `message.url`, `message.type`, and `message.requestId` as properties, but throughout this file they are called as methods (e.g., `message.url()`, `message.type()`). This test verifies method references exist rather than that calling them works. Consider calling them as methods: `expect(message.url()).toBeDefined()`.</violation>
<violation number="2" location="packages/core/lib/v3/tests/page-network.spec.ts:351">
P2: The variable `foundPostData` is declared and conditionally set to true, but there's no assertion checking its value. This test doesn't actually verify anything. Either add an assertion (possibly with a soft expectation given timing concerns) or convert this to documentation/example code outside the test suite.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
2091cdb to
2032a15
Compare
2032a15 to
32cff87
Compare
why
Added network-recording capabilities to Stagehand
what changed
.on,.once, and.offmethods can now have a"network"event instead of"console"onConsoleEvent,onceConsoleEvent, andoffConsoleEvent.NetworkMessagetype as a wrapper over Response and Request CDP events.test plan
packages/core/lib/v3/tests/page-network.spec.tsto test all these options.Summary by cubic
Adds a "network" event to Page.on/once/off to capture requests and responses via a new NetworkMessage. Enables network recording across main and child CDP sessions, with tests covering the API.
New Features
Refactors
Written for commit 32cff87. Summary will update automatically on new commits.