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

Add missing await for closing incognito browser context. #170

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

aarongoldenthal
Copy link
Contributor

Add missing await for closing incognito browser context. Errors have been seen running custom reporters with async functions, as noted in #168 (comment), with the stack trace pointing to this line. The browserContext.close function is async, so without the await there appears to be timing issues between the browserContext.close and parent browser.close (which are likely masked in other cases by pa11y-ci completing and exiting the process).

This change did resolve at least the reporter problem noted in #168 (as did running with "useIncognitoBrowserContext": false as a workaround). The build-in reporters (cli/html) are only synchronous code and don't appear to cause it.

I haven't seen this with other execution as noted in #168, so not clear if it will resolve the other issues, but those errors do also point to this line of code.

@aarongoldenthal
Copy link
Contributor Author

@sangitamane Is there any chance of this being pushed out as a patch release? At a minimum async reporters using incognito are broken (or at least flaky) without it (especially when processing data in afterAll, which hold the process open at the end of the run).

@sangitamane
Copy link
Member

I can merge this in master, but I am afraid if I can release it! So @josebolos or @joeyciechanowicz may release it once they are back.

@sangitamane sangitamane merged commit 3bb80d6 into pa11y:master Dec 17, 2021
@joeyciechanowicz
Copy link
Member

joeyciechanowicz commented Dec 17, 2021 via email

@josebolos
Copy link
Member

Released in v3.0.1. Thanks @aarongoldenthal!

@josebolos josebolos mentioned this pull request Mar 23, 2022
@aarongoldenthal aarongoldenthal deleted the missing-await branch September 1, 2024 18:57
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.

4 participants