diff --git a/packages/cli/src/utils/command/registry-core.mts b/packages/cli/src/utils/command/registry-core.mts index 71f9fe234..7e842d479 100644 --- a/packages/cli/src/utils/command/registry-core.mts +++ b/packages/cli/src/utils/command/registry-core.mts @@ -24,8 +24,9 @@ export class CommandRegistry implements ICommandRegistry { */ register(command: CommandDefinition): void { if (this.commands.has(command.name)) { + const existing = this.commands.get(command.name) throw new Error( - `Command "${command.name}" is already registered. Use a unique name or unregister first.`, + `cannot register command "${command.name}": already registered (existing definition has name="${existing?.name}"); call registry.unregister("${command.name}") first or pick a different name`, ) } @@ -36,7 +37,7 @@ export class CommandRegistry implements ICommandRegistry { for (const alias of command.aliases) { if (this.commands.has(alias)) { throw new Error( - `Alias "${alias}" conflicts with existing command "${this.commands.get(alias)?.name}"`, + `cannot register command "${command.name}" alias "${alias}": conflicts with command "${this.commands.get(alias)?.name}"; rename the alias or unregister the conflicting command first`, ) } // Store alias pointing to main command. @@ -221,7 +222,12 @@ export class CommandRegistry implements ICommandRegistry { const dispatch = async (i: number): Promise => { if (i <= index) { - throw new Error('next() called multiple times') + // Each middleware[k] receives a next() closure that calls dispatch(k + 1); + // if that closure was invoked twice, dispatch(i) is re-entered and the + // offender is the middleware that held the closure — position i - 1. + throw new Error( + `middleware at index ${i - 1} called next() more than once (each middleware may invoke next() at most once); remove the extra next() call or split the middleware`, + ) } index = i @@ -287,7 +293,9 @@ export class CommandRegistry implements ICommandRegistry { } else { // --flag value format. if (i + 1 >= args.length) { - throw new Error(`Missing value for flag --${flagName}`) + throw new Error( + `flag --${flagName} requires a ${flagDef.type} value but none was provided; pass it as --${flagName}= or --${flagName} `, + ) } value = args[++i] } @@ -295,9 +303,12 @@ export class CommandRegistry implements ICommandRegistry { // Type conversion switch (flagDef.type) { case 'number': { + const raw = value value = Number(value) if (Number.isNaN(value)) { - throw new Error(`Invalid number value for --${flagName}: ${value}`) + throw new Error( + `flag --${flagName} requires a numeric value (saw: "${String(raw)}"); pass an integer or decimal like --${flagName}=42`, + ) } break } @@ -321,7 +332,9 @@ export class CommandRegistry implements ICommandRegistry { // Validate required flags for (const [name, def] of Object.entries(command.flags)) { if (def.isRequired && flags[name] === undefined) { - throw new Error(`Required flag --${name} is missing`) + throw new Error( + `command "${command.name}" requires --${name} but it was not provided; pass --${name}=<${def.type}-value>`, + ) } } diff --git a/packages/cli/src/utils/update/checker.mts b/packages/cli/src/utils/update/checker.mts index 3b72b7d9f..d2ff65379 100644 --- a/packages/cli/src/utils/update/checker.mts +++ b/packages/cli/src/utils/update/checker.mts @@ -103,7 +103,9 @@ const NetworkUtils = { timeoutMs = UPDATE_NOTIFIER_TIMEOUT, ): Promise<{ version?: string }> { if (!isNonEmptyString(url)) { - throw new Error('Invalid URL provided to fetch') + throw new Error( + `NetworkUtils.fetch(url) requires a non-empty string (got: ${typeof url === 'string' ? '""' : typeof url}); pass a valid registry URL like https://registry.npmjs.org/`, + ) } const { authInfo } = { __proto__: null, ...options } as FetchOptions @@ -205,7 +207,9 @@ const NetworkUtils = { options: GetLatestVersionOptions = {}, ): Promise { if (!isNonEmptyString(name)) { - throw new Error('Package name must be a non-empty string') + throw new Error( + `getLatestVersion(name) requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); pass an npm package name like "socket" or "@socketsecurity/cli"`, + ) } const { authInfo, registryUrl = NPM_REGISTRY_URL } = { @@ -214,7 +218,9 @@ const NetworkUtils = { } as GetLatestVersionOptions if (!isNonEmptyString(registryUrl)) { - throw new Error('Registry URL must be a non-empty string') + throw new Error( + `getLatestVersion options.registryUrl must be a non-empty string (got: ${typeof registryUrl === 'string' ? '""' : typeof registryUrl}); omit it to default to ${NPM_REGISTRY_URL}`, + ) } let normalizedRegistryUrl: string @@ -222,7 +228,9 @@ const NetworkUtils = { const url = new URL(registryUrl) normalizedRegistryUrl = url.toString() } catch { - throw new Error(`Invalid registry URL: ${registryUrl}`) + throw new Error( + `options.registryUrl "${registryUrl}" is not a valid URL (new URL() threw); pass an absolute http(s) URL like ${NPM_REGISTRY_URL}`, + ) } const maybeSlash = normalizedRegistryUrl.endsWith('/') ? '' : '/' @@ -241,7 +249,9 @@ const NetworkUtils = { ) if (!json || !isNonEmptyString(json.version)) { - throw new Error('Invalid version data in registry response') + throw new Error( + `${latestUrl} responded without a .version string (got: ${JSON.stringify(json)?.slice(0, 200) ?? 'null'}); the registry may be misconfigured or ${name} may not exist — verify the URL in a browser`, + ) } return json.version @@ -284,11 +294,15 @@ async function checkForUpdates( } as UpdateCheckOptions if (!isNonEmptyString(name)) { - throw new Error('Package name must be a non-empty string') + throw new Error( + `checkForUpdates options.name requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); pass an npm package name like "socket" or "@socketsecurity/cli"`, + ) } if (!isNonEmptyString(version)) { - throw new Error('Current version must be a non-empty string') + throw new Error( + `checkForUpdates options.version requires a non-empty string (got: ${typeof version === 'string' ? '""' : typeof version}); pass the currently-installed semver like "1.2.3"`, + ) } try { @@ -298,7 +312,9 @@ async function checkForUpdates( }) if (!isNonEmptyString(latest)) { - throw new Error('No version information available from registry') + throw new Error( + `registry returned no latest version for ${name} (getLatestVersion resolved to ${JSON.stringify(latest)}); check that ${name} exists on ${registryUrl || NPM_REGISTRY_URL}`, + ) } const updateAvailable = isUpdateAvailable(version, latest) diff --git a/packages/cli/src/utils/update/manager.mts b/packages/cli/src/utils/update/manager.mts index 3214c8dd2..409c3fcbb 100644 --- a/packages/cli/src/utils/update/manager.mts +++ b/packages/cli/src/utils/update/manager.mts @@ -80,17 +80,23 @@ export async function checkForUpdates( // Validate required parameters. if (!isNonEmptyString(name)) { - loggerLocal.warn('Package name must be a non-empty string') + loggerLocal.warn( + `checkForUpdates options.name requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); skipping update check`, + ) return false } if (!isNonEmptyString(version)) { - loggerLocal.warn('Current version must be a non-empty string') + loggerLocal.warn( + `checkForUpdates options.version requires a non-empty string (got: ${typeof version === 'string' ? '""' : typeof version}); skipping update check`, + ) return false } if (ttl < 0) { - loggerLocal.warn('TTL must be a non-negative number') + loggerLocal.warn( + `checkForUpdates options.ttl must be >= 0 (saw: ${ttl}); pass a positive number of milliseconds, e.g. 86_400_000 for 24h`, + ) return false } diff --git a/packages/cli/test/unit/utils/command/registry-core.test.mts b/packages/cli/test/unit/utils/command/registry-core.test.mts index 10c832bdb..72525a810 100644 --- a/packages/cli/test/unit/utils/command/registry-core.test.mts +++ b/packages/cli/test/unit/utils/command/registry-core.test.mts @@ -60,7 +60,7 @@ describe('CommandRegistry', () => { registry.register(command) expect(() => registry.register(command)).toThrow( - 'Command "test" is already registered', + /cannot register command "test": already registered/, ) }) @@ -85,7 +85,7 @@ describe('CommandRegistry', () => { registry.register(cmd1) expect(() => registry.register(cmd2)).toThrow( - 'Alias "test" conflicts with existing command "test"', + /cannot register command "other" alias "test": conflicts with command "test"/, ) }) }) @@ -349,7 +349,9 @@ describe('CommandRegistry', () => { const result = await registry.execute('test', []) expect(result.ok).toBe(false) - expect(result.message).toContain('Required flag --name is missing') + expect(result.message).toContain( + 'command "test" requires --name but it was not provided', + ) }) it('should run validation function', async () => { @@ -404,7 +406,9 @@ describe('CommandRegistry', () => { const result = await registry.execute('test', ['--name']) expect(result.ok).toBe(false) - expect(result.message).toContain('Missing value for flag --name') + expect(result.message).toContain( + 'flag --name requires a string value but none was provided', + ) }) it('should error when number flag has invalid value', async () => { @@ -427,7 +431,7 @@ describe('CommandRegistry', () => { const result = await registry.execute('test', ['--count', 'notanumber']) expect(result.ok).toBe(false) - expect(result.message).toContain('Invalid number value for --count') + expect(result.message).toContain('flag --count requires a numeric value') }) it('should parse array flags', async () => { diff --git a/packages/cli/test/unit/utils/update/checker.test.mts b/packages/cli/test/unit/utils/update/checker.test.mts index e92c4be5d..966aecce4 100644 --- a/packages/cli/test/unit/utils/update/checker.test.mts +++ b/packages/cli/test/unit/utils/update/checker.test.mts @@ -122,7 +122,7 @@ describe('update/checker', () => { describe('NetworkUtils.fetch', () => { it('throws error for empty URL', async () => { await expect(NetworkUtils.fetch('')).rejects.toThrow( - 'Invalid URL provided to fetch', + /NetworkUtils\.fetch\(url\) requires a non-empty string/, ) }) @@ -264,14 +264,16 @@ describe('update/checker', () => { describe('NetworkUtils.getLatestVersion', () => { it('throws error for empty package name', async () => { await expect(NetworkUtils.getLatestVersion('')).rejects.toThrow( - 'Package name must be a non-empty string', + /getLatestVersion\(name\) requires a non-empty string/, ) }) it('throws error for invalid registry URL', async () => { await expect( NetworkUtils.getLatestVersion('test', { registryUrl: 'not-a-url' }), - ).rejects.toThrow('Invalid registry URL: not-a-url') + ).rejects.toThrow( + /options\.registryUrl "not-a-url" is not a valid URL/, + ) }) it('returns latest version on success', async () => { @@ -334,7 +336,7 @@ describe('update/checker', () => { await expect( NetworkUtils.getLatestVersion('test-package'), - ).rejects.toThrow('Invalid version data in registry response') + ).rejects.toThrow(/responded without a \.version string/) }) }) @@ -342,13 +344,17 @@ describe('update/checker', () => { it('throws error for empty package name', async () => { await expect( checkForUpdates({ name: '', version: '1.0.0' }), - ).rejects.toThrow('Package name must be a non-empty string') + ).rejects.toThrow( + /checkForUpdates options\.name requires a non-empty string/, + ) }) it('throws error for empty version', async () => { await expect( checkForUpdates({ name: 'test', version: '' }), - ).rejects.toThrow('Current version must be a non-empty string') + ).rejects.toThrow( + /checkForUpdates options\.version requires a non-empty string/, + ) }) it('returns update check result when update is available', async () => { diff --git a/packages/cli/test/unit/utils/update/manager.test.mts b/packages/cli/test/unit/utils/update/manager.test.mts index dc2cb2864..453c45742 100644 --- a/packages/cli/test/unit/utils/update/manager.test.mts +++ b/packages/cli/test/unit/utils/update/manager.test.mts @@ -92,7 +92,9 @@ describe('update manager', () => { expect(result).toBe(false) expect(mockLogger.warn).toHaveBeenCalledWith( - 'Package name must be a non-empty string', + expect.stringContaining( + 'checkForUpdates options.name requires a non-empty string', + ), ) }) @@ -104,7 +106,9 @@ describe('update manager', () => { expect(result).toBe(false) expect(mockLogger.warn).toHaveBeenCalledWith( - 'Current version must be a non-empty string', + expect.stringContaining( + 'checkForUpdates options.version requires a non-empty string', + ), ) }) @@ -117,7 +121,9 @@ describe('update manager', () => { expect(result).toBe(false) expect(mockLogger.warn).toHaveBeenCalledWith( - 'TTL must be a non-negative number', + expect.stringContaining( + 'checkForUpdates options.ttl must be >= 0 (saw: -1)', + ), ) })