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.

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

Replace networkidle with waitFor #8

Closed
bin101 opened this issue May 9, 2023 · 10 comments
Closed

Replace networkidle with waitFor #8

bin101 opened this issue May 9, 2023 · 10 comments

Comments

@bin101
Copy link
Contributor

bin101 commented May 9, 2023

I noticed that Strava seem to have changed their page lazy loading behaviour. Maybe we should switch from networkidle to waitFor (https://playwright.dev/python/docs/api/class-locator#locator-wait-for) for the dashboard page load.

@isaac-chung
Copy link
Owner

Hey, thanks for bringing this up. A few questions:

  • Impact: Wondering how much this is affecting the current setup? (I haven't noticed any missed kudos lately) and where would this apply to (only login?)?
  • Potential "fix": What should the state for wait_for be? Maybe visible?

@bin101
Copy link
Contributor Author

bin101 commented May 10, 2023

I noticed it on the dashboard page. The page was loaded and networkidle was triggered but there where no web-feed-entry available. So those get lazy loaded by strava.

wait_for waits until a DOM element is available so my guess is to wait until the first web-feed-entry is available and then add a delay for about further 500ms just to be sure?

In my private version I also save the browser local storage after the login happend and the dashboard is loaded to prevent a relogin for the next execution. I did it because I run the script every 5 minutes on my server and to quick logins seems to trigger a rate limit by strava and then they add a captcha check on the login page.

This is the documentation for the storage stuff:
https://playwright.dev/python/docs/auth#reusing-signed-in-state

You don't need to persist the session storage the first example is enough. Should be pretty easy to persist the json for further github actions runs

@isaac-chung
Copy link
Owner

Nice find on the session storage stuff. We could implement that if we choose to.

As for the lazy loading from strava, any idea what triggers the loading? A scroll maybe? or any random action? There might be a chance that we switch to wait_for and still no DOM elements are available. I also wonder if we can measure that somehow?

@bin101
Copy link
Contributor Author

bin101 commented May 10, 2023

The ajax call just needs some time, I currently still use networkidle but with another delay of 2sec. This also works fine but could break in the future.

@isaac-chung
Copy link
Owner

I see. Have you tried using waitFor in your fork then?

@bin101
Copy link
Contributor Author

bin101 commented May 17, 2023

Not yet, didn't have time for it and I am also busy the next 1-2 weeks. But the docs sound promising for this case

@isaac-chung
Copy link
Owner

All good, I'm in a similar situation in the next few weeks. Once some testing has been done and if it seems to be more reliable, then we can look at a PR together. Thanks again for bringing this up.

@feamster
Copy link

I messed around with waitFor, couldn't get it to work... retries still timing out. Will keep messing with it.

@isaac-chung
Copy link
Owner

Closing this now. Feel free to reopen later.

@isaac-chung
Copy link
Owner

Note that #11 is related to the lazy loading discussion. Instead of waitFor, scrolling was added. Please update your forks if you are seeing failures related to that.

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

No branches or pull requests

3 participants