Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

page.waitForNavigation causes time out #226

Closed
inancgumus opened this issue Feb 9, 2022 · 3 comments · Fixed by #247
Closed

page.waitForNavigation causes time out #226

inancgumus opened this issue Feb 9, 2022 · 3 comments · Fixed by #247
Assignees
@imiric
Labels
bug Something isn't working
Milestone
v0.2.0

Comments

@inancgumus
Copy link
Member

inancgumus commented Feb 9, 2022 •
edited
Loading

Please see #200 (comment) and #200 (comment) for details.

Adding page.waitForNavigation(); in place of page.waitForSelector('div.cb-content'); caused a waitForFrameNavigation timed out after 30s.

Script Source 
import launcher from 'k6/x/browser';
import { group, sleep } from 'k6';

export default function () {
  const browser = launcher.launch('chromium', {
    headless: false,
  });

  const context = browser.newContext({
  });

  const page = context.newPage();

  group("Navigate to Homepage", () => {
    page.goto('https://trollflix.com'); // without the below waitForSelector, the page.$$ expression returns a length of 0

    page.waitForSelector('div.cb-content'); // required here even if I were to use waitUntil: 'networkidle' above
    
    // grab the number of displayed videos (should be 12)
    const videoCount = page.$$('div.cb-content').length;
    console.log(`Found ${videoCount} in page`);
  });

  sleep(2);

  group("Filter by Gaming", () => {
    page.hover('#hn-categories');

    sleep(1);

    // without the below waitForSelector, the page.$$ expression returns a length of 0 (interestingly, not the previous value - 12 - so the expression was evaluated after the page partially refreshed)
    page.$('//div[text()="Gaming"]').click(); 

    page.waitForSelector('div.cb-content'); // waiting for the div here ensures the videoCount updates as expected

    // grab the number of displayed videos (should be 2)
    const videoCount = page.$$('div.cb-content').length;
    console.log(`Found ${videoCount} in page`);
  });
}
@inancgumus inancgumus added the bug Something isn't working label Feb 9, 2022
@inancgumus inancgumus mentioned this issue Feb 9, 2022
Open
@imiric imiric changed the title page.waitForSelector causes time out page.waitForNavigation causes time out Feb 9, 2022
@imiric
Copy link
Contributor

imiric commented Feb 9, 2022

I ran into this as well while testing the staging app. Looking into it.

It could be considered part of #200 though.

@imiric imiric self-assigned this Feb 9, 2022
@imiric
Copy link
Contributor

imiric commented Feb 15, 2022

I've been looking into this issue, and it seems to happen when the navigation happens within the same document, e.g. by using the History API.

In these cases the Page.navigatedWithinDocument event is received, for which we properly emit the internal NavigationEvent, but in WaitForFrameNavigation() we also wait for a FrameAddLifecycle event:

xk6-browser/common/frame_manager.go

Lines 725 to 736 in 917b83e

if frame.hasSubtreeLifecycleEventFired(parsedOpts.WaitUntil) {
m.logger.Debugf("FrameManager:WaitForFrameNavigation",
"fmid:%d furl:%s hasSubtreeLifecycleEventFired:true",
m.ID(), frame.URL())
_, err := waitForEvent(m.ctx, frame, []string{EventFrameAddLifecycle}, func(data interface{}) bool {
return data.(LifecycleEvent) == parsedOpts.WaitUntil
}, parsedOpts.Timeout)
if err != nil {
k6Throw(m.ctx, "waitForFrameNavigation cannot wait for event (EventFrameAddLifecycle): %v", err)
}
}

... which is never emitted in these internal navigations, so the wait times out.

I'm still not sure what the correct fix would be, and will keep investigating.

@imiric
Copy link
Contributor

imiric commented Feb 16, 2022 •
edited
Loading

Here's a test that reproduces the issue:

package tests

import (
	"net/http"
	"testing"
	"time"

	"github.com/grafana/xk6-browser/common"
	"github.com/stretchr/testify/require"
)

var navHTML = `
<html>
  <head>
    <title>History API navigation test</title>
  </head>
  <body>
    <a id="navigate" href="#">Navigate</a>
    <script>
      const el = document.querySelector('a#navigate');
      el.addEventListener('click', function(evt) {
        evt.preventDefault();
        history.pushState({}, 'navigated', '/nav2');
      });
    </script>
  </body>
</html>
`

func TestWaitForFrameNavigationWithinDocument(t *testing.T) {
	tb := newTestBrowser(t, withHTTPServer())
	tb.withHandler("/nav", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Type", "text/html")
		w.Write([]byte(navHTML))
	}))
	p := tb.NewPage(nil)

	resp := p.Goto(tb.URL("/nav"), nil)
	require.NotNil(t, resp)

	el := p.Query("a#navigate")
	require.NotNil(t, el)
	// A click right away could possibly trigger navigation before we had a
	// chance to call WaitForNavigation below, so give it some time to simulate
	// the JS overhead, waiting for XHR response, etc.
	time.AfterFunc(200*time.Millisecond, func() {
		el.Click(nil)
	})

	p.WaitForNavigation(tb.rt.ToValue(&common.FrameWaitForNavigationOptions{
		Timeout: 1000, // 1s
	}))
}

Running it results in:

--- FAIL: TestWaitForFrameNavigationWithinDocument (1.64s)
panic: waitForFrameNavigation cannot wait for event (EventFrameAddLifecycle): timed out [recovered]
        panic: waitForFrameNavigation cannot wait for event (EventFrameAddLifecycle): timed out

goroutine 24 [running]:
testing.tRunner.func1.2({0x1107ba0, 0xc000814b40})
        /home/ivan/.local/share/go/src/testing/testing.go:1209 +0x24e
testing.tRunner.func1()
        /home/ivan/.local/share/go/src/testing/testing.go:1212 +0x218
panic({0x1107ba0, 0xc000814b40})
        /home/ivan/.local/share/go/src/runtime/panic.go:1038 +0x215
go.k6.io/k6/js/common.Throw(0xf9b540, {0x12bf7c0, 0xc000812b10})
        /home/ivan/Projects/grafana/xk6-browser/vendor/go.k6.io/k6/js/common/util.go:36 +0x55
github.com/grafana/xk6-browser/common.k6Throw({0x12dc320, 0xc000932d80}, {0x11800b8, 0x49}, {0xc000455d28, 0x1, 0x1})
        /home/ivan/Projects/grafana/xk6-browser/common/helpers.go:214 +0x1ae
github.com/grafana/xk6-browser/common.(*FrameManager).WaitForFrameNavigation(0xc000584160, 0x5, {0x12f52f8, 0xc0008140c0})
        /home/ivan/Projects/grafana/xk6-browser/common/frame_manager.go:734 +0x94c
github.com/grafana/xk6-browser/common.(*Frame).WaitForNavigation(...)
        /home/ivan/Projects/grafana/xk6-browser/common/frame.go:1434
github.com/grafana/xk6-browser/common.(*Page).WaitForNavigation(0xc0008f2140, {0x12f52f8, 0xc0008140c0})
        /home/ivan/Projects/grafana/xk6-browser/common/page.go:881 +0xd7
github.com/grafana/xk6-browser/tests.TestWaitForFrameNavigationWithinDocument(0x0)
        /home/ivan/Projects/grafana/xk6-browser/tests/frame_manager_nav_test.go:50 +0x22e
testing.tRunner(0xc000623380, 0x11997f0)
        /home/ivan/.local/share/go/src/testing/testing.go:1259 +0x102
created by testing.(*T).Run
        /home/ivan/.local/share/go/src/testing/testing.go:1306 +0x35a
FAIL    github.com/grafana/xk6-browser/tests    1.659s

I'm still looking into the possible fix.

One possible option would be to emit a separate NavigationWithinDocumentEvent for which we don't wait for the FrameAddLifecycle event. I'll explore this tomorrow.

imiric pushed a commit that referenced this issue Feb 18, 2022
Fix WaitForNavigation when it happens within the same document
4db92c5 
Previously WaitForNavigation assumed that a LifecycleEvent would always
be fired, but this is not the case for navigation within the same
document (e.g. via anchor links or the History API), so in those cases
the call would timeout after 30s.

The fix simply checks if we received a new document, otherwise it skips
waiting for the LifecycleEvent. Even if that wait didn't time out, it
would've failed with a nil pointer panic accessing event.newDocument.request.

Closes #226
imiric pushed a commit that referenced this issue Feb 18, 2022
Fix WaitForNavigation within the same document
520b650 
Previously WaitForNavigation assumed that a LifecycleEvent would always
be fired, but this is not the case for navigation within the same
document (e.g. via anchor links or the History API), so in those cases
the call would timeout after 30s.

The fix simply checks if we received a new document, otherwise it skips
waiting for the LifecycleEvent. Even if that wait didn't time out, it
would've failed with a nil pointer panic accessing event.newDocument.request.

Closes #226
@imiric imiric mentioned this issue Feb 18, 2022
Merged
imiric pushed a commit that referenced this issue Feb 21, 2022
Fix WaitForNavigation within the same document (#247)
d885d6f 
* Fix WaitForNavigation within the same document

Previously WaitForNavigation assumed that a LifecycleEvent would always
be fired, but this is not the case for navigation within the same
document (e.g. via anchor links or the History API), so in those cases
the call would timeout after 30s.

The fix simply checks if we received a new document, otherwise it skips
waiting for the LifecycleEvent. Even if that wait didn't time out, it
would've failed with a nil pointer panic accessing event.newDocument.request.

Closes #226

* Move static HTML to standalone file

Resolves
- #247 (comment)
- #247 (comment)

* Send empty struct instead of closing done channel

Resolves #247 (comment)

* Use Page.click() instead of ElementHandle.Click()

Resolves #247 (comment)
@imiric imiric added this to the v0.2.0 milestone Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@imiric imiric

Labels
bug Something isn't working
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging a pull request may close this issue.

Fix WaitForNavigation within the same document grafana/xk6-browser
2 participants
@inancgumus @imiric