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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 12, 2024
Merged

[eas-cli] Fix display of errors when expo-updates CLI command fails #2324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
351deeb
[eas-cli] Fix display of errors when expo-updates CLI command fails
wschurman Apr 12, 2024
8932197
update CHANGELOG.md
wschurman Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/eas-cli/src/project/publish.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,16 +723,21 @@ async function getRuntimeVersionForPlatformAsync({

if (await isModernExpoUpdatesCLIWithRuntimeVersionCommandSupportedAsync(projectDir)) {
try {
Log.debug('Using expo-updates runtimeversion:resolve CLI for runtime version resolution');

const extraArgs = Log.isDebug ? ['--debug'] : [];

const resolvedRuntimeVersionJSONResult = await expoUpdatesCommandAsync(projectDir, [
'runtimeversion:resolve',
'--platform',
platform,
...extraArgs,
]);
const runtimeVersionResult = JSON.parse(resolvedRuntimeVersionJSONResult);
if (runtimeVersionResult.fingerprintSources) {
Log.debug(`Resolved fingeprint runtime version for platform "${platform}". Sources:`);
Log.debug(runtimeVersionResult.fingerprintSources);
}

Log.debug('runtimeversion:resolve output:');
Log.debug(resolvedRuntimeVersionJSONResult);

return nullthrows(
runtimeVersionResult.runtimeVersion,
`Unable to determine runtime version for ${
Expand Down
13 changes: 9 additions & 4 deletions packages/eas-cli/src/project/resolveRuntimeVersionAsync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,21 @@ export async function resolveRuntimeVersionAsync({
}

try {
Log.debug('Using expo-updates runtimeversion:resolve CLI for runtime version resolution');

const extraArgs = Log.isDebug ? ['--debug'] : [];

const resolvedRuntimeVersionJSONResult = await expoUpdatesCommandAsync(projectDir, [
'runtimeversion:resolve',
'--platform',
platform,
...extraArgs,
]);
const runtimeVersionResult = JSON.parse(resolvedRuntimeVersionJSONResult);
if (runtimeVersionResult.fingerprintSources) {
Log.debug(`Resolved fingeprint runtime version for platform "${platform}". Sources:`);
Log.debug(runtimeVersionResult.fingerprintSources);
}

Log.debug('runtimeversion:resolve output:');
Log.debug(resolvedRuntimeVersionJSONResult);

return runtimeVersionResult.runtimeVersion ?? null;
} catch (e: any) {
// if expo-updates is not installed, there's no need for a runtime version in the build
Expand Down
16 changes: 11 additions & 5 deletions packages/eas-cli/src/utils/expoUpdatesCli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { link } from '../log';

export class ExpoUpdatesCLIModuleNotFoundError extends Error {}
export class ExpoUpdatesCLIInvalidCommandError extends Error {}
export class ExpoUpdatesCLICommandFailedError extends Error {}

export async function expoUpdatesCommandAsync(projectDir: string, args: string[]): Promise<string> {
let expoUpdatesCli;
Expand All @@ -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;
Copy link
Contributor

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

Copy link
Member Author

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.

} catch (e: any) {
if (e.stderr && typeof e.stderr === 'string' && e.stderr.includes('Invalid command')) {
throw new ExpoUpdatesCLIInvalidCommandError(
`The command specified by ${args} was not valid in the \`expo-updates\` CLI.`
);
if (e.stderr && typeof e.stderr === 'string') {
if (e.stderr.includes('Invalid command')) {
throw new ExpoUpdatesCLIInvalidCommandError(
`The command specified by ${args} was not valid in the \`expo-updates\` CLI.`
);
} else {
throw new ExpoUpdatesCLICommandFailedError(e.stderr);
}
}

throw e;
}
}
Loading