fix: Exit with code 1 on command errors

src/lib/command-framework/apify-command.ts

@@ -299,41 +299,52 @@ export abstract class ApifyCommand<T extends typeof BuiltApifyCommand = typeof B
 
 		if (missingRequiredArgs.size) {
 			this._printMissingRequiredArgs(missingRequiredArgs);
-			return;
 		}
 
 		this._parseFlags(rawFlags, rawTokens);
 
+		let hadError: Error | null = null;
+
 		try {
 			await this.run();
 		} catch (err: any) {
 			error({ message: err.message });
-		} finally {
-			// analytics
-			if (!this.telemetryData.actorLanguage && COMMANDS_WITHIN_ACTOR.includes(this.commandString)) {
-				const cwdProject = await useCwdProject();
-
-				cwdProject.inspect((project) => {
-					if (project.type === ProjectLanguage.JavaScript) {
-						this.telemetryData.actorLanguage = 'javascript';
-						this.telemetryData.actorRuntime = project.runtime!.runtimeShorthand || 'node';
-						this.telemetryData.actorRuntimeVersion = project.runtime!.version;
-					} else if (project.type === ProjectLanguage.Python || project.type === ProjectLanguage.Scrapy) {
-						this.telemetryData.actorLanguage = 'python';
-						this.telemetryData.actorRuntime = 'python';
-						this.telemetryData.actorRuntimeVersion = project.runtime!.version;
-					}
-				});
-			}
+			hadError = err;
+		}
+
+		// analytics
+		if (!this.telemetryData.actorLanguage && COMMANDS_WITHIN_ACTOR.includes(this.commandString)) {
+			const cwdProject = await useCwdProject();
+
+			cwdProject.inspect((project) => {
+				if (project.type === ProjectLanguage.JavaScript) {
+					this.telemetryData.actorLanguage = 'javascript';
+					this.telemetryData.actorRuntime = project.runtime!.runtimeShorthand || 'node';
+					this.telemetryData.actorRuntimeVersion = project.runtime!.version;
+				} else if (project.type === ProjectLanguage.Python || project.type === ProjectLanguage.Scrapy) {
+					this.telemetryData.actorLanguage = 'python';
+					this.telemetryData.actorRuntime = 'python';
+					this.telemetryData.actorRuntimeVersion = project.runtime!.version;
+				}
+			});
+		}
 
-			this.telemetryData.flagsUsed = Object.keys(this.flags);
+		this.telemetryData.flagsUsed = Object.keys(this.flags);
 
-			if (!this.skipTelemetry) {
-				await trackEvent(
-					`cli_command_${this.commandString.replaceAll(' ', '_').toLowerCase()}` as const,
-					this.telemetryData,
-				);
+		if (!this.skipTelemetry) {
+			await trackEvent(
+				`cli_command_${this.commandString.replaceAll(' ', '_').toLowerCase()}` as const,
+				this.telemetryData,
+			);
+		}
+
+		if (hadError) {
+			// In test mode (skipTelemetry=true), throw the error so tests can catch it
+			// In normal mode, exit with code 1
+			if (this.skipTelemetry) {
+				throw hadError;
 			}
+			process.exit(1);
 		}
 	}
 
@@ -586,7 +597,7 @@ export abstract class ApifyCommand<T extends typeof BuiltApifyCommand = typeof B
 		}
 	}
 
-	private _printMissingRequiredArgs(missingRequiredArgs: Map<string, TaggedArgBuilder<ArgTag, unknown>>) {
+	private _printMissingRequiredArgs(missingRequiredArgs: Map<string, TaggedArgBuilder<ArgTag, unknown>>): never {
 		const help = selectiveRenderHelpForCommand(this.ctor, {
 			showUsageString: true,
 		});
@@ -614,6 +625,8 @@ export abstract class ApifyCommand<T extends typeof BuiltApifyCommand = typeof B
 				help,
 			].join('\n'),
 		});
+
+		process.exit(1);
 	}
 
 	private _handleStdin(mode: StdinMode) {

test/api/commands/info.test.ts

@@ -8,13 +8,11 @@ import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js';
 
 useAuthSetup();
 
-const { lastErrorMessage, logSpy } = useConsoleSpy();
+const { logSpy } = useConsoleSpy();
 
 describe('[api] apify info', () => {
 	it('should end with Error when not logged in', async () => {
-		await testRunCommand(InfoCommand, {});
-
-		expect(lastErrorMessage()).toMatch(/you are not logged in/i);
+		await expect(testRunCommand(InfoCommand, {})).rejects.toThrow(/you are not logged in/i);
 	});
 
 	it('should work when logged in', async () => {

test/api/commands/pull.test.ts

@@ -9,7 +9,6 @@ import { testRunCommand } from '../../../src/lib/command-framework/apify-command
 import { DEPRECATED_LOCAL_CONFIG_NAME, LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js';
 import { testUserClient } from '../../__setup__/config.js';
 import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js';
-import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js';
 import { useProcessMock } from '../../__setup__/hooks/useProcessMock.js';
 import { useUniqueId } from '../../__setup__/hooks/useUniqueId.js';
 
@@ -107,8 +106,6 @@ function setProcessCwd(newCwd: string) {
 
 useProcessMock({ cwdMock: () => cwd });
 
-const { lastErrorMessage } = useConsoleSpy();
-
 const { ActorsPullCommand } = await import('../../../src/commands/actors/pull.js');
 
 describe('[api] apify pull', () => {
@@ -130,9 +127,7 @@ describe('[api] apify pull', () => {
 	});
 
 	it('should fail outside Actor folder without actorId defined', async () => {
-		await testRunCommand(ActorsPullCommand, {});
-
-		expect(lastErrorMessage()).toMatch(/Cannot find Actor in this directory/i);
+		await expect(testRunCommand(ActorsPullCommand, {})).rejects.toThrow(/Cannot find Actor in this directory/i);
 	});
 
 	it('should work with Actor SOURCE_FILES', async () => {
@@ -219,10 +214,8 @@ describe('[api] apify pull', () => {
 	});
 
 	it('should fail if actor is private', async () => {
-		await testRunCommand(ActorsPullCommand, { args_actorId: 'apify/website-content-crawler' });
-
-		expect(lastErrorMessage()).toMatch(
-			/You cannot pull source code of this Actor because you do not have permission to do so./i,
-		);
+		await expect(
+			testRunCommand(ActorsPullCommand, { args_actorId: 'apify/website-content-crawler' }),
+		).rejects.toThrow(/You cannot pull source code of this Actor because you do not have permission to do so./i);
 	});
 });

test/api/commands/push.test.ts

@@ -307,10 +307,10 @@ describe('[api] apify push', () => {
 			// @ts-expect-error Wrong typing of update method
 			await testActorClient.version(actorJson.version).update({ buildTag: 'beta' });
 
-			await testRunCommand(ActorsPushCommand, { args_actorId: testActor.id, flags_noPrompt: true });
+			await expect(
+				testRunCommand(ActorsPushCommand, { args_actorId: testActor.id, flags_noPrompt: true }),
+			).rejects.toThrow(/is already on the platform/);
 			if (testActor) await testActorClient.delete();
-
-			expect(lastErrorMessage()).to.includes('is already on the platform');
 		},
 		TEST_TIMEOUT,
 	);

test/local/__fixtures__/commands/run/python/prints-error-message-on-project-with-no-detected-start.test.ts

@@ -1,7 +1,6 @@
 import { rm } from 'node:fs/promises';
 
 import { testRunCommand } from '../../../../../../src/lib/command-framework/apify-command.js';
-import { useConsoleSpy } from '../../../../../__setup__/hooks/useConsoleSpy.js';
 import { useTempPath } from '../../../../../__setup__/hooks/useTempPath.js';
 import { resetCwdCaches } from '../../../../../__setup__/reset-cwd-caches.js';
 
@@ -14,8 +13,6 @@ const { beforeAllCalls, afterAllCalls, joinPath, toggleCwdBetweenFullAndParentPa
 	cwdParent: true,
 });
 
-const { lastErrorMessage } = useConsoleSpy();
-
 const { CreateCommand } = await import('../../../../../../src/commands/create.js');
 const { RunCommand } = await import('../../../../../../src/commands/run.js');
 
@@ -41,8 +38,6 @@ describe('[python] prints error message on project with no detected start', () =
 	});
 
 	it('should print error message', async () => {
-		await testRunCommand(RunCommand, {});
-
-		expect(lastErrorMessage()).toMatch(/Actor is of an unknown format./i);
+		await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Actor is of an unknown format./i);
 	});
 });

test/local/commands/actor/calculate-memory.test.ts

@@ -49,9 +49,9 @@ describe('apify actor calculate-memory', () => {
 	it('should fail when default memory is not provided in flags or actor.json', async () => {
 		await createActorJson();
 
-		await testRunCommand(ActorCalculateMemoryCommand, {});
-
-		expect(lastErrorMessage()).toMatch(/No memory-calculation expression found./);
+		await expect(testRunCommand(ActorCalculateMemoryCommand, {})).rejects.toThrow(
+			/No memory-calculation expression found./,
+		);
 	});
 
 	it('should calculate memory using defaultMemoryMbytes flag', async () => {

test/local/commands/crawlee/run.test.ts

@@ -3,7 +3,6 @@ import { readFile, writeFile } from 'node:fs/promises';
 import { testRunCommand } from '../../../../src/lib/command-framework/apify-command.js';
 import { getLocalKeyValueStorePath } from '../../../../src/lib/utils.js';
 import { TEST_TIMEOUT } from '../../../__setup__/consts.js';
-import { useConsoleSpy } from '../../../__setup__/hooks/useConsoleSpy.js';
 import { useTempPath } from '../../../__setup__/hooks/useTempPath.js';
 import { defaultsInputSchemaPath } from '../../../__setup__/input-schemas/paths.js';
 
@@ -30,8 +29,6 @@ const { beforeAllCalls, afterAllCalls, joinPath, toggleCwdBetweenFullAndParentPa
 	cwdParent: true,
 });
 
-const { lastErrorMessage } = useConsoleSpy();
-
 const { CreateCommand } = await import('../../../../src/commands/create.js');
 const { RunCommand } = await import('../../../../src/commands/run.js');
 
@@ -63,9 +60,7 @@ describe('apify run', () => {
 	it('throws when required field is not provided', async () => {
 		await writeFile(inputPath, '{}');
 
-		await testRunCommand(RunCommand, {});
-
-		expect(lastErrorMessage()).toMatch(/Field awesome is required/i);
+		await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Field awesome is required/i);
 	});
 
 	it('prefills input with defaults', async () => {

test/local/commands/create.test.ts

@@ -6,7 +6,6 @@ import { KEY_VALUE_STORE_KEYS } from '@apify/consts';
 import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js';
 import { LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js';
 import { getLocalKeyValueStorePath } from '../../../src/lib/utils.js';
-import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js';
 import { useTempPath } from '../../__setup__/hooks/useTempPath.js';
 
 const actName = 'create-my-actor';
@@ -18,8 +17,6 @@ const { beforeAllCalls, afterAllCalls, joinPath, joinCwdPath, toggleCwdBetweenFu
 		cwdParent: true,
 	});
 
-const { lastErrorMessage } = useConsoleSpy();
-
 const { CreateCommand } = await import('../../../src/commands/create.js');
 
 describe('apify create', () => {
@@ -33,9 +30,9 @@ describe('apify create', () => {
 
 	['a'.repeat(151), 'sh', 'bad_escaped'].forEach((badActorName) => {
 		it(`returns error with bad Actor name ${badActorName}`, async () => {
-			await testRunCommand(CreateCommand, { args_actorName: badActorName });
-
-			expect(lastErrorMessage()).toMatch(/the actor name/i);
+			await expect(testRunCommand(CreateCommand, { args_actorName: badActorName })).rejects.toThrow(
+				/the actor name/i,
+			);
 		});
 	});
 

test/local/commands/run.test.ts

@@ -14,7 +14,6 @@ import {
 } from '../../../src/lib/utils.js';
 import { TEST_TIMEOUT } from '../../__setup__/consts.js';
 import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js';
-import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js';
 import { useTempPath } from '../../__setup__/hooks/useTempPath.js';
 import {
 	defaultsInputSchemaPath,
@@ -46,8 +45,6 @@ const { beforeAllCalls, afterAllCalls, joinPath, toggleCwdBetweenFullAndParentPa
 	cwdParent: true,
 });
 
-const { lastErrorMessage } = useConsoleSpy();
-
 const { CreateCommand } = await import('../../../src/commands/create.js');
 const { RunCommand } = await import('../../../src/commands/run.js');
 
@@ -292,45 +289,37 @@ writeFileSync(String.raw\`${joinPath('result.txt')}\`, 'hello world');
 			writeFileSync(inputPath, '{}', { flag: 'w' });
 			copyFileSync(missingRequiredPropertyInputSchemaPath, inputSchemaPath);
 
-			await testRunCommand(RunCommand, {});
-
-			expect(lastErrorMessage()).toMatch(/Field awesome is required/i);
+			await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Field awesome is required/i);
 		});
 
 		it('throws when required field has wrong type', async () => {
 			writeFileSync(inputPath, '{"awesome": 42}', { flag: 'w' });
 			copyFileSync(defaultsInputSchemaPath, inputSchemaPath);
 
-			await testRunCommand(RunCommand, {});
-
-			expect(lastErrorMessage()).toMatch(/Field awesome must be boolean/i);
+			await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Field awesome must be boolean/i);
 		});
 
 		it('throws when passing manual input, but local file has correct input', async () => {
 			writeFileSync(inputPath, '{"awesome": true}', { flag: 'w' });
 			copyFileSync(defaultsInputSchemaPath, inputSchemaPath);
 
-			await testRunCommand(RunCommand, { flags_input: handPassedInput });
-
-			expect(lastErrorMessage()).toMatch(/Field awesome must be boolean/i);
+			await expect(testRunCommand(RunCommand, { flags_input: handPassedInput })).rejects.toThrow(
+				/Field awesome must be boolean/i,
+			);
 		});
 
 		it('throws when input has default field of wrong type', async () => {
 			writeFileSync(inputPath, '{"awesome": true, "help": 123}', { flag: 'w' });
 			copyFileSync(defaultsInputSchemaPath, inputSchemaPath);
 
-			await testRunCommand(RunCommand, {});
-
-			expect(lastErrorMessage()).toMatch(/Field help must be string/i);
+			await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Field help must be string/i);
 		});
 
 		it('throws when input has prefilled field of wrong type', async () => {
 			writeFileSync(inputPath, '{"awesome": true, "help": 123}', { flag: 'w' });
 			copyFileSync(prefillsInputSchemaPath, inputSchemaPath);
 
-			await testRunCommand(RunCommand, {});
-
-			expect(lastErrorMessage()).toMatch(/Field help must be string/i);
+			await expect(testRunCommand(RunCommand, {})).rejects.toThrow(/Field help must be string/i);
 		});
 
 		it('automatically inserts missing defaulted fields', async () => {

test/local/commands/secrets/add.test.ts

@@ -16,6 +16,16 @@ describe('apify secrets add', () => {
 		}
 	});
 
+	afterEach(async () => {
+		// Clean up after each test
+		const secrets = getSecretsFile();
+		if (secrets[SECRET_KEY]) {
+			await testRunCommand(SecretsRmCommand, {
+				args_name: SECRET_KEY,
+			});
+		}
+	});
+
 	it('should work', async () => {
 		await testRunCommand(SecretsAddCommand, {
 			args_name: SECRET_KEY,
@@ -26,13 +36,19 @@ describe('apify secrets add', () => {
 		expect(secrets[SECRET_KEY]).to.eql(SECRET_VALUE);
 	});
 
-	afterAll(async () => {
-		const secrets = getSecretsFile();
+	it('should throw error when adding duplicate secret', async () => {
+		// First add a secret
+		await testRunCommand(SecretsAddCommand, {
+			args_name: SECRET_KEY,
+			args_value: SECRET_VALUE,
+		});
 
-		if (secrets[SECRET_KEY]) {
-			await testRunCommand(SecretsRmCommand, {
+		// Try to add the same secret again and expect it to throw
+		await expect(
+			testRunCommand(SecretsAddCommand, {
 				args_name: SECRET_KEY,
-			});
-		}
+				args_value: SECRET_VALUE,
+			}),
+		).rejects.toThrow(`Secret with name ${SECRET_KEY} already exists`);
 	});
 });