Skip to content

Commit 82e2b7a

Browse files
committed
fix(cli): align utils/update/ + utils/command/ error messages with 4-ingredient strategy
Rewrites error messages across packages/cli/src/utils/update/ and packages/cli/src/utils/command/ to follow the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md. Sources: - utils/update/checker.mts: 8 messages (URL validation, package name / registry URL validation, version-response validation). Each now names the function, the received type, and what a valid value looks like. - utils/update/manager.mts: 3 messages (mirror guards for name / version / ttl). Still warn-and-return-false, but the text now tells the caller exactly which option was wrong. - utils/command/registry-core.mts: 6 messages (command / alias registration conflicts, middleware next() misuse, flag parsing failures). Each now names the offending command, flag name, or index so debuggers don't need to read source. Tests updated: - test/unit/utils/update/checker.test.mts: 6 assertions (switched to regex) - test/unit/utils/update/manager.test.mts: 3 assertions (switched to expect.stringContaining) - test/unit/utils/command/registry-core.test.mts: 5 assertions All 153 tests in the affected suites pass. Follows strategy from #1254. Part of the multi-PR series started by #1255 (commands/) and continued in #1256 (utils/dlx/).
1 parent fc5591f commit 82e2b7a

6 files changed

Lines changed: 79 additions & 31 deletions

File tree

packages/cli/src/utils/command/registry-core.mts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ export class CommandRegistry implements ICommandRegistry {
2424
*/
2525
register(command: CommandDefinition): void {
2626
if (this.commands.has(command.name)) {
27+
const existing = this.commands.get(command.name)
2728
throw new Error(
28-
`Command "${command.name}" is already registered. Use a unique name or unregister first.`,
29+
`cannot register command "${command.name}": already registered (existing definition has name="${existing?.name}"); call registry.unregister("${command.name}") first or pick a different name`,
2930
)
3031
}
3132

@@ -36,7 +37,7 @@ export class CommandRegistry implements ICommandRegistry {
3637
for (const alias of command.aliases) {
3738
if (this.commands.has(alias)) {
3839
throw new Error(
39-
`Alias "${alias}" conflicts with existing command "${this.commands.get(alias)?.name}"`,
40+
`cannot register command "${command.name}" alias "${alias}": conflicts with command "${this.commands.get(alias)?.name}"; rename the alias or unregister the conflicting command first`,
4041
)
4142
}
4243
// Store alias pointing to main command.
@@ -221,7 +222,9 @@ export class CommandRegistry implements ICommandRegistry {
221222

222223
const dispatch = async (i: number): Promise<void> => {
223224
if (i <= index) {
224-
throw new Error('next() called multiple times')
225+
throw new Error(
226+
`middleware at index ${index} called next() more than once (each middleware may invoke next() at most once); remove the extra next() call or split the middleware`,
227+
)
225228
}
226229

227230
index = i
@@ -287,17 +290,22 @@ export class CommandRegistry implements ICommandRegistry {
287290
} else {
288291
// --flag value format.
289292
if (i + 1 >= args.length) {
290-
throw new Error(`Missing value for flag --${flagName}`)
293+
throw new Error(
294+
`flag --${flagName} requires a ${flagDef.type} value but none was provided; pass it as --${flagName}=<value> or --${flagName} <value>`,
295+
)
291296
}
292297
value = args[++i]
293298
}
294299

295300
// Type conversion
296301
switch (flagDef.type) {
297302
case 'number': {
303+
const raw = value
298304
value = Number(value)
299305
if (Number.isNaN(value)) {
300-
throw new Error(`Invalid number value for --${flagName}: ${value}`)
306+
throw new Error(
307+
`flag --${flagName} requires a numeric value (saw: "${String(raw)}"); pass an integer or decimal like --${flagName}=42`,
308+
)
301309
}
302310
break
303311
}
@@ -321,7 +329,9 @@ export class CommandRegistry implements ICommandRegistry {
321329
// Validate required flags
322330
for (const [name, def] of Object.entries(command.flags)) {
323331
if (def.isRequired && flags[name] === undefined) {
324-
throw new Error(`Required flag --${name} is missing`)
332+
throw new Error(
333+
`command "${command.name}" requires --${name} but it was not provided; pass --${name}=<${def.type}-value>`,
334+
)
325335
}
326336
}
327337

packages/cli/src/utils/update/checker.mts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ const NetworkUtils = {
103103
timeoutMs = UPDATE_NOTIFIER_TIMEOUT,
104104
): Promise<{ version?: string }> {
105105
if (!isNonEmptyString(url)) {
106-
throw new Error('Invalid URL provided to fetch')
106+
throw new Error(
107+
`UpdateChecker.fetch(url) requires a non-empty string (got: ${typeof url === 'string' ? '""' : typeof url}); pass a valid registry URL like https://registry.npmjs.org/<package>`,
108+
)
107109
}
108110

109111
const { authInfo } = { __proto__: null, ...options } as FetchOptions
@@ -205,7 +207,9 @@ const NetworkUtils = {
205207
options: GetLatestVersionOptions = {},
206208
): Promise<string | undefined> {
207209
if (!isNonEmptyString(name)) {
208-
throw new Error('Package name must be a non-empty string')
210+
throw new Error(
211+
`getLatestVersion(name) requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); pass an npm package name like "socket" or "@socketsecurity/cli"`,
212+
)
209213
}
210214

211215
const { authInfo, registryUrl = NPM_REGISTRY_URL } = {
@@ -214,15 +218,19 @@ const NetworkUtils = {
214218
} as GetLatestVersionOptions
215219

216220
if (!isNonEmptyString(registryUrl)) {
217-
throw new Error('Registry URL must be a non-empty string')
221+
throw new Error(
222+
`getLatestVersion options.registryUrl must be a non-empty string (got: ${typeof registryUrl === 'string' ? '""' : typeof registryUrl}); omit it to default to ${NPM_REGISTRY_URL}`,
223+
)
218224
}
219225

220226
let normalizedRegistryUrl: string
221227
try {
222228
const url = new URL(registryUrl)
223229
normalizedRegistryUrl = url.toString()
224230
} catch {
225-
throw new Error(`Invalid registry URL: ${registryUrl}`)
231+
throw new Error(
232+
`options.registryUrl "${registryUrl}" is not a valid URL (new URL() threw); pass an absolute http(s) URL like ${NPM_REGISTRY_URL}`,
233+
)
226234
}
227235

228236
const maybeSlash = normalizedRegistryUrl.endsWith('/') ? '' : '/'
@@ -241,7 +249,9 @@ const NetworkUtils = {
241249
)
242250

243251
if (!json || !isNonEmptyString(json.version)) {
244-
throw new Error('Invalid version data in registry response')
252+
throw new Error(
253+
`${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`,
254+
)
245255
}
246256

247257
return json.version
@@ -284,11 +294,15 @@ async function checkForUpdates(
284294
} as UpdateCheckOptions
285295

286296
if (!isNonEmptyString(name)) {
287-
throw new Error('Package name must be a non-empty string')
297+
throw new Error(
298+
`checkForUpdates options.name requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); pass an npm package name like "socket" or "@socketsecurity/cli"`,
299+
)
288300
}
289301

290302
if (!isNonEmptyString(version)) {
291-
throw new Error('Current version must be a non-empty string')
303+
throw new Error(
304+
`checkForUpdates options.version requires a non-empty string (got: ${typeof version === 'string' ? '""' : typeof version}); pass the currently-installed semver like "1.2.3"`,
305+
)
292306
}
293307

294308
try {
@@ -298,7 +312,9 @@ async function checkForUpdates(
298312
})
299313

300314
if (!isNonEmptyString(latest)) {
301-
throw new Error('No version information available from registry')
315+
throw new Error(
316+
`registry returned no latest version for ${name} (getLatestVersion resolved to ${JSON.stringify(latest)}); check that ${name} exists on ${registryUrl || NPM_REGISTRY_URL}`,
317+
)
302318
}
303319

304320
const updateAvailable = isUpdateAvailable(version, latest)

packages/cli/src/utils/update/manager.mts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,23 @@ export async function checkForUpdates(
8080

8181
// Validate required parameters.
8282
if (!isNonEmptyString(name)) {
83-
loggerLocal.warn('Package name must be a non-empty string')
83+
loggerLocal.warn(
84+
`checkForUpdates options.name requires a non-empty string (got: ${typeof name === 'string' ? '""' : typeof name}); skipping update check`,
85+
)
8486
return false
8587
}
8688

8789
if (!isNonEmptyString(version)) {
88-
loggerLocal.warn('Current version must be a non-empty string')
90+
loggerLocal.warn(
91+
`checkForUpdates options.version requires a non-empty string (got: ${typeof version === 'string' ? '""' : typeof version}); skipping update check`,
92+
)
8993
return false
9094
}
9195

9296
if (ttl < 0) {
93-
loggerLocal.warn('TTL must be a non-negative number')
97+
loggerLocal.warn(
98+
`checkForUpdates options.ttl must be >= 0 (saw: ${ttl}); pass a positive number of milliseconds, e.g. 86_400_000 for 24h`,
99+
)
94100
return false
95101
}
96102

packages/cli/test/unit/utils/command/registry-core.test.mts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ describe('CommandRegistry', () => {
6060
registry.register(command)
6161

6262
expect(() => registry.register(command)).toThrow(
63-
'Command "test" is already registered',
63+
/cannot register command "test": already registered/,
6464
)
6565
})
6666

@@ -85,7 +85,7 @@ describe('CommandRegistry', () => {
8585
registry.register(cmd1)
8686

8787
expect(() => registry.register(cmd2)).toThrow(
88-
'Alias "test" conflicts with existing command "test"',
88+
/cannot register command "other" alias "test": conflicts with command "test"/,
8989
)
9090
})
9191
})
@@ -349,7 +349,9 @@ describe('CommandRegistry', () => {
349349
const result = await registry.execute('test', [])
350350

351351
expect(result.ok).toBe(false)
352-
expect(result.message).toContain('Required flag --name is missing')
352+
expect(result.message).toContain(
353+
'command "test" requires --name but it was not provided',
354+
)
353355
})
354356

355357
it('should run validation function', async () => {
@@ -404,7 +406,9 @@ describe('CommandRegistry', () => {
404406
const result = await registry.execute('test', ['--name'])
405407

406408
expect(result.ok).toBe(false)
407-
expect(result.message).toContain('Missing value for flag --name')
409+
expect(result.message).toContain(
410+
'flag --name requires a string value but none was provided',
411+
)
408412
})
409413

410414
it('should error when number flag has invalid value', async () => {
@@ -427,7 +431,7 @@ describe('CommandRegistry', () => {
427431
const result = await registry.execute('test', ['--count', 'notanumber'])
428432

429433
expect(result.ok).toBe(false)
430-
expect(result.message).toContain('Invalid number value for --count')
434+
expect(result.message).toContain('flag --count requires a numeric value')
431435
})
432436

433437
it('should parse array flags', async () => {

packages/cli/test/unit/utils/update/checker.test.mts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('update/checker', () => {
122122
describe('NetworkUtils.fetch', () => {
123123
it('throws error for empty URL', async () => {
124124
await expect(NetworkUtils.fetch('')).rejects.toThrow(
125-
'Invalid URL provided to fetch',
125+
/UpdateChecker\.fetch\(url\) requires a non-empty string/,
126126
)
127127
})
128128

@@ -264,14 +264,16 @@ describe('update/checker', () => {
264264
describe('NetworkUtils.getLatestVersion', () => {
265265
it('throws error for empty package name', async () => {
266266
await expect(NetworkUtils.getLatestVersion('')).rejects.toThrow(
267-
'Package name must be a non-empty string',
267+
/getLatestVersion\(name\) requires a non-empty string/,
268268
)
269269
})
270270

271271
it('throws error for invalid registry URL', async () => {
272272
await expect(
273273
NetworkUtils.getLatestVersion('test', { registryUrl: 'not-a-url' }),
274-
).rejects.toThrow('Invalid registry URL: not-a-url')
274+
).rejects.toThrow(
275+
/options\.registryUrl "not-a-url" is not a valid URL/,
276+
)
275277
})
276278

277279
it('returns latest version on success', async () => {
@@ -334,21 +336,25 @@ describe('update/checker', () => {
334336

335337
await expect(
336338
NetworkUtils.getLatestVersion('test-package'),
337-
).rejects.toThrow('Invalid version data in registry response')
339+
).rejects.toThrow(/responded without a \.version string/)
338340
})
339341
})
340342

341343
describe('checkForUpdates', () => {
342344
it('throws error for empty package name', async () => {
343345
await expect(
344346
checkForUpdates({ name: '', version: '1.0.0' }),
345-
).rejects.toThrow('Package name must be a non-empty string')
347+
).rejects.toThrow(
348+
/checkForUpdates options\.name requires a non-empty string/,
349+
)
346350
})
347351

348352
it('throws error for empty version', async () => {
349353
await expect(
350354
checkForUpdates({ name: 'test', version: '' }),
351-
).rejects.toThrow('Current version must be a non-empty string')
355+
).rejects.toThrow(
356+
/checkForUpdates options\.version requires a non-empty string/,
357+
)
352358
})
353359

354360
it('returns update check result when update is available', async () => {

packages/cli/test/unit/utils/update/manager.test.mts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ describe('update manager', () => {
9292

9393
expect(result).toBe(false)
9494
expect(mockLogger.warn).toHaveBeenCalledWith(
95-
'Package name must be a non-empty string',
95+
expect.stringContaining(
96+
'checkForUpdates options.name requires a non-empty string',
97+
),
9698
)
9799
})
98100

@@ -104,7 +106,9 @@ describe('update manager', () => {
104106

105107
expect(result).toBe(false)
106108
expect(mockLogger.warn).toHaveBeenCalledWith(
107-
'Current version must be a non-empty string',
109+
expect.stringContaining(
110+
'checkForUpdates options.version requires a non-empty string',
111+
),
108112
)
109113
})
110114

@@ -117,7 +121,9 @@ describe('update manager', () => {
117121

118122
expect(result).toBe(false)
119123
expect(mockLogger.warn).toHaveBeenCalledWith(
120-
'TTL must be a non-negative number',
124+
expect.stringContaining(
125+
'checkForUpdates options.ttl must be >= 0 (saw: -1)',
126+
),
121127
)
122128
})
123129

0 commit comments

Comments
 (0)