From d99be7f960a81b4e52b619872a7728eed824bb48 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Fri, 5 Apr 2024 10:23:54 +0100 Subject: [PATCH 1/3] Refactor NewContext to return errors --- common/browser.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/browser.go b/common/browser.go index e8680615f..ca54727ac 100644 --- a/common/browser.go +++ b/common/browser.go @@ -557,12 +557,12 @@ func (b *Browser) NewContext(opts goja.Value) (*BrowserContext, error) { browserContextID, err := action.Do(cdp.WithExecutor(b.ctx, b.conn)) b.logger.Debugf("Browser:NewContext", "bctxid:%v", browserContextID) if err != nil { - k6ext.Panic(b.ctx, "creating browser context ID %s: %w", browserContextID, err) + return nil, fmt.Errorf("creating browser context ID %s: %w", browserContextID, err) } browserCtxOpts := NewBrowserContextOptions() if err := browserCtxOpts.Parse(b.ctx, opts); err != nil { - k6ext.Panic(b.ctx, "parsing newContext options: %w", err) + return nil, fmt.Errorf("parsing newContext options: %w", err) } browserCtx, err := NewBrowserContext(b.ctx, b, browserContextID, browserCtxOpts, b.logger) From fd5f5c606da98769b7bd352567c74faad89a2ac9 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Fri, 5 Apr 2024 10:27:55 +0100 Subject: [PATCH 2/3] Refactor locator.Type to return an error --- common/locator.go | 19 +++++++++---------- tests/locator_test.go | 8 +++++++- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/common/locator.go b/common/locator.go index 6f859cb67..c2062c9a2 100644 --- a/common/locator.go +++ b/common/locator.go @@ -509,7 +509,7 @@ func (l *Locator) press(key string, opts *FramePressOptions) error { // Type text on the element found that matches the locator's // selector with strict mode on. -func (l *Locator) Type(text string, opts goja.Value) { +func (l *Locator) Type(text string, opts goja.Value) error { l.log.Debugf( "Locator:Type", "fid:%s furl:%q sel:%q text:%q opts:%+v", l.frame.ID(), l.frame.URL(), l.selector, text, opts, @@ -517,18 +517,17 @@ func (l *Locator) Type(text string, opts goja.Value) { _, span := TraceAPICall(l.ctx, l.frame.page.targetID.String(), "locator.type") defer span.End() - var err error - defer func() { panicOrSlowMo(l.ctx, err) }() - copts := NewFrameTypeOptions(l.frame.defaultTimeout()) - if err = copts.Parse(l.ctx, opts); err != nil { - err = fmt.Errorf("parsing type options: %w", err) - return + if err := copts.Parse(l.ctx, opts); err != nil { + return fmt.Errorf("parsing type options: %w", err) } - if err = l.typ(text, copts); err != nil { - err = fmt.Errorf("typing %q in %q: %w", text, l.selector, err) - return + if err := l.typ(text, copts); err != nil { + return fmt.Errorf("typing %q in %q: %w", text, l.selector, err) } + + applySlowMo(l.ctx) + + return nil } func (l *Locator) typ(text string, opts *FrameTypeOptions) error { diff --git a/tests/locator_test.go b/tests/locator_test.go index 65b44c76b..55718953a 100644 --- a/tests/locator_test.go +++ b/tests/locator_test.go @@ -342,7 +342,13 @@ func TestLocator(t *testing.T) { }, }, { - "Type", func(l *common.Locator, tb *testBrowser) { l.Type("a", timeout(tb)) }, + "Type", func(l *common.Locator, tb *testBrowser) { + err := l.Type("a", timeout(tb)) + if err != nil { + // TODO: remove panic and update tests when all locator methods return error. + panic(err) + } + }, }, { "TextContent", func(l *common.Locator, tb *testBrowser) { l.TextContent(timeout(tb)) }, From 3ddc39e32f30657017bb2ade3e9c1d7cded338fa Mon Sep 17 00:00:00 2001 From: ankur22 Date: Fri, 5 Apr 2024 10:37:02 +0100 Subject: [PATCH 3/3] Refactor reload to return an error --- browser/mapping.go | 12 +++++++++--- common/page.go | 12 ++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/browser/mapping.go b/browser/mapping.go index c58dc564c..914c23dba 100644 --- a/browser/mapping.go +++ b/browser/mapping.go @@ -692,9 +692,15 @@ func mapPage(vu moduleVU, p *common.Page) mapping { "pause": p.Pause, "pdf": p.Pdf, "press": p.Press, - "reload": func(opts goja.Value) *goja.Object { - r := mapResponse(vu, p.Reload(opts)) - return rt.ToValue(r).ToObject(rt) + "reload": func(opts goja.Value) (*goja.Object, error) { + resp, err := p.Reload(opts) + if err != nil { + return nil, err //nolint:wrapcheck + } + + r := mapResponse(vu, resp) + + return rt.ToValue(r).ToObject(rt), nil }, "route": p.Route, "screenshot": func(opts goja.Value) (*goja.ArrayBuffer, error) { diff --git a/common/page.go b/common/page.go index db547acbe..766710b19 100644 --- a/common/page.go +++ b/common/page.go @@ -1034,7 +1034,7 @@ func (p *Page) QueryAll(selector string) ([]*ElementHandle, error) { } // Reload will reload the current page. -func (p *Page) Reload(opts goja.Value) *Response { //nolint:funlen,cyclop +func (p *Page) Reload(opts goja.Value) (*Response, error) { //nolint:funlen,cyclop p.logger.Debugf("Page:Reload", "sid:%v", p.sessionID()) _, span := TraceAPICall(p.ctx, p.targetID.String(), "page.reload") defer span.End() @@ -1044,7 +1044,7 @@ func (p *Page) Reload(opts goja.Value) *Response { //nolint:funlen,cyclop p.timeoutSettings.navigationTimeout(), ) if err := parsedOpts.Parse(p.ctx, opts); err != nil { - k6ext.Panic(p.ctx, "parsing reload options: %w", err) + return nil, fmt.Errorf("parsing reload options: %w", err) } timeoutCtx, timeoutCancelFn := context.WithTimeout(p.ctx, parsedOpts.Timeout) @@ -1070,7 +1070,7 @@ func (p *Page) Reload(opts goja.Value) *Response { //nolint:funlen,cyclop action := cdppage.Reload() if err := action.Do(cdp.WithExecutor(p.ctx, p.session)); err != nil { - k6ext.Panic(p.ctx, "reloading page: %w", err) + return nil, fmt.Errorf("reloading page: %w", err) } wrapTimeoutError := func(err error) error { @@ -1090,7 +1090,7 @@ func (p *Page) Reload(opts goja.Value) *Response { //nolint:funlen,cyclop select { case <-p.ctx.Done(): case <-timeoutCtx.Done(): - k6ext.Panic(p.ctx, "%w", wrapTimeoutError(timeoutCtx.Err())) + return nil, wrapTimeoutError(timeoutCtx.Err()) case data := <-ch: event = data.(*NavigationEvent) } @@ -1106,12 +1106,12 @@ func (p *Page) Reload(opts goja.Value) *Response { //nolint:funlen,cyclop select { case <-lifecycleEvtCh: case <-timeoutCtx.Done(): - k6ext.Panic(p.ctx, "%w", wrapTimeoutError(timeoutCtx.Err())) + return nil, wrapTimeoutError(timeoutCtx.Err()) } applySlowMo(p.ctx) - return resp + return resp, nil } // Route is not implemented.