diff --git a/go.mod b/go.mod index 6dbc5893e54..7f9f5677525 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/go-sourcemap/sourcemap v2.1.4+incompatible github.com/golang/protobuf v1.5.4 github.com/gorilla/websocket v1.5.1 - github.com/grafana/xk6-browser v1.7.1 + github.com/grafana/xk6-browser v1.8.0 github.com/grafana/xk6-dashboard v0.7.5 github.com/grafana/xk6-output-opentelemetry v0.2.0 github.com/grafana/xk6-output-prometheus-remote v0.4.0 diff --git a/go.sum b/go.sum index f9292180ab7..e5ec214d429 100644 --- a/go.sum +++ b/go.sum @@ -91,8 +91,8 @@ github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/ github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY= github.com/grafana/sobek v0.0.0-20240829081756-447e8c611945 h1:rxyoc2Ee/iKcm5EYNU/dEO0GcMZDN5vaDGyIo7pt804= github.com/grafana/sobek v0.0.0-20240829081756-447e8c611945/go.mod h1:FmcutBFPLiGgroH42I4/HBahv7GxVjODcVWFTw1ISes= -github.com/grafana/xk6-browser v1.7.1 h1:RKCcoFyKT97iGgbnK76WwxcXnkB23ijlO1LghqjHQ0E= -github.com/grafana/xk6-browser v1.7.1/go.mod h1:sZO7cT7/XQf2mz+rXkp6poswhOCA0JKA8isj3fQrfaU= +github.com/grafana/xk6-browser v1.8.0 h1:O2JuHT88dnUVu2+TAnI7BJ4hJ6E1YIX4TSmi7dho8GI= +github.com/grafana/xk6-browser v1.8.0/go.mod h1:we1lHAYSVZD5s/4yre2KWtaqY4G0ikNVpn0QE2EO8DA= github.com/grafana/xk6-dashboard v0.7.5 h1:TcILyffT/Ea/XD7xG1jMA5lwtusOPRbEQsQDHmO30Mk= github.com/grafana/xk6-dashboard v0.7.5/go.mod h1:Y75F8xmgCraKT+pBzFH6me9AyH5PkXD+Bxo1dm6Il/M= github.com/grafana/xk6-output-opentelemetry v0.2.0 h1:u/ctsfUQsyLh6paBjOmKshqeYr6HUsCJr29eO2QTb/s= diff --git a/vendor/github.com/grafana/xk6-browser/browser/module.go b/vendor/github.com/grafana/xk6-browser/browser/module.go index 57bc62c349c..1e42c852533 100644 --- a/vendor/github.com/grafana/xk6-browser/browser/module.go +++ b/vendor/github.com/grafana/xk6-browser/browser/module.go @@ -104,7 +104,6 @@ func (m *RootModule) NewModuleInstance(vu k6modules.VU) k6modules.Instance { VU: vu, pidRegistry: m.PidRegistry, browserRegistry: newBrowserRegistry( - context.Background(), vu, m.remoteRegistry, m.PidRegistry, diff --git a/vendor/github.com/grafana/xk6-browser/browser/page_mapping.go b/vendor/github.com/grafana/xk6-browser/browser/page_mapping.go index 7703b8e8d77..c494da38ef8 100644 --- a/vendor/github.com/grafana/xk6-browser/browser/page_mapping.go +++ b/vendor/github.com/grafana/xk6-browser/browser/page_mapping.go @@ -236,6 +236,10 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop return nil, err //nolint:wrapcheck } + if resp == nil { + return nil, nil + } + r := mapResponse(vu, resp) return rt.ToValue(r).ToObject(rt), nil diff --git a/vendor/github.com/grafana/xk6-browser/browser/registry.go b/vendor/github.com/grafana/xk6-browser/browser/registry.go index 5f61d459761..3310cf81025 100644 --- a/vendor/github.com/grafana/xk6-browser/browser/registry.go +++ b/vendor/github.com/grafana/xk6-browser/browser/registry.go @@ -185,13 +185,25 @@ type browserRegistry struct { stopped atomic.Bool // testing purposes } -type browserBuildFunc func(ctx context.Context) (*common.Browser, error) +type browserBuildFunc func(vuCtx context.Context) (*common.Browser, error) +// newBrowserRegistry should only take a background context, not a context from +// k6 (i.e. vu). The reason for this is that we want to control the chromium +// lifecycle with the k6 event system. +// +// The k6 event system gives this extension time to properly cleanup any running +// chromium subprocesses or connections to a remote chromium instance. +// +// A vu context (a context on an iteration) doesn't allow us to do this. Once k6 +// closes a vu context, it basically pulls the rug from under the extensions feet. func newBrowserRegistry( - ctx context.Context, vu k6modules.VU, remote *remoteRegistry, pids *pidRegistry, tracesMetadata map[string]string, + vu k6modules.VU, + remote *remoteRegistry, + pids *pidRegistry, + tracesMetadata map[string]string, ) *browserRegistry { bt := chromium.NewBrowserType(vu) - builder := func(ctx context.Context) (*common.Browser, error) { + builder := func(vuCtx context.Context) (*common.Browser, error) { var ( err error b *common.Browser @@ -199,13 +211,13 @@ func newBrowserRegistry( ) if isRemoteBrowser { - b, err = bt.Connect(ctx, wsURL) + b, err = bt.Connect(vuCtx, wsURL) if err != nil { return nil, err //nolint:wrapcheck } } else { var pid int - b, pid, err = bt.Launch(ctx) + b, pid, err = bt.Launch(vuCtx) if err != nil { return nil, err //nolint:wrapcheck } @@ -235,13 +247,13 @@ func newBrowserRegistry( } go r.handleExitEvent(exitCh, unsubscribe) - go r.handleIterEvents(ctx, eventsCh, unsubscribe) + go r.handleIterEvents(eventsCh, unsubscribe) return r } func (r *browserRegistry) handleIterEvents( //nolint:funlen - ctx context.Context, eventsCh <-chan *k6event.Event, unsubscribeFn func(), + eventsCh <-chan *k6event.Event, unsubscribeFn func(), ) { var ( ok bool @@ -285,8 +297,12 @@ func (r *browserRegistry) handleIterEvents( //nolint:funlen // so we can get access to the k6 TracerProvider. r.initTracesRegistry() - // Wrap the tracer into the browser context to make it accessible for the other - // components that inherit the context so these can use it to trace their actions. + // Wrap the tracer into the VU context to make it accessible for the + // other components during the iteration that inherit the VU context. + // + // All browser APIs should work with the vu context, and allow the + // k6 iteration control its lifecycle. + ctx := r.vu.Context() tracerCtx := common.WithTracer(ctx, r.tr.tracer) tracedCtx := r.tr.startIterationTrace(tracerCtx, data) @@ -355,6 +371,15 @@ func (r *browserRegistry) deleteBrowser(id int64) { } } +// This is only used in a test. Avoids having to manipulate the mutex in the +// test itself. +func (r *browserRegistry) browserCount() int { + r.mu.Lock() + defer r.mu.Unlock() + + return len(r.m) +} + func (r *browserRegistry) clear() { r.mu.Lock() defer r.mu.Unlock() @@ -462,6 +487,15 @@ func (r *tracesRegistry) stop() { } } +// This is only used in a test. Avoids having to manipulate the mutex in the +// test itself. +func (r *tracesRegistry) iterationTracesCount() int { + r.mu.Lock() + defer r.mu.Unlock() + + return len(r.m) +} + func parseTracesMetadata(envLookup env.LookupFunc) (map[string]string, error) { var ( ok bool diff --git a/vendor/github.com/grafana/xk6-browser/browser/sync_page_mapping.go b/vendor/github.com/grafana/xk6-browser/browser/sync_page_mapping.go index 19a6a7a7f09..28243ac4d89 100644 --- a/vendor/github.com/grafana/xk6-browser/browser/sync_page_mapping.go +++ b/vendor/github.com/grafana/xk6-browser/browser/sync_page_mapping.go @@ -140,6 +140,10 @@ func syncMapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop return nil, err //nolint:wrapcheck } + if resp == nil { + return nil, nil //nolint:nilnil + } + r := syncMapResponse(vu, resp) return rt.ToValue(r).ToObject(rt), nil diff --git a/vendor/github.com/grafana/xk6-browser/chromium/browser_type.go b/vendor/github.com/grafana/xk6-browser/chromium/browser_type.go index 4f3dc65c0be..cddd35503a7 100644 --- a/vendor/github.com/grafana/xk6-browser/chromium/browser_type.go +++ b/vendor/github.com/grafana/xk6-browser/chromium/browser_type.go @@ -93,13 +93,23 @@ func (b *BrowserType) initContext(ctx context.Context) context.Context { } // Connect attaches k6 browser to an existing browser instance. -func (b *BrowserType) Connect(ctx context.Context, wsEndpoint string) (*common.Browser, error) { - ctx, browserOpts, logger, err := b.init(ctx, true) +// +// vuCtx is the context coming from the VU itself. The k6 vu/iteration controls +// its lifecycle. +// +// context.background() is used when connecting to an instance of chromium. The +// connection lifecycle should be handled by the k6 event system. +// +// The separation is important to allow for the iteration to end when k6 requires +// the iteration to end (e.g. during a SIGTERM) and unblocks k6 to then fire off +// the events which allows the connection to close. +func (b *BrowserType) Connect(vuCtx context.Context, wsEndpoint string) (*common.Browser, error) { + vuCtx, browserOpts, logger, err := b.init(vuCtx, true) if err != nil { return nil, fmt.Errorf("initializing browser type: %w", err) } - bp, err := b.connect(ctx, wsEndpoint, browserOpts, logger) + bp, err := b.connect(vuCtx, wsEndpoint, browserOpts, logger) if err != nil { err = &k6ext.UserFriendlyError{ Err: err, @@ -112,16 +122,16 @@ func (b *BrowserType) Connect(ctx context.Context, wsEndpoint string) (*common.B } func (b *BrowserType) connect( - ctx context.Context, wsURL string, opts *common.BrowserOptions, logger *log.Logger, + vuCtx context.Context, wsURL string, opts *common.BrowserOptions, logger *log.Logger, ) (*common.Browser, error) { - browserProc, err := b.link(ctx, wsURL, logger) + browserProc, err := b.link(wsURL, logger) if browserProc == nil { return nil, fmt.Errorf("connecting to browser: %w", err) } // If this context is cancelled we'll initiate an extension wide // cancellation and shutdown. - browserCtx, browserCtxCancel := context.WithCancel(ctx) + browserCtx, browserCtxCancel := context.WithCancel(vuCtx) b.Ctx = browserCtx browser, err := common.NewBrowser( browserCtx, browserCtxCancel, browserProc, opts, logger, @@ -134,9 +144,9 @@ func (b *BrowserType) connect( } func (b *BrowserType) link( - ctx context.Context, wsURL string, logger *log.Logger, + wsURL string, logger *log.Logger, ) (*common.BrowserProcess, error) { - bProcCtx, bProcCtxCancel := context.WithCancel(ctx) + bProcCtx, bProcCtxCancel := context.WithCancel(context.Background()) p, err := common.NewRemoteBrowserProcess(bProcCtx, wsURL, bProcCtxCancel, logger) if err != nil { bProcCtxCancel() @@ -148,13 +158,23 @@ func (b *BrowserType) link( // Launch allocates a new Chrome browser process and returns a new Browser value, // which can be used for controlling the Chrome browser. -func (b *BrowserType) Launch(ctx context.Context) (_ *common.Browser, browserProcessID int, _ error) { - ctx, browserOpts, logger, err := b.init(ctx, false) +// +// vuCtx is the context coming from the VU itself. The k6 vu/iteration controls +// its lifecycle. +// +// context.background() is used when launching an instance of chromium. The +// chromium lifecycle should be handled by the k6 event system. +// +// The separation is important to allow for the iteration to end when k6 requires +// the iteration to end (e.g. during a SIGTERM) and unblocks k6 to then fire off +// the events which allows the chromium subprocess to shutdown. +func (b *BrowserType) Launch(vuCtx context.Context) (_ *common.Browser, browserProcessID int, _ error) { + vuCtx, browserOpts, logger, err := b.init(vuCtx, false) if err != nil { return nil, 0, fmt.Errorf("initializing browser type: %w", err) } - bp, pid, err := b.launch(ctx, browserOpts, logger) + bp, pid, err := b.launch(vuCtx, browserOpts, logger) if err != nil { err = &k6ext.UserFriendlyError{ Err: err, @@ -167,7 +187,7 @@ func (b *BrowserType) Launch(ctx context.Context) (_ *common.Browser, browserPro } func (b *BrowserType) launch( - ctx context.Context, opts *common.BrowserOptions, logger *log.Logger, + vuCtx context.Context, opts *common.BrowserOptions, logger *log.Logger, ) (_ *common.Browser, pid int, _ error) { flags, err := prepareFlags(opts, &(b.vu.State()).Options) if err != nil { @@ -185,14 +205,14 @@ func (b *BrowserType) launch( return nil, 0, fmt.Errorf("finding browser executable: %w", err) } - browserProc, err := b.allocate(ctx, path, flags, dataDir, logger) + browserProc, err := b.allocate(path, flags, dataDir, logger) if browserProc == nil { return nil, 0, fmt.Errorf("launching browser: %w", err) } // If this context is cancelled we'll initiate an extension wide // cancellation and shutdown. - browserCtx, browserCtxCancel := context.WithCancel(ctx) + browserCtx, browserCtxCancel := context.WithCancel(vuCtx) b.Ctx = browserCtx browser, err := common.NewBrowser(browserCtx, browserCtxCancel, browserProc, opts, logger) @@ -218,11 +238,11 @@ func (b *BrowserType) Name() string { // allocate starts a new Chromium browser process and returns it. func (b *BrowserType) allocate( - ctx context.Context, path string, + path string, flags map[string]any, dataDir *storage.Dir, logger *log.Logger, ) (_ *common.BrowserProcess, rerr error) { - bProcCtx, bProcCtxCancel := context.WithCancel(ctx) + bProcCtx, bProcCtxCancel := context.WithCancel(context.Background()) defer func() { if rerr != nil { bProcCtxCancel() diff --git a/vendor/github.com/grafana/xk6-browser/common/browser.go b/vendor/github.com/grafana/xk6-browser/common/browser.go index c8e617c0cc8..9efc793486d 100644 --- a/vendor/github.com/grafana/xk6-browser/common/browser.go +++ b/vendor/github.com/grafana/xk6-browser/common/browser.go @@ -17,8 +17,6 @@ import ( "github.com/grafana/xk6-browser/k6ext" "github.com/grafana/xk6-browser/log" - - k6modules "go.k6.io/k6/js/modules" ) const ( @@ -28,8 +26,14 @@ const ( // Browser stores a Browser context. type Browser struct { - ctx context.Context - cancelFn context.CancelFunc + // These are internal contexts which control the lifecycle of the goroutine + // that handles incoming CDP commands. It is shutdown when browser.close() + // is called. + initContext context.Context + initCancelFn context.CancelFunc + + vuCtx context.Context + vuCtxCancelFn context.CancelFunc state int64 @@ -53,9 +57,6 @@ type Browser struct { context *BrowserContext defaultContext *BrowserContext - // Cancel function to stop event listening - evCancelFn context.CancelFunc - // Needed as the targets map will be accessed from multiple Go routines, // the main VU/JS go routine and the Go routine listening for CDP messages. pagesMu sync.RWMutex @@ -70,8 +71,6 @@ type Browser struct { // version caches the browser version information. version browserVersion - vu k6modules.VU - logger *log.Logger } @@ -86,13 +85,13 @@ type browserVersion struct { // NewBrowser creates a new browser, connects to it, then returns it. func NewBrowser( - ctx context.Context, - cancel context.CancelFunc, + vuCtx context.Context, + vuCtxCancelFn context.CancelFunc, browserProc *BrowserProcess, browserOpts *BrowserOptions, logger *log.Logger, ) (*Browser, error) { - b := newBrowser(ctx, cancel, browserProc, browserOpts, logger) + b := newBrowser(vuCtx, vuCtxCancelFn, browserProc, browserOpts, logger) if err := b.connect(); err != nil { return nil, err } @@ -108,21 +107,20 @@ func NewBrowser( // newBrowser returns a ready to use Browser without connecting to an actual browser. func newBrowser( - ctx context.Context, - cancelFn context.CancelFunc, + vuCtx context.Context, + vuCtxCancelFn context.CancelFunc, browserProc *BrowserProcess, browserOpts *BrowserOptions, logger *log.Logger, ) *Browser { return &Browser{ - ctx: ctx, - cancelFn: cancelFn, + vuCtx: vuCtx, + vuCtxCancelFn: vuCtxCancelFn, state: int64(BrowserStateOpen), browserProc: browserProc, browserOpts: browserOpts, pages: make(map[target.ID]*Page), sessionIDtoTargetID: make(map[target.SessionID]target.ID), - vu: k6ext.GetVU(ctx), logger: logger, } } @@ -136,7 +134,7 @@ func (b *Browser) connect() error { // from doing unnecessary work. var err error b.conn, err = NewConnection( - b.ctx, + context.Background(), b.browserProc.WsURL(), b.logger, b.connectionOnAttachedToTarget, @@ -146,7 +144,7 @@ func (b *Browser) connect() error { } // We don't need to lock this because `connect()` is called only in NewBrowser - b.defaultContext, err = NewBrowserContext(b.ctx, b, "", NewBrowserContextOptions(), b.logger) + b.defaultContext, err = NewBrowserContext(b.vuCtx, b, "", NewBrowserContextOptions(), b.logger) if err != nil { return fmt.Errorf("browser connect: %w", err) } @@ -158,7 +156,7 @@ func (b *Browser) disposeContext(id cdp.BrowserContextID) error { b.logger.Debugf("Browser:disposeContext", "bctxid:%v", id) action := target.DisposeBrowserContext(id) - if err := action.Do(cdp.WithExecutor(b.ctx, b.conn)); err != nil { + if err := action.Do(cdp.WithExecutor(b.vuCtx, b.conn)); err != nil { return fmt.Errorf("disposing browser context ID %s: %w", id, err) } @@ -191,11 +189,14 @@ func (b *Browser) getPages() []*Page { } func (b *Browser) initEvents() error { //nolint:cyclop - var cancelCtx context.Context - cancelCtx, b.evCancelFn = context.WithCancel(b.ctx) chHandler := make(chan Event) - b.conn.on(cancelCtx, []string{ + // Using context.Background() here. Using vuCtx would close the connection/subprocess + // and therefore shutdown chromium when the iteration ends which isn't what we + // want to happen. Chromium should only be closed by the k6 event system. + b.initContext, b.initCancelFn = context.WithCancel(context.Background()) + + b.conn.on(b.initContext, []string{ cdproto.EventTargetAttachedToTarget, cdproto.EventTargetDetachedFromTarget, EventConnectionClose, @@ -203,21 +204,24 @@ func (b *Browser) initEvents() error { //nolint:cyclop go func() { defer func() { - b.logger.Debugf("Browser:initEvents:defer", "ctx err: %v", cancelCtx.Err()) b.browserProc.didLoseConnection() - if b.cancelFn != nil { - b.cancelFn() + // Closing the vuCtx incase it hasn't already been closed. Very likely + // already closed since the vuCtx is controlled by the k6 iteration, + // whereas the initContext is controlled by the k6 event system when + // browser.close() is called. k6 iteration ends before the event system. + if b.vuCtxCancelFn != nil { + b.vuCtxCancelFn() } }() for { select { - case <-cancelCtx.Done(): + case <-b.initContext.Done(): return case event := <-chHandler: if ev, ok := event.data.(*target.EventAttachedToTarget); ok { b.logger.Debugf("Browser:initEvents:onAttachedToTarget", "sid:%v tid:%v", ev.SessionID, ev.TargetInfo.TargetID) if err := b.onAttachedToTarget(ev); err != nil { - k6ext.Panic(b.ctx, "browser is attaching to target: %w", err) + k6ext.Panic(b.vuCtx, "browser is attaching to target: %w", err) } } else if ev, ok := event.data.(*target.EventDetachedFromTarget); ok { b.logger.Debugf("Browser:initEvents:onDetachedFromTarget", "sid:%v", ev.SessionID) @@ -231,7 +235,7 @@ func (b *Browser) initEvents() error { //nolint:cyclop }() action := target.SetAutoAttach(true, true).WithFlatten(true) - if err := action.Do(cdp.WithExecutor(b.ctx, b.conn)); err != nil { + if err := action.Do(cdp.WithExecutor(b.vuCtx, b.conn)); err != nil { return fmt.Errorf("internal error while auto-attaching to browser pages: %w", err) } @@ -239,7 +243,7 @@ func (b *Browser) initEvents() error { //nolint:cyclop // However making a dummy call afterwards fixes this. // This can be removed after https://chromium-review.googlesource.com/c/chromium/src/+/2885888 lands in stable. action2 := target.GetTargetInfo() - if _, err := action2.Do(cdp.WithExecutor(b.ctx, b.conn)); err != nil { + if _, err := action2.Do(cdp.WithExecutor(b.vuCtx, b.conn)); err != nil { return fmt.Errorf("internal error while getting browser target info: %w", err) } @@ -299,7 +303,7 @@ func (b *Browser) onAttachedToTarget(ev *target.EventAttachedToTarget) error { } b.pagesMu.RUnlock() } - p, err := NewPage(b.ctx, session, browserCtx, targetPage.TargetID, opener, isPage, b.logger) + p, err := NewPage(b.vuCtx, session, browserCtx, targetPage.TargetID, opener, isPage, b.logger) if err != nil && b.isPageAttachmentErrorIgnorable(ev, session, err) { return nil // Ignore this page. } @@ -388,10 +392,10 @@ func (b *Browser) isPageAttachmentErrorIgnorable(ev *target.EventAttachedToTarge } // No need to register the page if the test run is over. select { - case <-b.ctx.Done(): + case <-b.vuCtx.Done(): b.logger.Debugf("Browser:isPageAttachmentErrorIgnorable:return:<-ctx.Done", "sid:%v tid:%v pageType:%s err:%v", - ev.SessionID, targetPage.TargetID, targetPage.Type, b.ctx.Err()) + ev.SessionID, targetPage.TargetID, targetPage.Type, b.vuCtx.Err()) return true default: } @@ -439,7 +443,7 @@ func (b *Browser) newPageInContext(id cdp.BrowserContextID) (*Page, error) { return nil, fmt.Errorf("missing browser context %s, current context is %s", id, b.context.id) } - ctx, cancel := context.WithTimeout(b.ctx, b.browserOpts.Timeout) + ctx, cancel := context.WithTimeout(b.vuCtx, b.browserOpts.Timeout) defer cancel() // buffer of one is for sending the target ID whether an event handler @@ -502,6 +506,8 @@ func (b *Browser) Close() { if err := b.browserProc.Cleanup(); err != nil { b.logger.Errorf("Browser:Close", "cleaning up the user data directory: %v", err) } + + b.initCancelFn() }() b.logger.Debugf("Browser:Close", "") @@ -519,7 +525,9 @@ func (b *Browser) Close() { // command, which triggers the browser process to exit. if !b.browserOpts.isRemoteBrowser { var closeErr *websocket.CloseError - err := cdpbrowser.Close().Do(cdp.WithExecutor(b.ctx, b.conn)) + // Using a background context here since vu context will very likely be + // closed. + err := cdpbrowser.Close().Do(cdp.WithExecutor(context.Background(), b.conn)) if err != nil && !errors.As(err, &closeErr) { b.logger.Errorf("Browser:Close", "closing the browser: %v", err) } @@ -569,7 +577,7 @@ func (b *Browser) IsConnected() bool { // NewContext creates a new incognito-like browser context. func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, error) { - _, span := TraceAPICall(b.ctx, "", "browser.newContext") + _, span := TraceAPICall(b.vuCtx, "", "browser.newContext") defer span.End() if b.context != nil { @@ -579,7 +587,7 @@ func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, erro } action := target.CreateBrowserContext().WithDisposeOnDetach(true) - browserContextID, err := action.Do(cdp.WithExecutor(b.ctx, b.conn)) + browserContextID, err := action.Do(cdp.WithExecutor(b.vuCtx, b.conn)) b.logger.Debugf("Browser:NewContext", "bctxid:%v", browserContextID) if err != nil { err := fmt.Errorf("creating browser context ID %s: %w", browserContextID, err) @@ -587,7 +595,7 @@ func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, erro return nil, err } - browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, opts, b.logger) + browserCtx, err := NewBrowserContext(b.vuCtx, b, browserContextID, opts, b.logger) if err != nil { err := fmt.Errorf("new context: %w", err) spanRecordError(span, err) @@ -603,7 +611,7 @@ func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, erro // NewPage creates a new tab in the browser window. func (b *Browser) NewPage(opts *BrowserContextOptions) (*Page, error) { - _, span := TraceAPICall(b.ctx, "", "browser.newPage") + _, span := TraceAPICall(b.vuCtx, "", "browser.newPage") defer span.End() browserCtx, err := b.NewContext(opts) @@ -632,8 +640,8 @@ func (b *Browser) On(event string) (bool, error) { select { case <-b.browserProc.lostConnection: return true, nil - case <-b.ctx.Done(): - return false, fmt.Errorf("browser.on promise rejected: %w", b.ctx.Err()) + case <-b.vuCtx.Done(): + return false, fmt.Errorf("browser.on promise rejected: %w", b.vuCtx.Err()) } } @@ -660,7 +668,7 @@ func (b *Browser) fetchVersion() (browserVersion, error) { ) bv.protocolVersion, bv.product, bv.revision, bv.userAgent, bv.jsVersion, err = cdpbrowser. GetVersion(). - Do(cdp.WithExecutor(b.ctx, b.conn)) + Do(cdp.WithExecutor(b.vuCtx, b.conn)) if err != nil { return browserVersion{}, fmt.Errorf("getting browser version information: %w", err) } diff --git a/vendor/github.com/grafana/xk6-browser/common/frame_manager.go b/vendor/github.com/grafana/xk6-browser/common/frame_manager.go index ae41ef6960d..617f20f9eb0 100644 --- a/vendor/github.com/grafana/xk6-browser/common/frame_manager.go +++ b/vendor/github.com/grafana/xk6-browser/common/frame_manager.go @@ -577,6 +577,14 @@ func (m *FrameManager) setMainFrame(f *Frame) { m.mainFrame = f } +// MainFrameURL returns the main frame's url. +func (m *FrameManager) MainFrameURL() string { + m.mainFrameMu.RLock() + defer m.mainFrameMu.RUnlock() + + return m.mainFrame.URL() +} + // NavigateFrame will navigate specified frame to specified URL. // //nolint:funlen,cyclop diff --git a/vendor/github.com/grafana/xk6-browser/common/frame_session.go b/vendor/github.com/grafana/xk6-browser/common/frame_session.go index b7ae24a8a6c..370d019f9e1 100644 --- a/vendor/github.com/grafana/xk6-browser/common/frame_session.go +++ b/vendor/github.com/grafana/xk6-browser/common/frame_session.go @@ -78,6 +78,11 @@ type FrameSession struct { // Keep a reference to the main frame span so we can end it // when FrameSession.ctx is Done mainFrameSpan trace.Span + // The initial navigation when a new page is created navigates to about:blank. + // We want to make sure that the the navigation span is created for this in + // onFrameNavigated, but subsequent calls to onFrameNavigated in the same + // mainframe never again create a navigation span. + initialNavDone bool } // NewFrameSession initializes and returns a new FrameSession. @@ -237,6 +242,11 @@ func (fs *FrameSession) initEvents() { // If there is an active span for main frame, // end it before exiting so it can be flushed if fs.mainFrameSpan != nil { + // The url needs to be added here instead of at the start of the span + // because at the start of the span we don't know the correct url for + // the page we're navigating to. At the end of the span we do have this + // information. + fs.mainFrameSpan.SetAttributes(attribute.String("navigation.url", fs.manager.MainFrameURL())) fs.mainFrameSpan.End() fs.mainFrameSpan = nil } @@ -782,25 +792,44 @@ func (fs *FrameSession) onFrameNavigated(frame *cdp.Frame, initial bool) { frame.URL+frame.URLFragment, err) } + // Only create a navigation span once from here, since a new page navigating + // to about:blank doesn't call onFrameStartedLoading. All subsequent + // navigations call onFrameStartedLoading. + if fs.initialNavDone { + return + } + + fs.initialNavDone = true + fs.processNavigationSpan(frame.ID) +} + +func (fs *FrameSession) processNavigationSpan(id cdp.FrameID) { + newFrame, ok := fs.manager.getFrameByID(id) + if !ok { + return + } + // Trace navigation only for the main frame. // TODO: How will this affect sub frames such as iframes? - if isMainFrame := frame.ParentID == ""; !isMainFrame { + if newFrame.page.frameManager.MainFrame() != newFrame { return } + // End the navigation span if it is non-nil + if fs.mainFrameSpan != nil { + // The url needs to be added here instead of at the start of the span + // because at the start of the span we don't know the correct url for + // the page we're navigating to. At the end of the span we do have this + // information. + fs.mainFrameSpan.SetAttributes(attribute.String("navigation.url", fs.manager.MainFrameURL())) + fs.mainFrameSpan.End() + } + _, fs.mainFrameSpan = TraceNavigation( - fs.ctx, fs.targetID.String(), trace.WithAttributes(attribute.String("navigation.url", frame.URL)), + fs.ctx, fs.targetID.String(), ) - var ( - spanID = fs.mainFrameSpan.SpanContext().SpanID().String() - newFrame, ok = fs.manager.getFrameByID(frame.ID) - ) - - // Only set the k6SpanId reference if it's a new frame. - if !ok { - return - } + spanID := fs.mainFrameSpan.SpanContext().SpanID().String() // Set k6SpanId property in the page so it can be retrieved when pushing // the Web Vitals events from the page execution context and used to @@ -844,6 +873,8 @@ func (fs *FrameSession) onFrameStartedLoading(frameID cdp.FrameID) { "sid:%v tid:%v fid:%v", fs.session.ID(), fs.targetID, frameID) + fs.processNavigationSpan(frameID) + fs.manager.frameLoadingStarted(frameID) } diff --git a/vendor/github.com/grafana/xk6-browser/storage/file_persister.go b/vendor/github.com/grafana/xk6-browser/storage/file_persister.go index 04922851bd7..038f8f624a8 100644 --- a/vendor/github.com/grafana/xk6-browser/storage/file_persister.go +++ b/vendor/github.com/grafana/xk6-browser/storage/file_persister.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "mime/multipart" "net/http" "os" "path/filepath" @@ -52,27 +53,38 @@ func (l *LocalFilePersister) Persist(_ context.Context, path string, data io.Rea } // RemoteFilePersister is to be used when files created by the browser module need -// to be uploaded to a remote location. This uses a preSignedURLGetterURL to -// retrieve one pre-signed URL. The pre-signed url is used to upload the file +// to be uploaded to a remote location. This uses a presignedURLRequestURL to +// retrieve one presigned URL. The presigned url is used to upload the file // to the remote location. type RemoteFilePersister struct { - preSignedURLGetterURL string - headers map[string]string - basePath string + presignedURLRequestURL string + headers map[string]string + basePath string httpClient *http.Client } +// PresignedURLResponse holds the response from a presigned generation request. +type PresignedURLResponse struct { + Service string `json:"service"` + URLs []struct { + Name string `json:"name"` + PresignedURL string `json:"pre_signed_url"` //nolint:tagliatelle + Method string `json:"method"` + FormFields map[string]string `json:"form_fields"` //nolint:tagliatelle + } `json:"urls"` +} + // NewRemoteFilePersister creates a new instance of RemoteFilePersister. func NewRemoteFilePersister( - preSignedURLGetterURL string, + presignedURLRequestURL string, headers map[string]string, basePath string, ) *RemoteFilePersister { return &RemoteFilePersister{ - preSignedURLGetterURL: preSignedURLGetterURL, - headers: headers, - basePath: basePath, + presignedURLRequestURL: presignedURLRequestURL, + headers: headers, + basePath: basePath, httpClient: &http.Client{ Timeout: time.Second * 10, }, @@ -81,12 +93,12 @@ func NewRemoteFilePersister( // Persist will upload the contents of data to a remote location. func (r *RemoteFilePersister) Persist(ctx context.Context, path string, data io.Reader) (err error) { - pURL, err := r.getPreSignedURL(ctx, path) + psResp, err := r.requestPresignedURL(ctx, path) if err != nil { - return fmt.Errorf("getting presigned url: %w", err) + return fmt.Errorf("requesting presigned url: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodPut, pURL, data) + req, err := newFileUploadRequest(ctx, psResp, data) if err != nil { return fmt.Errorf("creating upload request: %w", err) } @@ -108,24 +120,22 @@ func (r *RemoteFilePersister) Persist(ctx context.Context, path string, data io. return nil } -func checkStatusCode(resp *http.Response) error { - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - return fmt.Errorf("server returned %d (%s)", resp.StatusCode, strings.ToLower(http.StatusText(resp.StatusCode))) - } - - return nil -} - -// getPreSignedURL will retrieve the presigned url for the current file. -func (r *RemoteFilePersister) getPreSignedURL(ctx context.Context, path string) (string, error) { +// requestPresignedURL will request a new presigned URL from the remote server +// and returns a [PresignedURLResponse] that contains the presigned URL details. +func (r *RemoteFilePersister) requestPresignedURL(ctx context.Context, path string) (PresignedURLResponse, error) { b, err := buildPresignedRequestBody(r.basePath, path) if err != nil { - return "", fmt.Errorf("building request body: %w", err) + return PresignedURLResponse{}, fmt.Errorf("building request body: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, r.preSignedURLGetterURL, bytes.NewReader(b)) + req, err := http.NewRequestWithContext( + ctx, + http.MethodPost, + r.presignedURLRequestURL, + bytes.NewReader(b), + ) if err != nil { - return "", fmt.Errorf("creating request: %w", err) + return PresignedURLResponse{}, fmt.Errorf("creating request: %w", err) } for k, v := range r.headers { @@ -134,15 +144,15 @@ func (r *RemoteFilePersister) getPreSignedURL(ctx context.Context, path string) resp, err := r.httpClient.Do(req) if err != nil { - return "", fmt.Errorf("performing request: %w", err) + return PresignedURLResponse{}, fmt.Errorf("performing request: %w", err) } defer resp.Body.Close() //nolint:errcheck if err := checkStatusCode(resp); err != nil { - return "", err + return PresignedURLResponse{}, err } - return readResponseBody(resp) + return readPresignedURLResponse(resp) } func buildPresignedRequestBody(basePath, path string) ([]byte, error) { @@ -154,7 +164,7 @@ func buildPresignedRequestBody(basePath, path string) ([]byte, error) { } `json:"files"` }{ Service: "aws_s3", - Operation: "upload", + Operation: "upload_post", Files: []struct { Name string `json:"name"` }{ @@ -172,24 +182,71 @@ func buildPresignedRequestBody(basePath, path string) ([]byte, error) { return bb, nil } -func readResponseBody(resp *http.Response) (string, error) { - rb := struct { - Service string `json:"service"` - URLs []struct { - Name string `json:"name"` - PreSignedURL string `json:"pre_signed_url"` //nolint:tagliatelle - } `json:"urls"` - }{} +func readPresignedURLResponse(resp *http.Response) (PresignedURLResponse, error) { + var rb PresignedURLResponse decoder := json.NewDecoder(resp.Body) err := decoder.Decode(&rb) if err != nil { - return "", fmt.Errorf("decoding response body: %w", err) + return PresignedURLResponse{}, fmt.Errorf("decoding response body: %w", err) } if len(rb.URLs) == 0 { - return "", errors.New("missing presigned url in response body") + return PresignedURLResponse{}, errors.New("missing presigned url in response body") + } + + return rb, nil +} + +// newFileUploadRequest creates a new HTTP request to upload a file as a multipart +// form to the presigned URL received from the server. +func newFileUploadRequest( + ctx context.Context, + resp PresignedURLResponse, + data io.Reader, +) (*http.Request, error) { + // we don't support multiple presigned URLs at the moment. + psu := resp.URLs[0] + + // copy all form fields received from a presigned URL + // response to the multipart form fields. + var form bytes.Buffer + fw := multipart.NewWriter(&form) + for k, v := range psu.FormFields { + if err := fw.WriteField(k, v); err != nil { + return nil, fmt.Errorf("writing form field key %q and value %q: %w", k, v, err) + } + } + // attach the file data to the form. + ff, err := fw.CreateFormFile("file", psu.Name) + if err != nil { + return nil, fmt.Errorf("creating multipart form file: %w", err) + } + if _, err := io.Copy(ff, data); err != nil { + return nil, fmt.Errorf("copying file data to multipart form: %w", err) + } + if err := fw.Close(); err != nil { + return nil, fmt.Errorf("closing multipart form writer: %w", err) } - return rb.URLs[0].PreSignedURL, nil + req, err := http.NewRequestWithContext( + ctx, + psu.Method, + psu.PresignedURL, + &form, + ) + if err != nil { + return nil, fmt.Errorf("creating new request: %w", err) + } + req.Header.Set("Content-Type", fw.FormDataContentType()) + + return req, nil +} + +func checkStatusCode(resp *http.Response) error { + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + return fmt.Errorf("server returned %d (%s)", resp.StatusCode, strings.ToLower(http.StatusText(resp.StatusCode))) + } + + return nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index 9162d446b7c..f949f372a4e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -173,8 +173,8 @@ github.com/grafana/sobek/ftoa/internal/fast github.com/grafana/sobek/parser github.com/grafana/sobek/token github.com/grafana/sobek/unistring -# github.com/grafana/xk6-browser v1.7.1 -## explicit; go 1.20 +# github.com/grafana/xk6-browser v1.8.0 +## explicit; go 1.21 github.com/grafana/xk6-browser/browser github.com/grafana/xk6-browser/chromium github.com/grafana/xk6-browser/common