diff --git a/common/browser.go b/common/browser.go index ca54727ac..bcb320f3c 100644 --- a/common/browser.go +++ b/common/browser.go @@ -550,24 +550,32 @@ func (b *Browser) NewContext(opts goja.Value) (*BrowserContext, error) { defer span.End() if b.context != nil { - return nil, errors.New("existing browser context must be closed before creating a new one") + err := errors.New("existing browser context must be closed before creating a new one") + spanRecordError(span, err) + return nil, err } action := target.CreateBrowserContext().WithDisposeOnDetach(true) browserContextID, err := action.Do(cdp.WithExecutor(b.ctx, b.conn)) b.logger.Debugf("Browser:NewContext", "bctxid:%v", browserContextID) if err != nil { - return nil, fmt.Errorf("creating browser context ID %s: %w", browserContextID, err) + err := fmt.Errorf("creating browser context ID %s: %w", browserContextID, err) + spanRecordError(span, err) + return nil, err } browserCtxOpts := NewBrowserContextOptions() if err := browserCtxOpts.Parse(b.ctx, opts); err != nil { - return nil, fmt.Errorf("parsing newContext options: %w", err) + err := fmt.Errorf("parsing newContext options: %w", err) + spanRecordError(span, err) + return nil, err } browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, browserCtxOpts, b.logger) if err != nil { - return nil, fmt.Errorf("new context: %w", err) + err := fmt.Errorf("new context: %w", err) + spanRecordError(span, err) + return nil, err } b.contextMu.Lock() @@ -584,10 +592,18 @@ func (b *Browser) NewPage(opts goja.Value) (*Page, error) { browserCtx, err := b.NewContext(opts) if err != nil { - return nil, fmt.Errorf("new page: %w", err) + err := fmt.Errorf("new page: %w", err) + spanRecordError(span, err) + return nil, err + } + + page, err := browserCtx.NewPage() + if err != nil { + spanRecordError(span, err) + return nil, err } - return browserCtx.NewPage() + return page, nil } // On returns a Promise that is resolved when the browser process is disconnected. diff --git a/common/browser_context.go b/common/browser_context.go index d543d20b3..09db5b44d 100644 --- a/common/browser_context.go +++ b/common/browser_context.go @@ -219,7 +219,9 @@ func (b *BrowserContext) NewPage() (*Page, error) { p, err := b.browser.newPageInContext(b.id) if err != nil { - return nil, fmt.Errorf("creating new page in browser context: %w", err) + err := fmt.Errorf("creating new page in browser context: %w", err) + spanRecordError(span, err) + return nil, err } var ( diff --git a/common/element_handle.go b/common/element_handle.go index 6bfc3dde3..5eebcb6d8 100644 --- a/common/element_handle.go +++ b/common/element_handle.go @@ -1183,7 +1183,9 @@ func (h *ElementHandle) Screenshot( s := newScreenshotter(spanCtx, sp) buf, err := s.screenshotElement(h, opts) if err != nil { - return nil, fmt.Errorf("taking screenshot of elementHandle: %w", err) + err := fmt.Errorf("taking screenshot of elementHandle: %w", err) + spanRecordError(span, err) + return nil, err } return buf, err diff --git a/common/locator.go b/common/locator.go index c2062c9a2..acc1b6fdd 100644 --- a/common/locator.go +++ b/common/locator.go @@ -64,7 +64,9 @@ func (l *Locator) Click(opts *FrameClickOptions) error { defer span.End() if err := l.click(opts); err != nil { - return fmt.Errorf("clicking on %q: %w", l.selector, err) + err := fmt.Errorf("clicking on %q: %w", l.selector, err) + spanRecordError(span, err) + return err } applySlowMo(l.ctx) @@ -519,10 +521,14 @@ func (l *Locator) Type(text string, opts goja.Value) error { copts := NewFrameTypeOptions(l.frame.defaultTimeout()) if err := copts.Parse(l.ctx, opts); err != nil { - return fmt.Errorf("parsing type options: %w", err) + err := fmt.Errorf("parsing type options: %w", err) + spanRecordError(span, err) + return err } if err := l.typ(text, copts); err != nil { - return fmt.Errorf("typing %q in %q: %w", text, l.selector, err) + err := fmt.Errorf("typing %q in %q: %w", text, l.selector, err) + spanRecordError(span, err) + return err } applySlowMo(l.ctx) diff --git a/common/page.go b/common/page.go index 3c9074a6a..39fa13933 100644 --- a/common/page.go +++ b/common/page.go @@ -657,7 +657,9 @@ func (p *Page) Close(_ goja.Value) error { add := runtime.RemoveBinding(webVitalBinding) if err := add.Do(cdp.WithExecutor(p.ctx, p.session)); err != nil { - return fmt.Errorf("internal error while removing binding from page: %w", err) + err := fmt.Errorf("internal error while removing binding from page: %w", err) + spanRecordError(span, err) + return err } action := target.CloseTarget(p.targetID) @@ -676,7 +678,9 @@ func (p *Page) Close(_ goja.Value) error { return nil } - return fmt.Errorf("closing a page: %w", err) + err := fmt.Errorf("closing a page: %w", err) + spanRecordError(span, err) + return err } return nil @@ -826,7 +830,13 @@ func (p *Page) Goto(url string, opts *FrameGotoOptions) (*Response, error) { ) defer span.End() - return p.MainFrame().Goto(url, opts) + resp, err := p.MainFrame().Goto(url, opts) + if err != nil { + spanRecordError(span, err) + return nil, err + } + + return resp, nil } func (p *Page) Hover(selector string, opts goja.Value) { @@ -985,7 +995,9 @@ func (p *Page) Reload(opts goja.Value) (*Response, error) { //nolint:funlen,cycl p.timeoutSettings.navigationTimeout(), ) if err := parsedOpts.Parse(p.ctx, opts); err != nil { - return nil, fmt.Errorf("parsing reload options: %w", err) + err := fmt.Errorf("parsing reload options: %w", err) + spanRecordError(span, err) + return nil, err } timeoutCtx, timeoutCancelFn := context.WithTimeout(p.ctx, parsedOpts.Timeout) @@ -1011,7 +1023,9 @@ func (p *Page) Reload(opts goja.Value) (*Response, error) { //nolint:funlen,cycl action := cdppage.Reload() if err := action.Do(cdp.WithExecutor(p.ctx, p.session)); err != nil { - return nil, fmt.Errorf("reloading page: %w", err) + err := fmt.Errorf("reloading page: %w", err) + spanRecordError(span, err) + return nil, err } wrapTimeoutError := func(err error) error { @@ -1031,7 +1045,9 @@ func (p *Page) Reload(opts goja.Value) (*Response, error) { //nolint:funlen,cycl select { case <-p.ctx.Done(): case <-timeoutCtx.Done(): - return nil, wrapTimeoutError(timeoutCtx.Err()) + err := wrapTimeoutError(timeoutCtx.Err()) + spanRecordError(span, err) + return nil, err case data := <-ch: event = data.(*NavigationEvent) } @@ -1047,7 +1063,9 @@ func (p *Page) Reload(opts goja.Value) (*Response, error) { //nolint:funlen,cycl select { case <-lifecycleEvtCh: case <-timeoutCtx.Done(): - return nil, wrapTimeoutError(timeoutCtx.Err()) + err := wrapTimeoutError(timeoutCtx.Err()) + spanRecordError(span, err) + return nil, err } applySlowMo(p.ctx) @@ -1065,7 +1083,9 @@ func (p *Page) Screenshot(opts *PageScreenshotOptions, sp ScreenshotPersister) ( s := newScreenshotter(spanCtx, sp) buf, err := s.screenshotPage(p, opts) if err != nil { - return nil, fmt.Errorf("taking screenshot of page: %w", err) + err := fmt.Errorf("taking screenshot of page: %w", err) + spanRecordError(span, err) + return nil, err } return buf, err @@ -1234,7 +1254,13 @@ func (p *Page) WaitForNavigation(opts *FrameWaitForNavigationOptions) (*Response _, span := TraceAPICall(p.ctx, p.targetID.String(), "page.waitForNavigation") defer span.End() - return p.frameManager.MainFrame().WaitForNavigation(opts) + resp, err := p.frameManager.MainFrame().WaitForNavigation(opts) + if err != nil { + spanRecordError(span, err) + return nil, err + } + + return resp, err } // WaitForSelector waits for the given selector to match the waiting criteria. diff --git a/common/trace.go b/common/trace.go index 9b8f71606..fea5edd27 100644 --- a/common/trace.go +++ b/common/trace.go @@ -3,6 +3,7 @@ package common import ( "context" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" browsertrace "github.com/grafana/xk6-browser/trace" @@ -56,3 +57,11 @@ func TraceEvent( } return ctx, browsertrace.NoopSpan{} } + +// spanRecordError will set the status of the span to error and record the +// error on the span. Check the documentation for trace.SetStatus and +// trace.RecordError for more details. +func spanRecordError(span trace.Span, err error) { + span.SetStatus(codes.Error, err.Error()) + span.RecordError(err) +}