Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Commit 77949eb

Browse files
committed
Fix frame session bug on browser close
Browser close happens and then we keep processing events from CDP. Instead of doing that, now we first try to kill the browser process and immediately tell CDP to close the browser. This way CDP stops emitting us more events. We also skip errors from FrameSession if browser is already closed.
1 parent a3aea00 commit 77949eb

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

common/browser.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,21 @@ func (b *Browser) Close() {
346346
b.logger.Debugf("Browser:Close", "already in a closing state")
347347
return
348348
}
349-
b.browserProc.GracefulClose()
350-
defer b.browserProc.Terminate()
349+
350+
atomic.CompareAndSwapInt64(&b.state, b.state, BrowserStateClosed)
351351

352352
action := cdpbrowser.Close()
353353
if err := action.Do(cdp.WithExecutor(b.ctx, b.conn)); err != nil {
354354
if _, ok := err.(*websocket.CloseError); !ok {
355355
k6Throw(b.ctx, "unable to execute %T: %v", action, err)
356356
}
357357
}
358-
atomic.CompareAndSwapInt64(&b.state, b.state, BrowserStateClosed)
358+
359+
// terminate the browser process early on, then tell the CDP
360+
// afterwards. this will take a little bit of time, and CDP
361+
// will stop emitting events.
362+
b.browserProc.GracefulClose()
363+
b.browserProc.Terminate()
359364
}
360365

361366
// Contexts returns list of browser contexts

common/frame_session.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -805,8 +805,10 @@ func (fs *FrameSession) onAttachedToTarget(event *target.EventAttachedToTarget)
805805
fs.session.id, fs.targetID, event.SessionID,
806806
event.TargetInfo.TargetID, event.TargetInfo.BrowserContextID,
807807
event.TargetInfo.Type, err)
808-
// If we're no longer connected to browser, then ignore WebSocket errors
809-
if !fs.page.browserCtx.browser.connected && strings.Contains(err.Error(), "websocket: close 1006 (abnormal closure)") {
808+
// ignore errors if we're no longer connected to browser
809+
// this happens when there is no browser but we still want to
810+
// attach a frame to it.
811+
if !fs.page.browserCtx.browser.IsConnected() {
810812
return
811813
}
812814
// Ignore context canceled error to gracefuly handle shutting down
@@ -843,8 +845,10 @@ func (fs *FrameSession) onAttachedToTarget(event *target.EventAttachedToTarget)
843845
// Handle new worker
844846
w, err := NewWorker(fs.ctx, session, targetID, event.TargetInfo.URL)
845847
if err != nil {
846-
if !fs.page.browserCtx.browser.connected && strings.Contains(err.Error(), "websocket: close 1006 (abnormal closure)") {
847-
// If we're no longer connected to browser, then ignore WebSocket errors
848+
// ignore errors if we're no longer connected to browser
849+
// this happens when there is no browser but we still want to
850+
// attach a worker to it.
851+
if !fs.page.browserCtx.browser.connected {
848852
return
849853
}
850854
select {

0 commit comments

Comments
 (0)