chore(ci): add timeout and retry to Playwright browser installs - #1259
Open
Wikid82 wants to merge 2 commits into
Open
chore(ci): add timeout and retry to Playwright browser installs#1259Wikid82 wants to merge 2 commits into
Wikid82 wants to merge 2 commits into
Conversation
…event job-timeout hangs apt-get inside `playwright install --with-deps` can silently hang against a flaky Azure apt mirror with no timeout of its own, burning the full 60-minute job timeout and surfacing as a misleading cancelled run instead of a clear failure. Wrap all 10 Playwright browser install steps in nick-fields/retry (10min/attempt, 3 attempts, 15s backoff), matching the existing pattern already used in docker-build.yml.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Contributor
✅ Supply Chain Verification Results✅ PASSED 📦 SBOM Summary
🔍 Vulnerability Scan
📎 Artifacts
Generated by Supply Chain Verification workflow • View Details |
…right installs nick-fields/retry's timeout handling calls process.kill() on the child process tree from its own Node process. `playwright install --with-deps` internally elevates part of that tree to root via sudo to run apt-get, so when the action's 10-minute timeout fires, the kill fails with EPERM (cross-UID kill) and throws unhandled instead of retrying. The step died on attempt 1 and never reached attempt 2 or 3. Replaced all 10 occurrences with a plain bash retry loop using coreutils `timeout`, which signals its own direct child and isn't subject to the EPERM bug class. Matches the existing bash retry-loop idiom already used in codecov-upload.yml and quality-checks.yml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apt-getinsidenpx playwright install --with-deps <browser>can silently hang against a flaky Azure apt mirror with no timeout of its own, burning the job's full 60-minutetimeout-minutesand surfacing as a misleadingcancelledrun instead of a clear, actionable failure (observed on run 32068948141 —E2E Chromium (Security Enforcement), triggered on release-please's PR chore(main): release 0.37.0 #1258 againstmain).e2e-tests-split.ymlwithnick-fields/retry(10 min/attempt, 3 attempts, 15s backoff), matching the existing pattern already used indocker-build.yml.timeout-minutes: 60and theconcurrency: cancel-in-progress: trueblock are left untouched — those are correct as-is.Test plan
actionlintclean,lefthook run pre-commitpasses (incl. semgrep, 0 findings)npx playwright install --with-depsoccurrences converted, no strayEXIT_CODE=$?remnants left behinde2e-tests-split.ymlhas an unfilteredpull_request:trigger (line 82) in addition toworkflow_call/workflow_dispatch, so this PR itself runs the full "E2E Tests" workflow directly — real, direct CI signal on this change, not deferred to nightly