-
Notifications
You must be signed in to change notification settings - Fork 85
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
[eas-cli] Fix display of errors when expo-updates CLI command fails #2324
Conversation
Size Change: +402 B (0%) Total Size: 51.4 MB
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2324 +/- ##
==========================================
- Coverage 53.68% 53.67% -0.01%
==========================================
Files 525 525
Lines 19190 19196 +6
Branches 4054 4056 +2
==========================================
+ Hits 10301 10302 +1
- Misses 8162 8169 +7
+ Partials 727 725 -2 ☔ View full report in Codecov by Sentry. |
@@ -24,13 +25,18 @@ export async function expoUpdatesCommandAsync(projectDir: string, args: string[] | |||
} | |||
|
|||
try { | |||
return (await spawnAsync(expoUpdatesCli, args)).stdout; | |||
return (await spawnAsync(expoUpdatesCli, args, { stdio: 'pipe' })).stdout; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This technically is not required here, because this spawn
should default to pipe
, right? Asking to confirm.
We're not using the turtle-spawn
here that defaults to inherit
:
https://github.com/expo/eas-build/blob/997d6e65c7fea16184d038fdf831352346c1685d/packages/turtle-spawn/src/index.ts#L16-L19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I added it just to keep the code reasonably consistent between this and build-tools
.
/changelog-entry bug-fix Fix display of errors when expo-updates CLI command fails |
✅ Thank you for adding the changelog entry! |
Why
It seems that the error thrown by the expo-updates CLI is a normal JS Error, and due to the way the CLI is called, the JS error doesn't print super well when it is thrown.
How
This PR wraps all stderr output in a new error type that at least shows the full error to the user. Still room for improvement on the expo-updates CLI end to make the stderr cleaner.
edit: expo-updates CLI PR: expo/expo#28188
Test Plan