Skip to content

Commit

Permalink
Warn about nil visualViewport
Browse files Browse the repository at this point in the history
  • Loading branch information
olegbespalov committed Dec 11, 2024
1 parent 47bbf6d commit 3282dd4
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
2 changes: 1 addition & 1 deletion common/element_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 17 additions & 3 deletions common/screenshotter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 3282dd4

Please sign in to comment.