Screenshotter fails after a JavaScript exception

inancgumus commented 3 weeks ago

What?

The screenshotter fails when called in a catch block (after an exception).

export async function someTest() {
  ...
  try {
    ...
  } catch (error) {
    await page.screenshot(...);
  } finally {
    await page.close();
  }

Exception:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x15e1f48]

goroutine 11479469 [running]:
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.deferwrap1()
go.opentelemetry.io/otel/sdk@v1.29.0/trace/span.go:398 +0x25
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc00ff77d40, {0x0, 0x0, 0xc0615c1a40?})
go.opentelemetry.io/otel/sdk@v1.29.0/trace/span.go:436 +0xa62
panic({0x197cc60?, 0x2f63d50?})
runtime/panic.go:785 +0x132
github.com/grafana/xk6-browser/common.(*screenshotter).screenshot(0xc017fa5ee0, {0x1f9b8c0, 0xc01df2c780}, 0x0, 0xc017fa5cd8, {0x1c15435, 0x3}, 0x0, 0xc04cc80d80?, {0xc09641fa10, ...})
github.com/grafana/xk6-browser@v1.9.1/common/screenshotter.go:185 +0x328
github.com/grafana/xk6-browser/common.(*screenshotter).screenshotPage(0xc017fa5ee0, 0xc03747c680, 0xc04cc80ac0)
github.com/grafana/xk6-browser@v1.9.1/common/screenshotter.go:398 +0x334
github.com/grafana/xk6-browser/common.(*Page).Screenshot(0xc03747c680, 0xc04cc80ac0, {0x7f691c1821d0, 0xc000451d10})
github.com/grafana/xk6-browser@v1.9.1/common/page.go:1356 +0x2c6
github.com/grafana/xk6-browser/browser.mapPage.func33.1()
github.com/grafana/xk6-browser@v1.9.1/browser/page_mapping.go:241 +0x4e
github.com/grafana/xk6-browser/k6ext.promise.func1()
github.com/grafana/xk6-browser@v1.9.1/k6ext/promise.go:24 +0x2c
created by github.com/grafana/xk6-browser/k6ext.promise in goroutine 730
github.com/grafana/xk6-browser@v1.9.1/k6ext/promise.go:23 +0x9a

Test Run ID: 3439289

Why?

Users should take screenshots even after an exception to see the issues in their web pages or scripts.

How?

Looking at the code:

screenshotPage calls screenshotter.screenshot with a non-nil viewport *Rect.
However, the code panics here while calculating Width: viewport.Width / visualViewport.Scale.

Since viewport should not be nil, visualViewport might be.

However, the signature in the panic log says that viewport is nil:

github.com/grafana/xk6-browser/common.(*screenshotter).screenshot(0xc017fa5ee0, {0x1f9b8c0, 0xc01df2c780}, 0x0, 0xc017fa5cd8, {0x1c15435, 0x3}, 0x0, 0xc04cc80d80?, {0xc09641fa10, ...})

According to the code, the screenshotter.go:398 seems not to pass a nil Rect, and always passes a non-nil one (here and here) unless there's an error.

Tasks

### Tasks
- [ ] Subissue
- [ ] PR
- [ ] Docs PR
- [ ] TypeScript Definitions PR
- [ ] Update the k6 release notes and release tasks

Related PR(s)/Issue(s)

No response

ankur22 commented 3 weeks ago

This could be linked to https://github.com/grafana/xk6-browser/issues/1056. clickablePoint works with cdppage.GetLayoutMetrics which is the same CDP request performed before the NPD in this issue.

ankur22 commented 2 weeks ago

The first argument in the stack trace (0xc017fa5ee0) is the receiver, which is a pointer to the screenshotter instance, so viewport is not nil since it is 0xc017fa5cd8, but doc is nil which matches the call from github.com/grafana/xk6-browser@v1.9.1/common/screenshotter.go:398. So my hypothesis is likely to be correct -- avoid working with cdppage.GetLayoutMetrics, and no more NPDs.

grafana / xk6-browser