Skip to content

Commit ca1c009

Browse files
l2yshoclaude
andauthored
fix(run): propagate the Actor's non-zero exit code (#1195)
## What & why `apify run` exited with code `0` even when the Actor failed. The `catch` block wrapping `execWithLog` in `RunCommand` read `stderr` but never set `process.exitCode` or re-threw (it held only a `TODO`), so the error was swallowed and the command framework's `process.exitCode ||= 1` never fired. This silently broke CI and shell chains like `apify run && deploy` — a failing Actor looked successful to any caller inspecting the exit code. Fixes #1190 Fixes #1180 ## Change In the `catch` block, forward the child's exit code: ```ts } catch (err) { process.exitCode = (err as ExecaError).exitCode ?? 1; } ``` - Preserves the Actor's **exact** exit code (e.g. `process.exit(10)` → CLI exits `10`). - Falls back to `1` for signal-killed children, where `ExecaError.exitCode` is `undefined`. - No change to error output — `execWithLog` already prints the message and the Actor's own stdout/stderr is inherited regardless. Only the exit code was being lost. ## Tests Added a scenario test (`test/local/__fixtures__/commands/run/javascript/propagates-non-zero-exit-code.test.ts`) following the existing fixture convention: it scaffolds a `project_empty` Actor, copies in a failing `main.js` fixture that exits `10`, runs it, and asserts the CLI's `process.exitCode` is `10`. Verified TDD red → green, plus end-to-end with the built CLI: | | Actor stderr shown | Error line shown | Exit code | |---|---|---|---| | Before | ✅ | ✅ | `0` ❌ | | After | ✅ | ✅ | `10` ✅ | `lint`, `format`, `build`, and `test:local` all pass; existing `run` tests unaffected. ## Follow-up Exit-code handling for `execWithLog` children is inconsistent across commands (`run.ts`, `create.ts`, `upgrade.ts` each differ), and `exec.ts` discards the friendly error it builds. Tracked in #1196 — out of scope for this fix. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 51089bd commit ca1c009

3 files changed

Lines changed: 57 additions & 5 deletions

File tree

src/commands/run.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,11 +447,9 @@ export class RunCommand extends ApifyCommand<typeof RunCommand> {
447447
});
448448
}
449449
} catch (err) {
450-
const { stderr } = err as ExecaError;
451-
452-
if (stderr) {
453-
// TODO: maybe throw in helpful tips for debugging issues (missing scripts, trying to start a ts file with old node, etc)
454-
}
450+
// Propagate the child's failure so the CLI exits non-zero (e.g. for CI/shell chains).
451+
// The command framework only defaults exitCode to 1; preserve the Actor's own code when present.
452+
process.exitCode = (err as ExecaError).exitCode ?? 1;
455453
} finally {
456454
if (storedInputResults) {
457455
if ('tempInputKey' in storedInputResults) {
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { copyFile } from 'node:fs/promises';
2+
import { fileURLToPath } from 'node:url';
3+
4+
import { testRunCommand } from '../../../../../../src/lib/command-framework/apify-command.js';
5+
import { useTempPath } from '../../../../../__setup__/hooks/useTempPath.js';
6+
7+
const actorName = 'propagates-non-zero-exit-code';
8+
9+
const failingActorMainPath = fileURLToPath(new URL('./sources/failing-main.js', import.meta.url));
10+
11+
const { beforeAllCalls, afterAllCalls, joinPath, toggleCwdBetweenFullAndParentPath } = useTempPath(actorName, {
12+
create: true,
13+
remove: true,
14+
cwd: true,
15+
cwdParent: true,
16+
});
17+
18+
const { CreateCommand } = await import('../../../../../../src/commands/create.js');
19+
const { RunCommand } = await import('../../../../../../src/commands/run.js');
20+
21+
describe('[javascript] propagates the non-zero exit code when the Actor fails', () => {
22+
beforeAll(async () => {
23+
await beforeAllCalls();
24+
25+
await testRunCommand(CreateCommand, {
26+
flags_template: 'project_empty',
27+
args_actorName: actorName,
28+
});
29+
toggleCwdBetweenFullAndParentPath();
30+
31+
await copyFile(failingActorMainPath, joinPath('src', 'main.js'));
32+
});
33+
34+
afterAll(async () => {
35+
await afterAllCalls();
36+
});
37+
38+
it('should set process.exitCode to the Actor exit code', async () => {
39+
// The cwd mock (useTempPath with cwd: true) replaces the `node:process` module, so the
40+
// command writes process.exitCode to that mocked copy. Read it back from the same instance.
41+
const { default: cliProcess } = await import('node:process');
42+
cliProcess.exitCode = 0;
43+
44+
await testRunCommand(RunCommand, { flags_purge: true });
45+
46+
// Matches the `process.exit(10)` in sources/failing-main.js
47+
expect(cliProcess.exitCode).toBe(10);
48+
});
49+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// Fixture for the run exit-code propagation test.
2+
// See https://github.com/apify/apify-cli/issues/1180 and https://github.com/apify/apify-cli/issues/1190
3+
// A failing Actor: `npm start` exits with a non-zero code that the CLI must
4+
// propagate instead of swallowing.
5+
process.exit(10);

0 commit comments

Comments
 (0)