Fix: Exit with code 1 on command errors

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

@@ -301,41 +301,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);
 		}
 	}
 
@@ -588,7 +599,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,
 		});
@@ -616,6 +627,8 @@ export abstract class ApifyCommand<T extends typeof BuiltApifyCommand = typeof B
 				help,
 			].join('\n'),
 		});
+
+		process.exit(1);
 	}
 
 	private _handleStdin(mode: StdinMode) {

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

@@ -1,7 +1,6 @@
 import { rename } 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');
 
@@ -37,8 +34,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/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`);
 	});
 });

test/local/commands/validate-schema.test.ts

@@ -19,20 +19,18 @@ describe('apify validate-schema', () => {
 	});
 
 	it('should correctly validate schema 2', async () => {
-		await testRunCommand(ValidateInputSchemaCommand, {
-			args_path: invalidInputSchemaPath,
-		});
-
-		expect(lastErrorMessage()).to.contain(
-			'Field schema.properties.queries.editor must be equal to one of the allowed values',
-		);
+		await expect(
+			testRunCommand(ValidateInputSchemaCommand, {
+				args_path: invalidInputSchemaPath,
+			}),
+		).rejects.toThrow(/Field schema.properties.queries.editor must be equal to one of the allowed values/);
 	});
 
 	it('should correctly validate schema 3', async () => {
-		await testRunCommand(ValidateInputSchemaCommand, {
-			args_path: unparsableInputSchemaPath,
-		});
-
-		expect(lastErrorMessage()).to.contain.oneOf(['Unexpected token }', "Expected ',' or ']' after array element"]);
+		await expect(
+			testRunCommand(ValidateInputSchemaCommand, {
+				args_path: unparsableInputSchemaPath,
+			}),
+		).rejects.toThrow();
 	});
 });