Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions packages/cli/src/utils/command/registry-core.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
)
}

Expand All @@ -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.
Expand Down Expand Up @@ -221,7 +222,12 @@ export class CommandRegistry implements ICommandRegistry {

const dispatch = async (i: number): Promise<void> => {
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`,
)
Comment thread
cursor[bot] marked this conversation as resolved.
}

index = i
Expand Down Expand Up @@ -287,17 +293,22 @@ 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}=<value> or --${flagName} <value>`,
)
}
value = args[++i]
}

// 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
}
Expand All @@ -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>`,
)
}
}

Expand Down
32 changes: 24 additions & 8 deletions packages/cli/src/utils/update/checker.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<package>`,
)
Comment thread
cursor[bot] marked this conversation as resolved.
}

const { authInfo } = { __proto__: null, ...options } as FetchOptions
Expand Down Expand Up @@ -205,7 +207,9 @@ const NetworkUtils = {
options: GetLatestVersionOptions = {},
): Promise<string | undefined> {
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 } = {
Expand All @@ -214,15 +218,19 @@ 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
try {
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('/') ? '' : '/'
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions packages/cli/src/utils/update/manager.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
14 changes: 9 additions & 5 deletions packages/cli/test/unit/utils/command/registry-core.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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/,
)
})

Expand All @@ -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"/,
)
})
})
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
18 changes: 12 additions & 6 deletions packages/cli/test/unit/utils/update/checker.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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/,
)
})

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -334,21 +336,25 @@ describe('update/checker', () => {

await expect(
NetworkUtils.getLatestVersion('test-package'),
).rejects.toThrow('Invalid version data in registry response')
).rejects.toThrow(/responded without a \.version string/)
})
})

describe('checkForUpdates', () => {
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 () => {
Expand Down
12 changes: 9 additions & 3 deletions packages/cli/test/unit/utils/update/manager.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
),
)
})

Expand All @@ -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',
),
)
})

Expand All @@ -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)',
),
)
})

Expand Down