From 8d4dd0a62caf662f8d3e895c36852d545179448b Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Thu, 17 Sep 2026 14:52:25 +0200 Subject: [PATCH] Reject packaging errors instead of exiting Allow createVSIX callers to catch validation and secret scanning failures so their cleanup handlers can run. Keep process termination at the existing CLI error boundary. Fixes #1220. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/package.ts | 18 +++++--------- src/secretLint.ts | 7 ++---- src/test/package.test.ts | 51 ++++++++++++++++++---------------------- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/src/package.ts b/src/package.ts index a7eaf907..047f9f24 100644 --- a/src/package.ts +++ b/src/package.ts @@ -1026,8 +1026,7 @@ export class ReadmeProcessor extends MarkdownProcessor { override async onEnd(): Promise { if (this.options.readmePath && this.filesProcessed === 0) { - util.log.error(`The provided readme file (${this.options.readmePath}) could not be found.`); - process.exit(1); + throw new Error(`The provided readme file (${this.options.readmePath}) could not be found.`); } } } @@ -1050,8 +1049,7 @@ export class ChangelogProcessor extends MarkdownProcessor { override async onEnd(): Promise { if (this.options.changelogPath && this.filesProcessed === 0) { - util.log.error(`The provided changelog file (${this.options.changelogPath}) could not be found.`); - process.exit(1); + throw new Error(`The provided changelog file (${this.options.changelogPath}) could not be found.`); } } } @@ -2153,8 +2151,7 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string, message += `Both a ${styleText('bold', '.vscodeignore')} file and a ${styleText('bold', '"files"')} property in package.json were found. `; message += `VSCE does not support combining both strategies. `; message += `Either remove the ${styleText('bold', '.vscodeignore')} file or the ${styleText('bold', '"files"')} property in package.json.`; - util.log.error(message); - process.exit(1); + throw new Error(message); } // Throw an error if the extension uses the files property in package.json and // the package does not include at least one file for each include pattern @@ -2184,8 +2181,7 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string, message += '\nRemove any include pattern which is not needed.\n'; message += `\n=> Run ${styleText('bold', 'vsce ls --tree')} to see all included files.\n`; message += `=> Use ${styleText('bold', '--allow-unused-files-pattern')} to skip this check`; - util.log.error(message); - process.exit(1); + throw new Error(message); } } @@ -2263,8 +2259,7 @@ export async function scanFilesForSecrets(files: IFile[], fileExclusion: FileExc hintMessage += secretsFoundRuleNames.map(name => `--allow-package-secrets ${name}`).join(' '); hintMessage += ` or use --allow-package-all-secrets to skip this check entirely (not recommended).`; - util.log.error(errorMessage + styleText('italic', hintMessage)); - process.exit(1); + throw new Error(errorMessage + styleText('italic', hintMessage)); } // .env file found @@ -2283,8 +2278,7 @@ export async function scanFilesForSecrets(files: IFile[], fileExclusion: FileExc const hintMessage = `\nTo ignore this check, you can use --allow-package-env-file (not recommended).`; - util.log.error(errorMessage + styleText('italic', hintMessage)); - process.exit(1); + throw new Error(errorMessage + styleText('italic', hintMessage)); } } diff --git a/src/secretLint.ts b/src/secretLint.ts index 0a755e06..b2d6fbaf 100644 --- a/src/secretLint.ts +++ b/src/secretLint.ts @@ -8,7 +8,6 @@ import type { SecretLintRuleCreator, SecretLintRulePresetCreator } from "@secretlint/types"; -import { log } from "./util"; interface SecretLintFinding { message: string; @@ -127,8 +126,7 @@ export async function lintFiles( }) ); } catch (error) { - log.error('Error occurred while scanning secrets (files):', error); - process.exit(1); + throw new Error('Error occurred while scanning secrets (files)', { cause: error }); } return parseResult(results); @@ -159,8 +157,7 @@ export async function lintText( } }); } catch (error) { - log.error('Error occurred while scanning secrets (content):', error); - process.exit(1); + throw new Error('Error occurred while scanning secrets (content)', { cause: error }); } return parseResult([result]); } diff --git a/src/test/package.test.ts b/src/test/package.test.ts index 43de34de..0c90ffdd 100644 --- a/src/test/package.test.ts +++ b/src/test/package.test.ts @@ -16,6 +16,7 @@ import { LicenseProcessor, printAndValidatePackagedFiles, pack } from '../package'; +import { createVSIX } from '../api'; import { ManifestPackage } from '../manifest'; import * as path from 'path'; import * as fs from 'fs'; @@ -91,38 +92,28 @@ function createManifest(extra: Partial = {}): ManifestPackage { const PROCESS_ERROR_MESSAGE = 'PROCESS ERROR'; async function testPrintAndValidatePackagedFiles(files: IFile[], cwd: string, manifest: ManifestPackage, options: IPackageOptions, errorExpected: boolean, warningExpected: boolean): Promise { - const originalLogError = log.error; const originalLogWarn = log.warn; const originalProcessExit = process.exit; const warns: string[] = []; - const errors: string[] = []; let exited = false; - let errorThrown: string | undefined; - log.error = (message: string) => errors.push(message); + let errorThrown: Error | undefined; log.warn = (message: string) => warns.push(message); process.exit = (() => { exited = true; throw Error(PROCESS_ERROR_MESSAGE); }) as () => never; try { await printAndValidatePackagedFiles(files, cwd, manifest, options); } catch (e: any) { - if (e instanceof Error && e.message !== PROCESS_ERROR_MESSAGE) { - errorThrown = e.message + '\n' + e.stack; - } + errorThrown = e; } finally { process.exit = originalProcessExit; - log.error = originalLogError; log.warn = originalLogWarn; } // Validate that the correct number of errors and warnings were thrown const messages = []; - if (errorExpected !== !!errors.length) { - if (errors.length) { - messages.push(...errors); - } else { - messages.push('Expected an error'); - } + if (errorExpected !== !!errorThrown) { + messages.push(errorThrown ? `Unexpected error: ${errorThrown.stack}` : 'Expected an error'); } if (warningExpected !== !!warns.length) { @@ -133,20 +124,16 @@ async function testPrintAndValidatePackagedFiles(files: IFile[], cwd: string, ma } } - if (!errorExpected && exited) { + if (exited) { messages.push('Process exited'); } - if (!errorExpected && !!errorThrown && !exited) { - messages.push('Error thrown: ' + errorThrown); - } - if (messages.length) { throw new Error(messages.join('\n')); } } -async function processExitExpected(fn: () => Promise, errorMessage: string): Promise { +async function rejectionExpected(fn: () => Promise, errorMessage: string): Promise { const originalExit = process.exit; let exitCalled = false; @@ -156,10 +143,8 @@ async function processExitExpected(fn: () => Promise, errorMessage: string) throw new Error('Process exit was called'); }) as any; - await fn(); - assert.fail(errorMessage); - } catch (error) { - assert.ok(exitCalled, errorMessage); + await assert.rejects(fn, errorMessage); + assert.strictEqual(exitCalled, false, 'Process exit was called'); } finally { process.exit = originalExit; } @@ -286,6 +271,16 @@ describe('collect', function () { await testPrintAndValidatePackagedFiles(files, cwd, manifestCopy, {}, false, false); }); + it('createVSIX rejects instead of exiting the process when package validation fails', async () => { + const cwd = fixture('manifestFiles'); + const ignoreFile = path.join(cwd, 'README.md'); + + await rejectionExpected( + () => createVSIX({ cwd, ignoreFile, packagePath: getVisxOutputPath() }), + 'Expected createVSIX to reject when both an ignore file and package.json files are used' + ); + }); + it('should ignore devDependencies', () => { const cwd = fixture('devDependencies'); return readManifest(cwd) @@ -399,7 +394,7 @@ describe('collect', function () { it('should not package .env file', async function () { const cwd = fixture('env'); - await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath() }), 'Expected package to throw: .env file should not be packaged'); + await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath() }), 'Expected package to throw: .env file should not be packaged'); }); it('allow packaging .env file with --allow-package-env-file', async function () { @@ -410,7 +405,7 @@ describe('collect', function () { it('should not package file which has a private key', async function () { const cwd = fixture('secrets'); const ignoreFile = path.join(cwd, 'secret1Ignore'); - await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: file which has a private key should not be packaged'); + await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: file which has a private key should not be packaged'); }); it('should scan files when CPU information is unavailable', async function () { @@ -422,7 +417,7 @@ describe('collect', function () { try { const cwd = fixture('secrets'); const ignoreFile = path.join(cwd, 'secret1Ignore'); - await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected secret scanning to run without CPU information'); + await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected secret scanning to run without CPU information'); } finally { Object.defineProperty(os, 'cpus', cpusDescriptor); } @@ -455,7 +450,7 @@ describe('collect', function () { it('should not package npm token', async function () { const cwd = fixture('secrets'); const ignoreFile = path.join(cwd, 'secret2Ignore'); - await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: should not package npm token'); + await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: should not package npm token'); }); it('npm token false positive 1', async function () {