[eas-cli] Do not hang in workflow:run when stdin never delivers data - #4261
Open
giaBaoJS wants to merge 1 commit into
Open
[eas-cli] Do not hang in workflow:run when stdin never delivers data#4261giaBaoJS wants to merge 1 commit into
workflow:run when stdin never delivers data#4261giaBaoJS wants to merge 1 commit into
Conversation
|
Subscribed to pull request
Generated by CodeMention Warning: The preamble and epilogue options in commentConfiguration are deprecated. Use template instead. |
`maybeReadStdinAsync()` only short-circuited on `process.stdin.isTTY`. On a CI agent stdin is an open pipe: not a TTY, and it never emits 'end' either, so the promise never settled and `eas workflow:run` waited forever without creating a run. Wait a short grace period for the first chunk of piped input and fall back to `null` when nothing arrives. Once any data has arrived we wait for 'end' with no deadline, so piping a large or slowly written payload still works. Also short-circuit when stdin has already ended or been destroyed, and unreference the stdin handle once we stop reading so it cannot keep the process alive. Fixes expo#3164 Fixes expo#3774
giaBaoJS
force-pushed
the
fix/workflow-run-stdin-hang
branch
from
August 25, 2026 02:03
200d975 to
7aee20b
Compare
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 #3774 and #3164.
eas workflow:runprintsUsing workflow file from ...and then hangs forever on a CI agent. No workflow run is ever created, and the pipeline eventually times out. Five people have confirmed it on #3164, the reported environment is Azure Pipelines. The community workarounds are</dev/nullorecho '{}' |.@sjchmiela guessed at the cause on #3164:
and
It is the other way around.
maybeReadStdinAsync()inpackages/eas-cli/src/commandUtils/workflow/utils.tsshort-circuits only whenprocess.stdin.isTTY:On a CI agent stdin is an open pipe. It is not a TTY, so the short-circuit does not fire, and nobody ever writes to it or closes it, so
'end'never fires either. The promise never settles.workflow/run.tsawaits it at the top of the command, before any flag is looked at, so the command stops there.eas buildis unaffected because it never reads stdin.How
There is no way to ask a pipe whether anyone will ever write to it, so the only thing that can be done is to wait a bounded amount of time for the first byte:
nulland carry on.'end'with no timeout. A writer that has started talking to us is never cut off mid-payload, socat inputs.json | eas workflow:runkeeps working no matter how big or slow the payload is. Truncating input silently would be worse than hanging.'end'listener to such a stream would wait forever too.stdin.pause()does not work here, because a stream with a'readable'listener already hasflowing === falseandpause()returns early without emitting the'pause'event that Node's own stdin handling listens for.This is suggestions 3 and 4 from #3774. I did not implement suggestion 1 (skip the read when
--non-interactiveis set), because piping JSON in is documented at the top ofworkflow/run.tsas a first class input source and is independent of interactivity.echo '{}' | eas workflow:run --non-interactiveis exactly what people are running today as the workaround, and skipping stdin under--non-interactivewould break it.The trade off worth flagging: a producer that takes longer than 1s to emit its first byte, say
curl ... | eas workflow:run, would now be treated as "no stdin". 1s is the value suggested in #3774 and it is generous for anything already buffered, but it is a judgement call and easy to raise if you would rather have it higher. Missing required inputs still fail loudly rather than silently, since they are validated after this point.Test Plan
New tests in
packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.tsswapprocess.stdinfor aPassThroughand race the call against a sentinel, so a promise that never settles fails with a readable assertion instead of a bare timeout.cd packages/eas-cli && yarn jest src/commandUtils/workflow/__tests__/utils-test.tsReverting only
utils.tsand keeping the tests, the two hang cases go red for the right reason and the four behaviour cases stay green, which is what shows the change is not silently altering how real piped input is handled:I also ran the built
maybeReadStdinAsyncin a real process against a real fd, since the unit tests cannot cover process exit. The script calls the function and prints the result, the harness reports when the process exits and kills it at 15s:Case D is the trade off described above, shown rather than hidden.
Rest of the package:
cd packages/eas-cli && yarn jestThe two failing suites are
src/observe/__tests__/formatEvents.test.tsandsrc/observe/__tests__/formatCustomEvents.test.ts, and they fail identically on a clean checkout ofmain(4 failed, 2438 passed, 2446 total). It is a date locale mismatch,1 Jan 2025vs the expectedJan 1, 2025, unrelated to this change.yarn lintclean,yarn fmt:checkclean,tsc --noEmitclean,yarn lint-changelogreportsCHANGELOG.md is valid.