Add error handling to wildcard selectors #820

inancgumus · 2023-03-09T12:50:44Z

This allows us to:

Better handle extension errors.
Leaves the decision of properly handling errors to Goja when an element is not found. This is how it's done in the k6-core and other extensions.

Returning an error from the mapping layer also allows us to prevent multiple unnecesary panics.

You can use this script to test the behavior (main and this branch)

import { check } from 'k6';
import { chromium } from 'k6/x/browser';

export const options = {
  thresholds: {
    checks: ["rate==1.0"]
  }
}

export default async function() {
  const browser = chromium.launch({
    headless: __ENV.XK6_HEADLESS ? true : false,
  });
  const context = browser.newContext();
  const page = context.newPage();

  try {
    await page.goto('https://test.k6.io/');

    check(page, {
      'Title with CSS selector':
        p => {
          console.log('checking page textcontext');
          return p.$('header h1.titlex').textContent() == 'test.k6.io'
        },
    });
  } finally {
    page.close();
    browser.close();
  }
}

Error output (current v0.8.0)

ERRO[0001] panic: runtime error: invalid memory address or nil pointer dereference
goroutine 68 [running]:
runtime/debug.Stack()
        runtime/debug/stack.go:24 +0x64
go.k6.io/k6/js/common.RunWithPanicCatching.func1()
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/common/util.go:82 +0x190
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*Runtime).runWrapped.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2480 +0x168
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*vm).handleThrow(0x140005fc240, {0x101be1820, 0x102a6d620})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:730 +0x334
github.com/dop251/goja.(*vm).try.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:749 +0x48
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*vm).handleThrow(0x140005fc240, {0x101be1820, 0x102a6d620})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:730 +0x334
github.com/dop251/goja.(*vm).runTryInner.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:772 +0x48
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*Runtime).runWrapped.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2480 +0x168
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*vm).handleThrow(0x140005fc240, {0x101be1820, 0x102a6d620})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:730 +0x334
github.com/dop251/goja.(*vm).try.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:749 +0x48
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/dop251/goja.(*vm).handleThrow(0x140005fc240, {0x101be1820, 0x102a6d620})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:730 +0x334
github.com/dop251/goja.(*vm).runTryInner.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:772 +0x48
panic({0x101be1820, 0x102a6d620})
        runtime/panic.go:884 +0x1f4
github.com/grafana/xk6-browser/browser.mapElementHandle({{0x101debdc0?, 0x14001a0c990?}, 0x140004127e0?}, {0x0?, 0x0})
        github.com/grafana/xk6-browser@v0.0.0-00010101000000-000000000000/browser/mapping.go:198 +0x70
github.com/grafana/xk6-browser/browser.mapPage.func12({0x1400119c400?, 0x1?})
        github.com/grafana/xk6-browser@v0.0.0-00010101000000-000000000000/browser/mapping.go:545 +0x70
reflect.Value.call({0x101b8d9e0?, 0x140006a8900?, 0x10a3c1688?}, {0x1017d290b, 0x4}, {0x140005c6bb8, 0x1, 0x0?})
        reflect/value.go:586 +0x838
reflect.Value.Call({0x101b8d9e0?, 0x140006a8900?, 0x1400119f880?}, {0x140005c6bb8?, 0x140025148e0?, 0x280?})
        reflect/value.go:370 +0x90
github.com/dop251/goja.(*Runtime).wrapReflectFunc.func1({{0x101df6c98, 0x14001f6aba0}, {0x140025602b0, 0x1, 0xd}})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2020 +0x2d8
github.com/dop251/goja.(*nativeFuncObject).vmCall(0x14001c9ad80, 0x140005fc240, 0x1)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:511 +0x178
github.com/dop251/goja.call.exec(0x5fc240?, 0x140005fc240)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:3308 +0x74
github.com/dop251/goja.(*vm).run(0x140005fc240)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:567 +0x40
github.com/dop251/goja.(*vm).runTryInner(0x140005fc240?)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:776 +0x58
github.com/dop251/goja.(*baseJsFuncObject).__call(0x140025421c0, {0x140025147b0?, 0x1, 0x140010023e0?}, {0x0?, 0x0}, {0x0?, 0x0?})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:378 +0x638
github.com/dop251/goja.(*baseJsFuncObject)._call(...)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:394
github.com/dop251/goja.(*arrowFuncObject).Call(0x101baea60?, {{0x101df7580, 0x102ac5f40}, {0x140025147b0, 0x1, 0x1}})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:338 +0x54
github.com/dop251/goja.AssertFunction.func1.1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2440 +0x6c
github.com/dop251/goja.(*vm).try(0x140005fc240, 0x14001ca2508)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:753 +0x1dc
github.com/dop251/goja.(*Runtime).runWrapped(0x140000bc800, 0x102fe0f18?)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2484 +0x64
github.com/dop251/goja.AssertFunction.func1({0x101df7580?, 0x102ac5f40?}, {0x140025147b0?, 0x14002782120?, 0x17?})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2439 +0x78
go.k6.io/k6/js/modules/k6.(*K6).Check(0x14001c94170, {0x101df6c98?, 0x14001f6aba0}, {0x101df6c98, 0x1400252a720}, {0x102ac6318, 0x0, 0x100a46570?})
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/modules/k6/k6.go:182 +0x2f4
reflect.Value.call({0x101bdd260?, 0x14001c94180?, 0x10a3c1688?}, {0x1017d290b, 0x4}, {0x1400252a810, 0x2, 0x0?})
        reflect/value.go:586 +0x838
reflect.Value.Call({0x101bdd260?, 0x14001c94180?, 0x1400252a720?}, {0x1400252a810?, 0x140025146d0?, 0x1009b3fa4?})
        reflect/value.go:370 +0x90
github.com/dop251/goja.(*Runtime).wrapReflectFunc.func1({{0x101df7580, 0x102ac5f40}, {0x14002542160, 0x2, 0x6}})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2020 +0x2d8
github.com/dop251/goja.(*nativeFuncObject).vmCall(0x14001c9ac00, 0x140005fc240, 0x2)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:511 +0x178
github.com/dop251/goja.call.exec(0x5fc240?, 0x140005fc240)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:3308 +0x74
github.com/dop251/goja.(*vm).run(0x140005fc240)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:567 +0x40
github.com/dop251/goja.(*vm).runTryInner(0x140012a3530?)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:776 +0x58
github.com/dop251/goja.(*generator).step(0x14001da01e0)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:722 +0x30
github.com/dop251/goja.(*generator).next(0x14001da01e0, {0x101df6c98, 0x1400252a5d0})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:756 +0x16c
github.com/dop251/goja.(*asyncRunner).onFulfilled(0x14001da01e0, {{0x101df7580, 0x102ac5f40}, {0x14002514660, 0x1, 0x1}})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/func.go:636 +0x128
github.com/dop251/goja.(*Runtime).callJobCallback(...)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2898
github.com/dop251/goja.(*Runtime).newPromiseReactionJob.func1.1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/builtin_promise.go:204 +0xe0
github.com/dop251/goja.(*vm).try(0x140005fc240, 0x14001ca3500)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/vm.go:753 +0x1dc
github.com/dop251/goja.(*Runtime).newPromiseReactionJob.func1()
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/builtin_promise.go:203 +0xa0
github.com/dop251/goja.(*Runtime).leave(0x140000bc800)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2771 +0xec
github.com/dop251/goja.(*Runtime).runWrapped(0x140000bc800, 0x102fe0f18?)
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2489 +0xf0
github.com/dop251/goja.AssertFunction.func1({0x0?, 0x0?}, {0x14002514650?, 0x0?, 0x0?})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/runtime.go:2439 +0x78
github.com/dop251/goja.(*Runtime).wrapPromiseReaction.func1({0x101bd6a40?, 0x1400252a5a0?})
        github.com/dop251/goja@v0.0.0-20230128084908-78b980256d04/builtin_promise.go:576 +0xa8
github.com/grafana/xk6-browser/k6ext.promise.func1.1()
        github.com/grafana/xk6-browser@v0.0.0-00010101000000-000000000000/k6ext/promise.go:46 +0x70
go.k6.io/k6/js/eventloop.(*EventLoop).Start(0x140005f85f0, 0x14001d12a20)
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/eventloop/eventloop.go:171 +0x164
go.k6.io/k6/js.(*VU).runFn.func2()
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/runner.go:795 +0xe4
go.k6.io/k6/js/common.RunWithPanicCatching({0x101dfa9e8?, 0x1400070aa80?}, 0x14001b45200?, 0x14001b45288?)
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/common/util.go:86 +0x74
go.k6.io/k6/js.(*VU).runFn(0x14001c9a780, {0x101de9368, 0x140005f9680}, 0x80?, 0x140000b0ab0, 0x14001c94d60, {0x14001c94d70, 0x1, 0x1})
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/runner.go:794 +0x1e8
go.k6.io/k6/js.(*ActiveVU).RunOnce(0x1400067ae40)
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/js/runner.go:739 +0x348
go.k6.io/k6/lib/executor.getIterationRunner.func1({0x101de9410, 0x14001d12810}, {0x101dddcf8?, 0x1400067ae40?})
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/lib/executor/helpers.go:81 +0x4c
go.k6.io/k6/lib/executor.PerVUIterations.Run.func5({0x101de4508, 0x14001c9a780})
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/lib/executor/per_vu_iterations.go:227 +0x378
created by go.k6.io/k6/lib/executor.PerVUIterations.Run
        go.k6.io/k6@v0.42.1-0.20230130080633-582ec4d3940c/lib/executor/per_vu_iterations.go:240 +0x9bc

Goja stack: 
ERRO[0001] communicating with browser: websocket: close 1006 (abnormal closure): unexpected EOF  category=cdp elapsed="0 ms" goroutine=16
ERRO[0001] process with PID 5864 unexpectedly ended: signal: killed  category=browser elapsed="0 ms" goroutine=72

Error output (this PR)

ERRO[0001] Uncaught (in promise) GoError: querying selector "header h1.titlex"
        at github.com/grafana/xk6-browser/browser.mapPage.func12 (native)
        at Title with CSS selector (file:///Users/inanc/grafana/k6browser/tests/issue814.js:24:21(9))
        at go.k6.io/k6/js/modules/k6.(*K6).Check-fm (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue814.js:20:10(36)  executor=per-vu-iterations scenario=default

The error output if we panicked in the mapping layer, instead.

ERRO[0001] communicating with browser: read tcp 127.0.0.1:56064->127.0.0.1:56063: read: connection reset by peer  category=cdp elapsed="0 ms" goroutine=82
ERRO[0001] process with PID 6834 unexpectedly ended: signal: killed  category=browser elapsed="0 ms" goroutine=47
ERRO[0001] Uncaught (in promise) GoError: disposing browser context: disposing browser context ID 51E2EDDC7717E43861E3C62287000B86: read tcp 127.0.0.1:56064->127.0.0.1:56063: read: connection reset by peer
        at github.com/grafana/xk6-browser/api.Page.Close-fm (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue814.js:28:4(41)  executor=per-vu-iterations scenario=default

The error from $ is swallowed. Even if we silence the close panic and log, this happens:

ERRO[0001] communicating with browser: read tcp 127.0.0.1:56106->127.0.0.1:56105: read: connection reset by peer  category=cdp elapsed="0 ms" goroutine=78
ERRO[0001] process with PID 8012 unexpectedly ended: signal: killed  category=browser elapsed="0 ms" goroutine=75
WARN[0001] bctxid:FBF0BEA2D27232D0C863AC0FAE541A44 err:disposing browser context ID FBF0BEA2D27232D0C863AC0FAE541A44: read tcp 127.0.0.1:56106->127.0.0.1:56105: read: connection reset by peer  category="BrowserContext:Close" elapsed="1 ms" goroutine=71
ERRO[0001] Uncaught (in promise) GoError: closing the browser: canceled
        at github.com/grafana/xk6-browser/api.Browser.Close-fm (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue814.js:29:4(45)  executor=per-vu-iterations scenario=default

If we also log in the Browser.Close instead of panicking, this happens:

ERRO[0001] communicating with browser: read tcp 127.0.0.1:56137->127.0.0.1:56136: read: connection reset by peer  category=cdp elapsed="0 ms" goroutine=82
ERRO[0001] process with PID 8893 unexpectedly ended: signal: killed  category=browser elapsed="0 ms" goroutine=71
WARN[0001] bctxid:B727D7F1443FCF198513661E5F5BEFED err:disposing browser context ID B727D7F1443FCF198513661E5F5BEFED: read tcp 127.0.0.1:56137->127.0.0.1:56136: read: connection reset by peer  category="BrowserContext:Close" elapsed="1 ms" goroutine=67
WARN[0001] closing the browser: context canceled         category="Browser:Close" elapsed="0 ms" goroutine=67
ERRO[0001] Uncaught (in promise) GoError: could not find element with selector "header h1.titlex"
        at github.com/grafana/xk6-browser/browser.mapPage.func12 (native)
        at Title with CSS selector (file:///Users/inanc/grafana/k6browser/tests/issue814.js:24:21(9))
        at go.k6.io/k6/js/modules/k6.(*K6).Check-fm (native)
        at file:///Users/inanc/grafana/k6browser/tests/issue814.js:20:10(36)  executor=per-vu-iterations scenario=default

Updates: #804
Related: grafana/k6#4403

This allows us to: - Better handle extension errors - Leaves the decision of handling to Goja when an element is not found. This is how it's done in the k6-core and other extensions. Updates: #804 Related: #688

ka3de

Thanks for taking care of this, definitely the error handling and output message is much better now!
Just made a small comment.

common/element_handle.go

ka3de

LGTM.

ankur22

LGTM

Add error handling to wildcard selectors

7b5f51f

This allows us to: - Better handle extension errors - Leaves the decision of handling to Goja when an element is not found. This is how it's done in the k6-core and other extensions. Updates: #804 Related: #688

inancgumus requested review from ankur22 and ka3de March 9, 2023 13:38

inancgumus self-assigned this Mar 9, 2023

inancgumus added enhancement New feature or request bug Something isn't working labels Mar 9, 2023

inancgumus marked this pull request as ready for review March 9, 2023 13:39

inancgumus mentioned this pull request Mar 9, 2023

Edge-case: Mapping fails if the extension code returns nil values #804

Closed

ka3de reviewed Mar 10, 2023

View reviewed changes

common/element_handle.go Outdated Show resolved Hide resolved

fixup! ElementHandle.QueryAll nil err

f883c1f

ka3de approved these changes Mar 10, 2023

View reviewed changes

inancgumus added the ux label Mar 10, 2023

ankur22 approved these changes Mar 10, 2023

View reviewed changes

inancgumus merged commit a802711 into main Mar 10, 2023

inancgumus deleted the fix/804-harden-mappings-selectors branch March 10, 2023 15:55

inancgumus mentioned this pull request Mar 14, 2023

Fix Edge-case: Mapping fails if the extension code returns nil values #804 #826

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add error handling to wildcard selectors #820

Add error handling to wildcard selectors #820

Uh oh!

inancgumus commented Mar 9, 2023 •

edited

Loading

Uh oh!

ka3de left a comment

Uh oh!

Uh oh!

ka3de left a comment

Uh oh!

ankur22 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add error handling to wildcard selectors #820

Add error handling to wildcard selectors #820

Uh oh!

Conversation

inancgumus commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ka3de left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ka3de left a comment

Choose a reason for hiding this comment

Uh oh!

ankur22 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

inancgumus commented Mar 9, 2023 •

edited

Loading