Skip to content

Fix WaitRepaint to respect timeout#1225

Open
BobLuursema wants to merge 1 commit intogo-rod:mainfrom
BobLuursema:fix-context-bug
Open

Fix WaitRepaint to respect timeout#1225
BobLuursema wants to merge 1 commit intogo-rod:mainfrom
BobLuursema:fix-context-bug

Conversation

@BobLuursema
Copy link

I have this bit of code to click a button on a site. The site is a little janky every now and then so I've put it in a timeout.

err := rod.Try(func() {
	buttonElement.Timeout(5*time.Second).Click(proto.InputMouseButtonLeft, 1)
})

But every now and then this hangs for hours. Which to my knowledge is longer than 5 seconds, so it should timeout. When using SIGQUIT to kill the process I got this stacktrace, at the bottom is the Click that I call from my code.

goroutine 1286 gp=0xc0008fcfc0 m=nil [select, 357 minutes]:
runtime.gopark(0xc000b38838?, 0x2?, 0x0?, 0x25?, 0xc000b38794?)
        runtime/proc.go:460 +0xce fp=0xc000b38618 sp=0xc000b385f8 pc=0x47d94e
runtime.selectgo(0xc000b38838, 0xc000b38790, 0xf8c220?, 0x0, 0xf98ea0?, 0x1)
        runtime/select.go:351 +0x8b7 fp=0xc000b38758 sp=0xc000b38618 pc=0x45c3d7
github.com/go-rod/rod/lib/cdp.(*Client).Call(0xc000180540, {0x1741690, 0xc0001b82d0}, {0xc0009f9160, 0x20}, {0x117244c, 0x16}, {0x110de00, 0xc000fbb280})
        github.com/go-rod/rod@v0.116.2/lib/cdp/client.go:117 +0x43a fp=0xc000b388d0 sp=0xc000b38758 pc=0xe0a47a
github.com/go-rod/rod.(*Browser).Call(0xc000039d40, {0x1741690?, 0xc0001b82d0?}, {0xc0009f9160, 0x20}, {0x117244c, 0x16}, {0x110de00, 0xc000fbb280})
        github.com/go-rod/rod@v0.116.2/browser.go:241 +0x6e fp=0xc000b38950 sp=0xc000b388d0 pc=0xe2850e
github.com/go-rod/rod.(*Page).Call(0xc000b389d8?, {0x1741690?, 0xc0001b82d0?}, {0xc0009f9160?, 0x105ef60?}, {0x117244c?, 0xc000b38a08?}, {0x110de00?, 0xc000fbb280?})
        github.com/go-rod/rod@v0.116.2/page.go:1009 +0x3e fp=0xc000b389a8 sp=0xc000b38950 pc=0xe4affe
github.com/go-rod/rod/lib/proto.call({0x117244c, 0x16}, {0x110de00, 0xc000fbb280}, {0xf62360, 0xc000487340}, {0x173ab00, 0xc0009f5130})
        github.com/go-rod/rod@v0.116.2/lib/proto/a_interface.go:63 +0x175 fp=0xc000b38a18 sp=0xc000b389a8 pc=0xda4bb5
github.com/go-rod/rod/lib/proto.RuntimeCallFunctionOn.Call(...)
        github.com/go-rod/rod@v0.116.2/lib/proto/runtime.go:806
github.com/go-rod/rod.(*Page).evaluate(0xc0009f5130, 0xc000b38be0)
        github.com/go-rod/rod@v0.116.2/page_eval.go:172 +0x19e fp=0xc000b38b78 sp=0xc000b38a18 pc=0xe4c21e
github.com/go-rod/rod.(*Page).Evaluate(0xc0009f5130, 0xc000b38be0)
        github.com/go-rod/rod@v0.116.2/page_eval.go:129 +0x4a fp=0xc000b38bd0 sp=0xc000b38b78 pc=0xe4bbca
github.com/go-rod/rod.(*Page).Eval(0xc0000bf6b0?, {0x1191e28?, 0xc000ace9f0?}, {0x0?, 0x10?, 0xfa10e0?})
        github.com/go-rod/rod@v0.116.2/page_eval.go:119 +0x65 fp=0xc000b38c30 sp=0xc000b38bd0 pc=0xe4bb25
github.com/go-rod/rod.(*Page).WaitRepaint(...)
        github.com/go-rod/rod@v0.116.2/page.go:851
github.com/go-rod/rod.(*Element).WaitStableRAF(0xc000acea20)
        github.com/go-rod/rod@v0.116.2/element.go:568 +0xda fp=0xc000b38ca8 sp=0xc000b38c30 pc=0xe3263a
github.com/go-rod/rod.(*Element).ScrollIntoView(0xc000acea20)
        github.com/go-rod/rod@v0.116.2/element.go:72 +0xba fp=0xc000b38d40 sp=0xc000b38ca8 pc=0xe2f07a
github.com/go-rod/rod.(*Element).WaitInteractable.func1(...)
        github.com/go-rod/rod@v0.116.2/element.go:593
github.com/go-rod/rod/lib/utils.Retry(...)
        github.com/go-rod/rod@v0.116.2/lib/utils/sleeper.go:140
github.com/go-rod/rod.(*Element).WaitInteractable(0xc000acea20)
        github.com/go-rod/rod@v0.116.2/element.go:590 +0xcb fp=0xc000b38dd0 sp=0xc000b38d40 pc=0xe327eb
github.com/go-rod/rod.(*Element).Hover(0xc000acea20)
        github.com/go-rod/rod@v0.116.2/element.go:83 +0x18 fp=0xc000b38e00 sp=0xc000b38dd0 pc=0xe2f198
github.com/go-rod/rod.(*Element).Click(0xc000acea20, {0x114e2fd, 0x4}, 0x1)
        github.com/go-rod/rod@v0.116.2/element.go:105 +0x45 fp=0xc000b38e68 sp=0xc000b38e00 pc=0xe2f2c5

The function that causes the infinite hanging is the WaitRepaint function. This function uses p.root to run an eval:

func (p *Page) WaitRepaint() error {
	// we use root here because iframe doesn't trigger requestAnimationFrame
	_, err := p.root.Eval(`() => new Promise(r => requestAnimationFrame(r))`)
	return err
}

But that means that the context with the timeout that is on p is no longer respected for this eval, since the eval runs on p.root. The site I am clicking the button on is apparently doing something such that requestAnimationFrame is never triggered sometimes. And thus this eval hangs without end.

The solution is to create a clone of p.root with the context from p before doing the eval:

func (p *Page) WaitRepaint() error {
	// we use root here because iframe doesn't trigger requestAnimationFrame
	// but we use the current page's context to respect timeouts
	_, err := p.root.Context(p.ctx).Eval(`() => new Promise(r => requestAnimationFrame(r))`)
	return err
}
A test demonstrating the bug
package rod

import (
  "context"
  "testing"
  "time"
)

// TestWaitRepaintContextBug demonstrates the bug where WaitRepaint uses p.root.Eval()
// instead of p.Eval(), causing timeout contexts to be ignored
func TestWaitRepaintContextBug(t *testing.T) {
  browser := New().MustConnect()
  defer browser.MustClose()

  page := browser.MustPage("about:blank")
  page.MustSetDocumentContent(`
  	<html>
  	<head>
  		<style>
  			#test-element {
  				width: 100px;
  				height: 100px;
  				background: red;
  				position: absolute;
  				top: 10000px;
  			}
  		</style>
  	</head>
  	<body>
  		<div id="test-element"></div>
  		<script>
  			// Override requestAnimationFrame to never call the callback
  			// This will cause WaitRepaint to hang indefinitely
  			window.requestAnimationFrame = function(callback) {
  				return window.setTimeout(function() {
  					// Never call the callback - this simulates a browser that's stuck
  					console.log('requestAnimationFrame called but callback never invoked');
  				}, 100000); // Very long delay
  			};
  		</script>
  	</body>
  	</html>
  `)

  // Create a page with a short timeout context
  timeout := 2 * time.Second
  ctx, cancel := context.WithTimeout(context.Background(), timeout)
  defer cancel()

  timeoutPage := page.Context(ctx)

  // This should timeout but will hang due to WaitRepaint using p.root
  errChan := make(chan error, 1)
  startTime := time.Now()

  go func() {
  	err := timeoutPage.WaitRepaint()
  	errChan <- err
  }()

  select {
  case err := <-errChan:
  	elapsed := time.Since(startTime)
  	if err != nil {
  		t.Logf("WaitRepaint completed with error after %v: %v", elapsed, err)
  		// Check if it was a context timeout error
  		if err == context.DeadlineExceeded {
  			t.Log("SUCCESS: Context timeout worked as expected")
  		} else {
  			t.Logf("UNEXPECTED: Got different error: %v", err)
  		}
  	} else {
  		t.Logf("WaitRepaint completed successfully after %v (unexpected)", elapsed)
  	}
  case <-time.After(timeout + 3*time.Second): // Give it 3 extra seconds to be sure
  	elapsed := time.Since(startTime)
  	t.Errorf("BUG CONFIRMED: WaitRepaint timeout not working - operation hung for %v", elapsed)
  	// Clean up the goroutine
  	go func() {
  		<-errChan
  	}()
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant