Handle panic in screenshoter

common/element_handle.go

@@ -1317,7 +1317,7 @@ func (h *ElementHandle) Screenshot(
 
 	span.SetAttributes(attribute.String("screenshot.path", opts.Path))
 
-	s := newScreenshotter(spanCtx, sp)
+	s := newScreenshotter(spanCtx, sp, h.logger)
 	buf, err := s.screenshotElement(h, opts)
 	if err != nil {
 		err := fmt.Errorf("taking screenshot of elementHandle: %w", err)

common/page.go

@@ -1327,7 +1327,7 @@ func (p *Page) Screenshot(opts *PageScreenshotOptions, sp ScreenshotPersister) (
 
 	span.SetAttributes(attribute.String("screenshot.path", opts.Path))
 
-	s := newScreenshotter(spanCtx, sp)
+	s := newScreenshotter(spanCtx, sp, p.logger)
 	buf, err := s.screenshotPage(p, opts)
 	if err != nil {
 		err := fmt.Errorf("taking screenshot of page: %w", err)

common/screenshotter.go

@@ -13,6 +13,7 @@ import (
 	"github.com/chromedp/cdproto/cdp"
 	"github.com/chromedp/cdproto/emulation"
 	cdppage "github.com/chromedp/cdproto/page"
+	"github.com/grafana/xk6-browser/log"
 )
 
 // ScreenshotPersister is the type that all file persisters must implement. It's job is
@@ -67,10 +68,15 @@ func (f *ImageFormat) UnmarshalJSON(b []byte) error {
 type screenshotter struct {
 	ctx       context.Context
 	persister ScreenshotPersister
+	logger    *log.Logger
 }
 
-func newScreenshotter(ctx context.Context, sp ScreenshotPersister) *screenshotter {
-	return &screenshotter{ctx, sp}
+func newScreenshotter(
+	ctx context.Context,
+	sp ScreenshotPersister,
+	logger *log.Logger,
+) *screenshotter {
+	return &screenshotter{ctx, sp, logger}
 }
 
 func (s *screenshotter) fullPageSize(p *Page) (*Size, error) {
@@ -144,7 +150,6 @@ func (s *screenshotter) restoreViewport(p *Page, originalViewport *Size) error {
 	return p.resetViewport()
 }
 
-//nolint:funlen
 func (s *screenshotter) screenshot(
 	sess session, doc, viewport *Rect, format ImageFormat, omitBackground bool, quality int64, path string,
 ) ([]byte, error) {
@@ -172,29 +177,27 @@ func (s *screenshotter) screenshot(
 		capture.WithFormat(cdppage.CaptureScreenshotFormatPng)
 	}
 
-	// Add clip region
-	//nolint:dogsled
-	_, visualViewport, _, _, _, _, err := cdppage.GetLayoutMetrics().Do(cdp.WithExecutor(s.ctx, sess))
+	visualViewportScale, visualViewportPageX, visualViewportPageY, err := getViewPortDimensions(s.ctx, sess, s.logger)
 	if err != nil {
-		return nil, fmt.Errorf("getting layout metrics for screenshot: %w", err)
+		return nil, err
 	}
 
 	if doc == nil {
 		s := Size{
-			Width:  viewport.Width / visualViewport.Scale,
-			Height: viewport.Height / visualViewport.Scale,
+			Width:  viewport.Width / visualViewportScale,
+			Height: viewport.Height / visualViewportScale,
 		}.enclosingIntSize()
 		doc = &Rect{
-			X:      visualViewport.PageX + viewport.X,
-			Y:      visualViewport.PageY + viewport.Y,
+			X:      visualViewportPageX + viewport.X,
+			Y:      visualViewportPageY + viewport.Y,
 			Width:  s.Width,
 			Height: s.Height,
 		}
 	}
 
 	scale := 1.0
 	if viewport != nil {
-		scale = visualViewport.Scale
+		scale = visualViewportScale
 	}
 	clip = &cdppage.Viewport{
 		X:      doc.X,
@@ -230,6 +233,42 @@ func (s *screenshotter) screenshot(
 	return buf, nil
 }
 
+func getViewPortDimensions(ctx context.Context, sess session, logger *log.Logger) (float64, float64, float64, error) {
+	visualViewportScale := 1.0
+	visualViewportPageX, visualViewportPageY := 0.0, 0.0
+
+	// Add clip region
+	//nolint:dogsled
+	_, visualViewport, _, _, cssVisualViewport, _, err := cdppage.GetLayoutMetrics().Do(cdp.WithExecutor(ctx, sess))
+	if err != nil {
+		return 0, 0, 0, fmt.Errorf("getting layout metrics for screenshot: %w", err)
+	}
+
+	// we had a null pointer panic cases, when visualViewport is nil
+	// instead of the erroring out, we fallback to defaults and still try to do a screenshot
+	switch {
+	case cssVisualViewport != nil:
+		visualViewportScale = cssVisualViewport.Scale
+		visualViewportPageX = cssVisualViewport.PageX
+		visualViewportPageY = cssVisualViewport.PageY
+	case visualViewport != nil:
+		visualViewportScale = visualViewport.Scale
+		visualViewportPageX = visualViewport.PageX
+		visualViewportPageY = visualViewport.PageY
+	default:
+		logger.Warnf(
+			"Screenshotter::screenshot",
+			"chrome browser returned nil on page.getLayoutMetrics, falling back to defaults for visualViewport "+
+				"(scale: %v, pageX: %v, pageY: %v)."+
+				"This is non-standard behavior, if possible please report this issue (with reproducible script) "+
+				"to the https://github.com/grafana/xk6-browser/issues/1502.",
+			visualViewportScale, visualViewportPageX, visualViewportPageY,
+		)
+	}
+
+	return visualViewportScale, visualViewportPageX, visualViewportPageY, nil
+}
+
 //nolint:funlen
 func (s *screenshotter) screenshotElement(h *ElementHandle, opts *ElementHandleScreenshotOptions) ([]byte, error) {
 	format := opts.Format