From 3282dd46567953a43b800a4b794b51a099321a3a Mon Sep 17 00:00:00 2001
From: Oleg Bespalov <oleg.bespalov@grafana.com>
Date: Wed, 11 Dec 2024 14:05:52 +0100
Subject: [PATCH] Warn about nil visualViewport

---
 common/element_handle.go |  2 +-
 common/page.go           |  2 +-
 common/screenshotter.go  | 20 +++++++++++++++++---
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/common/element_handle.go b/common/element_handle.go
index 8efdda1dc..faf314047 100644
--- a/common/element_handle.go
+++ b/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)
diff --git a/common/page.go b/common/page.go
index 45a430201..3442a1158 100644
--- a/common/page.go
+++ b/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)
diff --git a/common/screenshotter.go b/common/screenshotter.go
index 5cd8634b0..842b8b2af 100644
--- a/common/screenshotter.go
+++ b/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) {
@@ -183,11 +189,19 @@ func (s *screenshotter) screenshot(
 	visualViewportPageX, visualViewportPageY := 0.0, 0.0
 	// 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
-	// ideally this case also should be logged
 	if visualViewport != nil {
 		visualViewportScale = visualViewport.Scale
 		visualViewportPageX = visualViewport.PageX
 		visualViewportPageY = visualViewport.PageY
+	} else {
+		s.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,
+		)
 	}
 
 	if doc == nil {