Repository navigation
Conversation
Contributor
|
eas-cli now uses changesets for changelogs (#4580). The required "Check for a changeset" check fails on PRs that edit To update this PR:
The release adds the PR link and the author to the entry. If this PR needs no changelog entry, revert the |
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
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.
Why
Fixes #4479.
When
expo doctorhits its 30s timeout,runExpoDoctorinpackages/build-tools/src/common/setup.tscollects the process tree withgetParentAndDescendantProcessPidsAsyncand then callsprocess.kill(pid)on each pid inside anasyncsetTimeoutcallback. The tree is a snapshot: a pid can exit betweenpgrepand the kill (a child finishing on its own, or exiting because its parent was just killed).process.killthen throwsESRCH, the async callback rejects, nothing handles it, and Node terminates the whole build with the stack from the issue:So a timeout that is supposed to be skipped with a warning (
"expo doctor" timed out, skipping...) fails the build instead. The same loop is copy-pasted in the install-dependencies kill timeout (setup.ts) and the install-pods kill timeout (builders/ios.ts), with the same race.How
Add
killProcesses(pids, signal = 'SIGTERM')toutils/processes.ts, next togetParentAndDescendantProcessPidsAsync. It kills each pid and skipsESRCH(the process is already gone, which is the goal), rethrowing anything else. This matches the existingkillProcessGrouphelper in the same file, which already treatsESRCHas "nothing left to stop". The threepids.forEach(pid => process.kill(pid))loops now call it. A side effect is that one missing pid no longer stops the remaining pids from being killed.No change to the timeouts, the signal, or the error reporting.
Test Plan
src/utils/__tests__/processes.test.tswithkillProcessescases: kills every pid with the given signal; anESRCHon one pid is ignored and the next pid is still killed; a real process that already exited does not throw; other errors (EPERM) are rethrown.jest --config jest/unit-config.ts src/utils/__tests__/processes.test.ts: 11 tests pass.jest --config jest/unit-config.ts src/utils src/common src/builders: 341 tests pass. The 3 suites that fail locally (custom,hookableBuildPhase,jobHooks) fail the same way onmainin my checkout (a local@types/node/ TypeScript mismatch inandroidLocalEgress.ts), unrelated to this change.Error: kill ESRCH ... at Array.forEach ... at Timeout._onTimeoutstack from the issue (exit code 1); withkillProcessesthe callback finishes and the process exits 0.oxfmt --checkandoxlintclean on the changed files.tscreports no errors in the changed files.