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

[Feature]: ability to cancel "waitFor" promises #9709

Open
olalonde opened this issue Feb 19, 2023 · 7 comments · Fixed by #10018
Open

[Feature]: ability to cancel "waitFor" promises #9709

olalonde opened this issue Feb 19, 2023 · 7 comments · Fixed by #10018
Labels

Comments

@olalonde
Copy link

Feature description

I'm using page.waitForSelector in this way to determine which page I landed on:

const element = await Promise.race([page.waitForSelector("#login-success"), page.waitForSelector("#login-error")]);

// do something...

The problem with this is that if one of the promises completes early, the others will keep on retrying until their timeout expires (I assume waitFor is internally implemented using setTimeout/setInterval). That can create a lot of unnecessary computation especially for a long array of page.waitForSelector.

It would be good if there was a way to cancel the promise it returns. For example, it could enable this:

const promises = [page.waitForSelector("#login-success"), page.waitForSelector("#login-error")];
const element = await Promise.race(promises);
promises.forEach(p => p.cancel?())

I would be able to work on a pull request if there is interest.

@OrKoN
Copy link
Collaborator

OrKoN commented Feb 20, 2023

Have you tried using page.wairForSelector('#login-success, #login-error')? The feature request is probably still something to consider but I don't think we will change API to return non-standard promises. Perhaps, waitForSelector should accept an AbortController instance?

@olalonde
Copy link
Author

Thanks, I hadn't considered that. I still think this might be a useful feature since you could be waiting on different kind of things, e.g. const element = await Promise.any([page.waitForSelector("#login-success"), page.waitForNavigation()]);.

I could try implementing using an AbortController strategy. So it would be used like this?

// calls abort() once one of the promises fulfills
async function smartAny(abort, promises) {
  const result = await Promise.any(promises);
  abort();
  return result;
}
const { signal, abort } = new AbortController();
await smartAny(abort, [page.waitForSelector("#login-success", { signal }), page.waitForNavigation({ signal })])

@olalonde
Copy link
Author

Actually, instead of calling abort() externally, we could pass the whole abort controller to waitForSelector and let it call abort once it fulfills, canceling any other pending waitFor tasks that share the same controller. e.g.:

const abortController = new AbortController();
await Promise.any([
  page.waitForSelector("#login-success", { abortController }),
  page.waitForNavigation({ abortController })
])

@OrKoN
Copy link
Collaborator

OrKoN commented Feb 20, 2023

Is AbortController available in Node14+?

@olalonde
Copy link
Author

Nope, but there are a few JavaScript implementations available, e.g. https://github.com/southpolesteve/node-abort-controller

@OrKoN
Copy link
Collaborator

OrKoN commented Feb 21, 2023

looks like it might available in Node 14.17.0+
Screenshot 2023-02-21 at 11 45 26

@Kazimazi
Copy link

Can this be added to other waitFor functions, like waitForResponse?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants