Skip to content

Commit

Permalink
Distinguish EjecaSpawnError from EjecaError
Browse files Browse the repository at this point in the history
Summary:
There's two error types from Ejeca:
1. a command spawns then exits non-zero
2. a command does not spawn (syscall to spawn fails)

(1) is what we typically think of as an EjecaError, and contains most normal errors.

(2) can happen with ENOENT if `sl` or other commands like `gh` are not on your PATH.

The distinction is important for good error messages. Let's make them separate types to keep these separate.

Reviewed By: quark-zju

Differential Revision: D67995419

fbshipit-source-id: ee8df2d40929277280f564b36ac07e9a1a56c336
  • Loading branch information
evangrayk authored and facebook-github-bot committed Jan 10, 2025
1 parent 7540058 commit 7b82043
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
10 changes: 6 additions & 4 deletions addons/isl-server/src/github/queryGraphQL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import {Internal} from '../Internal';
import {isEjecaError} from '../utils';
import {isEjecaError, isEjecaSpawnError} from '../utils';
import {ejeca} from 'shared/ejeca';

export default async function queryGraphQL<TData, TVariables>(
Expand Down Expand Up @@ -52,12 +52,14 @@ export default async function queryGraphQL<TData, TVariables>(

return json.data;
} catch (error: unknown) {
if (isEjecaError(error)) {
// FIXME: we're never setting `code` in ejeca, so this is always false!
if (isEjecaSpawnError(error)) {
if (error.code === 'ENOENT' || error.code === 'EACCES') {
// `gh` not installed on path
throw new Error(`GhNotInstalledError: ${(error as Error).stack}`);
} else if (error.exitCode === 4) {
}
} else if (isEjecaError(error)) {
// FIXME: we're never setting `code` in ejeca, so this is always false!
if (error.exitCode === 4) {
// `gh` CLI exit code 4 => authentication issue
throw new Error(`NotAuthenticatedError: ${(error as Error).stack}`);
}
Expand Down
15 changes: 13 additions & 2 deletions addons/isl-server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,22 @@ export function parseExecJson<T>(
});
}

// FIXME: we're not ever setting `code`!
export function isEjecaError(s: unknown): s is EjecaError & {code?: string} {
export type EjecaSpawnError = Error & {code: string; path: string};

/**
* True if an Ejeca spawned process exits non-zero or is killed.
* @see {EjecaSpawnError} for when a process fails to spawn in the first place (e.g. ENOENT).
*/
export function isEjecaError(s: unknown): s is EjecaError {
return s != null && typeof s === 'object' && 'exitCode' in s;
}

/** True when Ejeca fails to spawn a process, e.g. ENOENT.
* (as opposed to the command spawning, then exiting non-zero) */
export function isEjecaSpawnError(s: unknown): s is EjecaSpawnError {
return s != null && typeof s === 'object' && 'code' in s;
}

export function fromEntries<V>(entries: Array<[string, V]>): {
[key: string]: V;
} {
Expand Down

0 comments on commit 7b82043

Please sign in to comment.