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

Fix hanging close browser cdp req #1442

Merged
merged 7 commits into from
Sep 25, 2024
Merged

Fix hanging close browser cdp req #1442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aa261ce
Refactor to use existing background context
ankur22 Sep 24, 2024
0f14a6c
Refactor to use the existing background context
ankur22 Sep 24, 2024
6f62444
Refactor to work with a timeout on cdp close req
ankur22 Sep 24, 2024
e70422f
Refactor to use timed context in send
ankur22 Sep 24, 2024
a472330
Fix lint issue nlreturn
ankur22 Sep 24, 2024
b2bd13c
Update k6 with the k6 event system fix
ankur22 Sep 25, 2024
b4b16ba
Update github action go versions
ankur22 Sep 25, 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
4 changes: 2 additions & 2 deletions .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.21.x
go-version: 1.23.x
inancgumus marked this conversation as resolved.
Show resolved Hide resolved
check-latest: true
- name: Check dependencies
run: |
Expand All @@ -42,7 +42,7 @@ jobs:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.21.x
go-version: 1.23.x
check-latest: true
- name: Retrieve golangci-lint version
run: |
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version: [1.20.x]
go-version: [1.22.x]
platform: [ubuntu-latest]
runs-on: ${{ matrix.platform }}
steps:
Expand Down Expand Up @@ -72,7 +72,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version: [1.21.x]
go-version: [1.23.x]
platform: [ubuntu-latest]
runs-on: ${{ matrix.platform }}
steps:
Expand Down Expand Up @@ -110,7 +110,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version: [1.21.x]
go-version: [1.23.x]
platform: [ubuntu-latest]
runs-on: ${{ matrix.platform }}
steps:
Expand Down
10 changes: 6 additions & 4 deletions chromium/browser_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (b *BrowserType) Connect(ctx, vuCtx context.Context, wsEndpoint string) (*c
func (b *BrowserType) connect(
ctx, vuCtx context.Context, wsURL string, opts *common.BrowserOptions, logger *log.Logger,
) (*common.Browser, error) {
browserProc, err := b.link(wsURL, logger)
browserProc, err := b.link(ctx, wsURL, logger)
if browserProc == nil {
return nil, fmt.Errorf("connecting to browser: %w", err)
}
Expand All @@ -144,9 +144,10 @@ func (b *BrowserType) connect(
}

func (b *BrowserType) link(
ctx context.Context,
wsURL string, logger *log.Logger,
) (*common.BrowserProcess, error) {
bProcCtx, bProcCtxCancel := context.WithCancel(context.Background())
bProcCtx, bProcCtxCancel := context.WithCancel(ctx)
inancgumus marked this conversation as resolved.
Show resolved Hide resolved
p, err := common.NewRemoteBrowserProcess(bProcCtx, wsURL, bProcCtxCancel, logger)
if err != nil {
bProcCtxCancel()
Expand Down Expand Up @@ -205,7 +206,7 @@ func (b *BrowserType) launch(
return nil, 0, fmt.Errorf("finding browser executable: %w", err)
}

browserProc, err := b.allocate(path, flags, dataDir, logger)
browserProc, err := b.allocate(ctx, path, flags, dataDir, logger)
if browserProc == nil {
return nil, 0, fmt.Errorf("launching browser: %w", err)
}
Expand Down Expand Up @@ -238,11 +239,12 @@ func (b *BrowserType) Name() string {

// allocate starts a new Chromium browser process and returns it.
func (b *BrowserType) allocate(
ctx context.Context,
path string,
flags map[string]any, dataDir *storage.Dir,
logger *log.Logger,
) (_ *common.BrowserProcess, rerr error) {
bProcCtx, bProcCtxCancel := context.WithCancel(context.Background())
bProcCtx, bProcCtxCancel := context.WithCancel(ctx)
defer func() {
if rerr != nil {
bProcCtxCancel()
Expand Down
11 changes: 8 additions & 3 deletions common/browser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,14 @@ func (b *Browser) Close() {
// command, which triggers the browser process to exit.
if !b.browserOpts.isRemoteBrowser {
var closeErr *websocket.CloseError
// Using a internal context here since vu context will very likely be
// closed.
err := cdpbrowser.Close().Do(cdp.WithExecutor(b.browserCtx, b.conn))
// Using the internal context with a timeout of 10 seconds here since
// 1. vu context will very likely be closed;
// 2. there's a chance that the process has died but the connection still
// thinks it's open.
toCtx, toCancelCtx := context.WithTimeout(b.browserCtx, time.Second*10)
inancgumus marked this conversation as resolved.
Show resolved Hide resolved
defer toCancelCtx()

err := cdpbrowser.Close().Do(cdp.WithExecutor(toCtx, b.conn))
if err != nil && !errors.As(err, &closeErr) {
b.logger.Errorf("Browser:Close", "closing the browser: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion common/connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@ func (c *Connection) Execute(ctx context.Context, method string, params easyjson
Method: cdproto.MethodType(method),
Params: buf,
}
return c.send(c.ctx, msg, ch, res)

return c.send(evCancelCtx, msg, ch, res)
}

// IgnoreIOErrors signals that the connection will soon be closed, so that any
Expand Down
44 changes: 22 additions & 22 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ go 1.21
require (
github.com/chromedp/cdproto v0.0.0-20221023212508-67ada9507fb2
github.com/gorilla/websocket v1.5.1
github.com/grafana/sobek v0.0.0-20240808084414-f7ac208544fe
github.com/grafana/sobek v0.0.0-20240829081756-447e8c611945
github.com/mailru/easyjson v0.7.7
github.com/mccutchen/go-httpbin v1.1.2-0.20190116014521-c5cb2f4802fa
github.com/mstoykov/k6-taskqueue-lib v0.1.0
github.com/sirupsen/logrus v1.9.3
github.com/stretchr/testify v1.9.0
go.k6.io/k6 v0.53.0
go.opentelemetry.io/otel v1.24.0
go.opentelemetry.io/otel/trace v1.24.0
golang.org/x/net v0.27.0
golang.org/x/sync v0.7.0
go.k6.io/k6 v0.53.1-0.20240925100229-86ab6e3ceee8
go.opentelemetry.io/otel v1.29.0
go.opentelemetry.io/otel/trace v1.29.0
golang.org/x/net v0.28.0
golang.org/x/sync v0.8.0
gopkg.in/guregu/null.v3 v3.3.0
)

Expand All @@ -28,15 +28,16 @@ require (
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/chromedp/sysutil v1.0.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dlclark/regexp2 v1.9.0 // indirect
github.com/dlclark/regexp2 v1.11.4 // indirect
github.com/dop251/goja v0.0.0-20240610225006-393f6d42497b // indirect
github.com/evanw/esbuild v0.21.2 // indirect
github.com/fatih/color v1.17.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-sourcemap/sourcemap v2.1.4+incompatible // indirect
github.com/google/pprof v0.0.0-20230728192033-2ba5b33183c6 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/klauspost/compress v1.17.9 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
Expand All @@ -46,25 +47,24 @@ require (
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/onsi/gomega v1.20.2 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.11.0 // indirect
github.com/serenize/snaker v0.0.0-20201027110005-a7ad2135616e // indirect
github.com/spf13/afero v1.1.2 // indirect
github.com/tidwall/gjson v1.17.1 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.24.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0 // indirect
go.opentelemetry.io/otel/metric v1.24.0 // indirect
go.opentelemetry.io/otel/sdk v1.24.0 // indirect
go.opentelemetry.io/proto/otlp v1.1.0 // indirect
golang.org/x/crypto v0.25.0 // indirect
golang.org/x/sys v0.22.0 // indirect
golang.org/x/text v0.16.0 // indirect
golang.org/x/time v0.5.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240318140521-94a12d6c2237 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect
google.golang.org/grpc v1.64.1 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.29.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.29.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.29.0 // indirect
go.opentelemetry.io/otel/metric v1.29.0 // indirect
go.opentelemetry.io/otel/sdk v1.29.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
golang.org/x/crypto v0.26.0 // indirect
golang.org/x/sys v0.24.0 // indirect
golang.org/x/text v0.17.0 // indirect
golang.org/x/time v0.6.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240822170219-fc7c04adadcd // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240822170219-fc7c04adadcd // indirect
google.golang.org/grpc v1.65.0 // indirect
google.golang.org/protobuf v1.34.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading
Loading