Centralize error handling

change/beachball-4c8bb372-aff5-438c-9706-ca20aaac11bb.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Centralize error handling instead of calling process.exit() throughout the code",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

scripts/jestSetup.js

@@ -1,6 +1,10 @@
 // @ts-check
 const { jest, afterAll } = require('@jest/globals');
 
+// Safety net: production code should never call process.exit() directly
+// (errors should be thrown and caught at the top level in cli.ts).
+// This mock ensures any accidental process.exit() call in tests immediately
+// throws rather than silently exiting the test runner.
 jest.spyOn(process, 'exit').mockImplementation(code => {
   throw new Error(`process.exit called with code ${code}`);
 });

src/__e2e__/validate.test.ts

@@ -5,13 +5,12 @@ import { initMockLogs } from '../__fixtures__/mockLogs';
 import { validate, type ValidateOptions } from '../validation/validate';
 import type { Repository } from '../__fixtures__/repository';
 import { getParsedOptions } from '../options/getOptions';
-import { mockProcessExit } from '../__fixtures__/mockProcessExit';
+import { BeachballError } from '../types/BeachballError';
 
 describe('validate', () => {
   let repositoryFactory: RepositoryFactory;
   let repo: Repository | undefined;
   const logs = initMockLogs();
-  const processExit = mockProcessExit(logs);
 
   function validateWrapper(validateOptions?: ValidateOptions) {
     const parsedOptions = getParsedOptions({
@@ -30,7 +29,6 @@ describe('validate', () => {
   });
 
   afterEach(() => {
-    processExit.mockClear();
     repo = undefined;
   });
 
@@ -54,8 +52,7 @@ describe('validate', () => {
     repo.checkout('-b', 'test');
     repo.stageChange('packages/foo/test.js');
 
-    expect(() => validateWrapper({ checkChangeNeeded: true })).toThrowError(/process\.exit/);
-    expect(processExit).toHaveBeenCalledWith(1);
+    expect(() => validateWrapper({ checkChangeNeeded: true })).toThrow(BeachballError);
     expect(logs.mocks.error).toHaveBeenCalledWith('ERROR: Change files are needed!');
   });
 

src/__fixtures__/mockNpm.test.ts

@@ -2,7 +2,7 @@
 // But this added complexity greatly speeds up the other npm-related tests by removing the
 // dependency on actual npm CLI calls and a fake registry (which are very slow).
 
-import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
 import fs from 'fs';
 // import fetch from 'npm-registry-fetch';
 import { type NpmResult, npm } from '../packageManager/npm';
@@ -208,10 +208,6 @@ describe('_mockNpmPublish', () => {
     packageJson = undefined;
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('throws if cwd is not specified', async () => {
     await expect(() => _mockNpmPublish({}, [], { cwd: undefined })).rejects.toThrow(
       'cwd is required for mock npm publish'
@@ -311,10 +307,6 @@ describe('_mockNpmPack', () => {
     writtenFiles = [];
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('throws if cwd is not specified', async () => {
     await expect(() => _mockNpmPack({}, [], { cwd: undefined })).rejects.toThrow('cwd is required for mock npm pack');
   });
@@ -370,10 +362,6 @@ describe('mockNpm', () => {
     packageJson = undefined;
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   // describe('mockFetchJson', () => {
   //   it('mocks registry fetch', async () => {
   //     npmMock.setRegistryData({ foo: { versions: ['1.0.0'] } });

src/__functional__/options/getCliOptions.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, afterEach, describe, expect, it, jest } from '@jest/globals';
+import { afterEach, describe, expect, it, jest } from '@jest/globals';
 import { getCliOptions } from '../../options/getCliOptions';
 import { findProjectRoot, getDefaultRemoteBranch } from 'workspace-tools';
 
@@ -34,10 +34,6 @@ describe('getCliOptions', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   // start by making sure nothing went wrong with the mock
   it('uses fake project root', () => {
     expect(findProjectRoot(process.cwd())).toEqual(projectRoot);

src/__functional__/options/getOptions.test.ts

@@ -30,7 +30,6 @@ describe('getOptions (deprecated)', () => {
 
   afterAll(() => {
     repositoryFactory.cleanUp();
-    jest.restoreAllMocks();
   });
 
   it('uses the branch name defined in beachball.config.js', () => {
@@ -115,7 +114,6 @@ describe('getParsedOptions', () => {
 
   afterAll(() => {
     repositoryFactory.cleanUp();
-    jest.restoreAllMocks();
   });
 
   it('uses the branch name defined in beachball.config.js', () => {

src/__functional__/packageManager/getNpmPackageInfo.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, afterAll, beforeEach, jest } from '@jest/globals';
+import { describe, expect, it, beforeEach, jest } from '@jest/globals';
 // import fetch from 'npm-registry-fetch';
 import {
   _npmShowProperties,
@@ -24,10 +24,6 @@ describe('getNpmPackageInfo', () => {
     // fetchJsonSpy.mockClear();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it.each<{ desc: string; name: string; knownVersion: string }>([
     { desc: 'unscoped', name: 'beachball', knownVersion: '2.60.1' },
     { desc: 'scoped', name: '@lage-run/cli', knownVersion: '0.33.0' },

src/__functional__/publish/publishToRegistry.test.ts

@@ -158,8 +158,7 @@ describe('publishToRegistry', () => {
     // Pre-populate registry so version 1.0.0 already exists
     npmMock.setRegistryData({ foo: { versions: ['1.0.0'] } });
 
-    // process.exit is mocked in jestSetup.js to throw
-    await expect(publishToRegistry(bumpInfo, defaultOptions)).rejects.toThrow('process.exit called with code 1');
+    await expect(publishToRegistry(bumpInfo, defaultOptions)).rejects.toThrow('Pre-publish validation failed');
 
     expect(logs.getMockLines('all')).toMatchInlineSnapshot(`
       "[log] Validating new package versions...

src/__tests__/bump/bumpInMemory.test.ts

@@ -1,8 +1,7 @@
-import { afterAll, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { describe, expect, it } from '@jest/globals';
 import path from 'path';
 import { generateChanges, type PartialChangeFile } from '../../__fixtures__/changeFiles';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
-import { mockProcessExit } from '../../__fixtures__/mockProcessExit';
 import { makePackageInfosByFolder, type PartialPackageInfo } from '../../__fixtures__/packageInfos';
 import { bumpInMemory } from '../../bump/bumpInMemory';
 import { getParsedOptions } from '../../options/getOptions';
@@ -12,7 +11,7 @@ import { getScopedPackages } from '../../monorepo/getScopedPackages';
 import { getPackageGroups } from '../../monorepo/getPackageGroups';
 
 describe('bumpInMemory', () => {
-  const logs = initMockLogs();
+  initMockLogs();
   const cwd = path.resolve('/fake-root');
 
   function gatherBumpInfoWrapper(params: {
@@ -42,15 +41,6 @@ describe('bumpInMemory', () => {
     return { bumpInfo, options, originalPackageInfos };
   }
 
-  beforeAll(() => {
-    // getPackageGroups currently can call process.exit
-    mockProcessExit(logs);
-  });
-
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('bumps only packages with change files with bumpDeps: false', () => {
     const { bumpInfo, originalPackageInfos } = gatherBumpInfoWrapper({
       packageFolders: {

src/__tests__/bump/setDependentVersions.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, afterEach, jest, beforeAll, afterAll } from '@jest/globals';
+import { describe, it, expect, afterEach, jest, beforeAll } from '@jest/globals';
 import { setDependentVersions } from '../../bump/setDependentVersions';
 import { makePackageInfos, type PartialPackageInfos } from '../../__fixtures__/packageInfos';
 import { consideredDependencies } from '../../types/PackageInfo';
@@ -35,10 +35,6 @@ describe('setDependentVersions', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('returns empty object when no packages are in scope', () => {
     const bumpInfo = makeBumpInfo({
       packageInfos: { 'pkg-a': {} },

src/__tests__/bump/updateLockFile.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, jest, afterAll, afterEach, beforeAll, beforeEach } from '@jest/globals';
+import { describe, it, expect, jest, afterEach, beforeAll, beforeEach } from '@jest/globals';
 import fs from 'fs';
 import path from 'path';
 import { updateLockFile } from '../../bump/updateLockFile';
@@ -35,10 +35,6 @@ describe('updateLockFile', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('updates package-lock.json when it exists', async () => {
     mockFs.existsSync.mockImplementation(p => p === path.join(mockRoot, 'package-lock.json'));
 

src/__tests__/bump/updatePackageJsons.test.ts

@@ -1,4 +1,4 @@
-import { jest, describe, it, expect, beforeEach, afterEach, beforeAll, afterAll } from '@jest/globals';
+import { jest, describe, it, expect, beforeEach, afterEach, beforeAll } from '@jest/globals';
 import fs from 'fs';
 import { updatePackageJsons } from '../../bump/updatePackageJsons';
 import { makePackageInfos } from '../../__fixtures__/packageInfos';
@@ -41,10 +41,6 @@ describe('updatePackageJsons', () => {
     jest.resetAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('updates version for non-private packages', () => {
     const modifiedPackages = new Set(['pkg-a']);
     const packageInfos = makePackageInfos({

src/__tests__/monorepo/getPackageGroups.test.ts

@@ -1,10 +1,10 @@
-import { afterAll, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { describe, expect, it } from '@jest/globals';
 import path from 'path';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
-import { mockProcessExit } from '../../__fixtures__/mockProcessExit';
 import { makePackageInfosByFolder, type PartialPackageInfos } from '../../__fixtures__/packageInfos';
 import { getPackageGroups } from '../../monorepo/getPackageGroups';
 import type { RepoOptions } from '../../types/BeachballOptions';
+import { BeachballError } from '../../types/BeachballError';
 
 describe('getPackageGroups', () => {
   const root = path.resolve('/fake-root');
@@ -21,14 +21,6 @@ describe('getPackageGroups', () => {
     return getPackageGroups(packageInfos, root, params.repoOptions?.groups);
   }
 
-  beforeAll(() => {
-    mockProcessExit(logs);
-  });
-
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('returns empty object if no groups are defined', () => {
     const groups = getPackageGroupsWrapper({
       packageFolders: { 'packages/foo': {}, 'packages/bar': {} },
@@ -173,7 +165,7 @@ describe('getPackageGroups', () => {
           ],
         },
       })
-    ).toThrow('process.exit(1) called');
+    ).toThrow(BeachballError);
 
     expect(logs.getMockLines('error')).toMatchInlineSnapshot(`
       "ERROR: Found package(s) belonging to multiple groups:

src/__tests__/publish/tagPackages.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, jest, beforeEach, afterAll } from '@jest/globals';
+import { describe, expect, it, jest, beforeEach } from '@jest/globals';
 import { gitFailFast } from 'workspace-tools';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { tagPackages } from '../../publish/tagPackages';
@@ -63,10 +63,6 @@ describe('tagPackages', () => {
     (gitFailFast as jest.Mock).mockReset();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('does not create package tag for packages with gitTags=false', () => {
     // Also verifies that if `gitTags` is false overall, it doesn't create a git tag for the dist tag (`tag`)
     tagPackages(noTagBumpInfo, { path: '', gitTags: false, tag: '' });

src/__tests__/validate/isValidChangelogOptions.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
 import { isValidChangelogOptions } from '../../validation/isValidChangelogOptions';
 import type { ChangelogGroupOptions, ChangelogOptions } from '../../types/ChangelogOptions';
 
@@ -13,10 +13,6 @@ describe('isValidChangelogOptions', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('returns true when options have no groups', () => {
     const options: ChangelogOptions = {};
     expect(isValidChangelogOptions(options)).toBe(true);

src/__tests__/validate/isValidGroupOptions.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
 import { isValidGroupOptions, isValidGroupedPackageOptions } from '../../validation/isValidGroupOptions';
 import type { VersionGroupOptions } from '../../types/BeachballOptions';
 import type { PackageGroups } from '../../types/PackageInfo';
@@ -15,10 +15,6 @@ describe('isValidGroupOptions', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('returns true for valid groups', () => {
     const groups: VersionGroupOptions[] = [
       { name: 'group1', include: ['pkg1', 'pkg2'], disallowedChangeTypes: null },
@@ -83,10 +79,6 @@ describe('isValidGroupedPackageOptions', () => {
     jest.clearAllMocks();
   });
 
-  afterAll(() => {
-    jest.restoreAllMocks();
-  });
-
   it('returns true when no packages have disallowedChangeTypes', () => {
     const packageInfos = makePackageInfos({ pkg1: {}, pkg2: {} });
     const packageGroups: PackageGroups = {

src/cli.ts

@@ -11,15 +11,14 @@ import { getPackageInfos } from './monorepo/getPackageInfos';
 import { getParsedOptions } from './options/getOptions';
 import { validate } from './validation/validate';
 import { getScopedPackages } from './monorepo/getScopedPackages';
+import { BeachballError } from './types/BeachballError';
 
 (async () => {
   try {
     // eslint-disable-next-line no-restricted-properties -- top-level call
     findGitRoot(process.cwd());
   } catch {
-    console.error('beachball only works in a git repository. Please initialize git and try again.');
-    // eslint-disable-next-line no-restricted-properties
-    process.exit(1);
+    throw new BeachballError('beachball only works in a git repository. Please initialize git and try again.');
   }
 
   // eslint-disable-next-line no-restricted-properties -- this is the top level
@@ -98,10 +97,18 @@ import { getScopedPackages } from './monorepo/getScopedPackages';
       throw new Error('Invalid command: ' + options.command);
   }
 })().catch(e => {
-  showVersion();
-  console.error('An error has been detected while running beachball!');
-  console.error((e as Error)?.stack || e);
+  if (e instanceof BeachballError && e.alreadyLogged) {
+    // Error details were already printed -- just exit
+  } else if (e instanceof BeachballError) {
+    // Expected error, not yet logged -- print the message (no stack trace)
+    console.error(e.message);
+  } else {
+    // Unexpected error -- print full details including stack
+    showVersion();
+    console.error('Unexpected error while running beachball!');
+    console.error((e as Error)?.stack || e);
+  }
 
-  // eslint-disable-next-line no-restricted-properties -- this is the top level
+  // eslint-disable-next-line no-restricted-properties -- this is the only place that should call process.exit
   process.exit(1);
 });