diff --git a/CLAUDE.md b/CLAUDE.md index 07034a21..550a72dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,6 +97,21 @@ Features: TypeScript support, API client, package analysis, security scanning, o - **Check all**: `pnpm check` - **Coverage**: `pnpm run cover` +## ERROR MESSAGES + +An error message is UI. The reader should be able to fix the problem from the message alone, without opening your source. Every message needs four ingredients, in order: + +1. **What** — the rule that was broken, not the fallout (`must be non-empty`, not `invalid`). +2. **Where** — exact method, argument, field, or URL. +3. **Saw vs. wanted** — the bad value and the allowed shape or set. +4. **Fix** — one concrete action, imperative voice (`pass an org slug`, not `the org slug was missing`). + +SDK errors are **terse** — callers may `catch` and match on message text, so every word counts. One sentence covering all four is the norm: `throw new Error('orgSlug is required')`. + +Prefer the caught-value helpers from `@socketsecurity/lib/errors` (`isError`, `isErrnoException`, `errorMessage`, `errorStack`) over hand-rolled `instanceof Error` / `'code' in e` checks. For allowed-set / conflict lists, use `joinAnd` / `joinOr` from `@socketsecurity/lib/arrays`. + +See `docs/references/error-messages.md` for length tiers (validator / programmatic), the full rule list, worked examples, anti-patterns, and helper signatures. + ## Agents & Skills - `/security-scan` — AgentShield + zizmor security audit diff --git a/docs/references/error-messages.md b/docs/references/error-messages.md new file mode 100644 index 00000000..d7848a31 --- /dev/null +++ b/docs/references/error-messages.md @@ -0,0 +1,170 @@ +# Error Messages — Worked Examples + +Companion to the `## Error Messages` section of `CLAUDE.md`. That section +holds the rules; this file holds longer examples and anti-patterns that +would bloat CLAUDE.md if inlined. + +## The four ingredients + +Every message needs, in order: + +1. **What** — the rule that was broken. +2. **Where** — the exact file, line, key, field, or CLI flag. +3. **Saw vs. wanted** — the bad value and the allowed shape or set. +4. **Fix** — one concrete action, in imperative voice. + +## Library API errors (terse) + +Callers may match on the message text, so stability matters. Aim for one +sentence. + +| ✗ / ✓ | Message | Notes | +| ----- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | +| ✗ | `Error: invalid component` | No rule, no saw, no where. | +| ✗ | `The "name" component of type "npm" failed validation because the provided value "" is empty, which is not allowed because names are required; please provide a non-empty name.` | Restates the rule three times. | +| ✓ | `npm "name" component is required` | Rule + where + implied saw (missing). Six words. | +| ✗ | `Error: bad name` | No rule. | +| ✓ | `name "__proto__" cannot start with an underscore` | Rule, where (`name`), saw (`__proto__`), fix implied. | +| ✗ | `Error: invalid argument` | No where, no rule, no fix. | +| ✓ | `orgSlug is required` | Rule + where (`orgSlug`), saw (missing), implies fix. | +| ✗ | `Error: request failed` | No status, no hint what to check. | +| ✓ | `Socket API rejected the token (401); check SOCKET_API_TOKEN` | Rule (401), where (token), fix (check env var). | + +## Validator / config / build-tool errors (verbose) + +The reader is looking at a file and wants to fix the record without +re-running the tool. Give each ingredient its own words. + +✗ `Error: invalid tour config` + +✓ `tour.json: part 3 ("Parsing & Normalization") is missing "filename". Add a single-word lowercase filename (e.g. "parsing") to this part — one per part is required to route //part/3 at publish time.` + +Breakdown: + +- **What**: `is missing "filename"` — the rule is "each part has a filename". +- **Where**: `tour.json: part 3 ("Parsing & Normalization")` — file + record + human label. +- **Saw vs. wanted**: saw = missing; wanted = a single-word lowercase filename, with `"parsing"` as a concrete model. +- **Fix**: `Add … to this part` — imperative, specific. + +The trailing `to route //part/3 at publish time` is optional. Include a _why_ clause only when the rule is non-obvious; skip it for rules the reader already knows (e.g. "names can't start with an underscore"). + +## Programmatic errors (terse, rule only) + +Internal assertions and invariant checks. No end user will read them; +terse keeps the assertion readable when you skim the code. + +- ✓ `assert(queue.length > 0)` with message `queue drained before worker exit` +- ✓ `pool size must be positive` +- ✗ `An unexpected error occurred while trying to acquire a connection from the pool because the pool size was not positive.` — nothing a maintainer can act on that the rule itself doesn't already say. + +## Common anti-patterns + +**"Invalid X" with no rule.** + +- ✗ `Invalid filename 'My Part'` +- ✓ `filename 'My Part' must be [a-z]+ (lowercase, no spaces)` + +**Passive voice on the fix.** + +- ✗ `"filename" was missing` +- ✓ `add "filename" to part 3` + +**Naming only one side of a collision.** + +- ✗ `duplicate key "foo"` (which record won, which lost?) +- ✓ `duplicate key "foo" in config.json (lines 12 and 47) — rename one` + +**Silently auto-correcting.** + +- ✗ Stripping a trailing slash from a URL and continuing. The next run will hit the same bug; nothing learned. +- ✓ `url "https://api/" has a trailing slash — remove it`. + +**Bloat that restates the rule.** + +- ✗ `The value provided for "timeout" is invalid because timeouts must be positive numbers and the value you provided was not a positive number.` +- ✓ `timeout must be a positive number (saw: -5)` + +## Formatting lists of values + +When the error needs to show an allowed set, a list of conflicting +records, or multiple missing fields, use the list formatters from +`@socketsecurity/lib/arrays` rather than hand-joining with commas: + +- `joinAnd(['a', 'b', 'c'])` → `"a, b, and c"` — for conjunctions ("missing foo, bar, and baz") +- `joinOr(['npm', 'pypi', 'maven'])` → `"npm, pypi, or maven"` — for disjunctions ("must be one of: …") + +Both wrap `Intl.ListFormat`, so the Oxford comma and one-/two-item cases come out right for free (`joinOr(['a'])` → `"a"`; `joinOr(['a', 'b'])` → `"a or b"`). + +- ✗ `--reach-ecosystems must be one of: npm, pypi, maven (saw: "foo")` — hand-joined, breaks if the list has one or two entries. +- ✓ `` `--reach-ecosystems must be one of: ${joinOr(ALLOWED)} (saw: "foo")` `` +- ✗ `missing keys: filename slug title` — no separators, no grammar. +- ✓ `` `missing keys: ${joinAnd(missing)}` `` → `"missing keys: filename, slug, and title"` + +Use `joinOr` whenever the error is "must be one of X", `joinAnd` whenever it's "all of X are required / missing / in conflict". + +## Working with caught values + +`catch (e)` binds `unknown`. The helpers in `@socketsecurity/lib/errors` cover the four patterns that recur everywhere: + +```ts +import { + errorMessage, + errorStack, + isError, + isErrnoException, +} from '@socketsecurity/lib/errors' +``` + +### `isError(value)` — replaces `value instanceof Error` + +Cross-realm-safe. Uses the native ES2025 `Error.isError` when the engine ships it, falls back to a spec-compliant shim otherwise. Catches Errors from worker threads, `vm` contexts, and iframes that same-realm `instanceof Error` silently misses. + +- ✗ `if (e instanceof Error) { … }` +- ✓ `if (isError(e)) { … }` + +### `isErrnoException(value)` — replaces `'code' in err` guards + +Narrows to `NodeJS.ErrnoException` (an Error with a string `code` set by libuv/syscalls like `ENOENT`, `EACCES`, `EBUSY`, `EPERM`). Builds on `isError`, so it's also cross-realm-safe, and it checks that `code` is a string — a merely branded Error without a real errno code returns `false`. + +- ✗ `if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') { … }` +- ✓ `if (isErrnoException(e) && e.code === 'ENOENT') { … }` + +### `errorMessage(value)` — replaces the `instanceof Error ? e.message : String(e)` pattern + +Walks the `cause` chain via `messageWithCauses`, coerces primitives and objects to string, and returns the shared `UNKNOWN_ERROR` sentinel (the string `'Unknown error'`) for `null`, `undefined`, empty strings, `[object Object]`, or Errors with no message. + +That last bullet is the important one: **every `|| 'Unknown error'` fallback in the fleet should collapse into a single `errorMessage(e)` call.** + +- ✗ `` `Failed: ${e instanceof Error ? e.message : String(e)}` `` +- ✗ `` `Failed: ${(e as Error)?.message ?? 'Unknown error'}` `` +- ✗ `` `Failed: ${e instanceof Error ? e.message : 'Unknown error'}` `` +- ✓ `` `Failed: ${errorMessage(e)}` `` + +When you want to preserve the cause chain upstream (recommended), pair it with `{ cause }`: + +```ts +try { + await readConfig(path) +} catch (e) { + throw new Error(`Failed to read ${path}: ${errorMessage(e)}`, { cause: e }) +} +``` + +### `errorStack(value)` — cause-aware stack, or `undefined` + +Returns the cause-walking stack for Errors; returns `undefined` for non-Errors so logger calls stay safe: + +```ts +logger.error(`rebuild failed: ${errorMessage(e)}`, { stack: errorStack(e) }) +``` + +## Voice & tone + +- Imperative for the fix: `rename`, `add`, `remove`, `set`. +- Present tense for the rule: `must be`, `cannot`, `is required`. +- No apology ("Sorry, …"), no blame ("You provided …"). State the rule and the fix. +- Don't end with "please"; it doesn't add information and it makes the message feel longer than it is. + +## Bloat check + +Before shipping a message, cross out any word that, if removed, leaves the information intact. If only rhythm or politeness disappears, drop it. diff --git a/package.json b/package.json index b776b7bc..9ae02d43 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "@babel/traverse": "7.26.4", "@babel/types": "7.26.3", "@oxlint/migrate": "1.52.0", - "@socketsecurity/lib": "5.21.0", + "@socketsecurity/lib": "5.24.0", "@sveltejs/acorn-typescript": "1.0.8", "@types/babel__traverse": "7.28.0", "@types/node": "24.9.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f54f019b..1f2b3711 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -79,8 +79,8 @@ importers: specifier: 1.52.0 version: 1.52.0(@emnapi/core@1.9.2)(@emnapi/runtime@1.9.2) '@socketsecurity/lib': - specifier: 5.21.0 - version: 5.21.0(typescript@5.9.3) + specifier: 5.24.0 + version: 5.24.0(typescript@5.9.3) '@sveltejs/acorn-typescript': specifier: 1.0.8 version: 1.0.8(acorn@8.15.0) @@ -1236,6 +1236,15 @@ packages: typescript: optional: true + '@socketsecurity/lib@5.24.0': + resolution: {integrity: sha512-4Yar8oo4N12ESoNt/i2PNf08HRABUC0OcfUfwzIF3xjq89E5VMDN+aeOtnn6Oo4Y6u3TiuZRG7NgEBZ83LQ1Lw==} + engines: {node: '>=22', pnpm: '>=11.0.0-rc.0'} + peerDependencies: + typescript: '>=5.0.0' + peerDependenciesMeta: + typescript: + optional: true + '@socketsecurity/sdk@4.0.1': resolution: {integrity: sha512-fe3DQp2dFwhc0G6Za36GIMSV+QaPAP5L96K3ZOtywt9nhbwxc9IQwqzdOVztdn5Rbez3t9EHU9Esj24/hWdP0g==} engines: {node: '>=18.20.8', pnpm: '>=11.0.0-rc.0'} @@ -2958,6 +2967,10 @@ snapshots: optionalDependencies: typescript: 5.9.3 + '@socketsecurity/lib@5.24.0(typescript@5.9.3)': + optionalDependencies: + typescript: 5.9.3 + '@socketsecurity/sdk@4.0.1': {} '@standard-schema/spec@1.1.0': {} diff --git a/src/file-upload.ts b/src/file-upload.ts index 23485130..94247341 100644 --- a/src/file-upload.ts +++ b/src/file-upload.ts @@ -3,6 +3,7 @@ import path from 'node:path' import FormData from 'form-data' +import { isErrnoException } from '@socketsecurity/lib/errors' import { httpRequest } from '@socketsecurity/lib/http-request' import { normalizePath } from '@socketsecurity/lib/paths/normalize' @@ -27,19 +28,21 @@ export function createRequestBodyForFilepaths( try { stream = createReadStream(absPath, { highWaterMark: 1024 * 1024 }) /* c8 ignore next 13 - createReadStream throws synchronously only for type validation errors; file system errors (ENOENT, EISDIR) are emitted asynchronously */ - } catch (error) { - const err = error as NodeJS.ErrnoException + } catch (e) { let message = `Failed to read file: ${absPath}` - if (err.code === 'ENOENT') { - message += '\n→ File does not exist. Check the file path and try again.' - } else if (err.code === 'EACCES') { - message += `\n→ Permission denied. Run: chmod +r "${absPath}"` - } else if (err.code === 'EISDIR') { - message += '\n→ Expected a file but found a directory.' - } else if (err.code) { - message += `\n→ Error code: ${err.code}` + if (isErrnoException(e)) { + if (e.code === 'ENOENT') { + message += + '\n→ File does not exist. Check the file path and try again.' + } else if (e.code === 'EACCES') { + message += `\n→ Permission denied. Run: chmod +r "${absPath}"` + } else if (e.code === 'EISDIR') { + message += '\n→ Expected a file but found a directory.' + } else if (e.code) { + message += `\n→ Error code: ${e.code}` + } } - throw new Error(message, { cause: error }) + throw new Error(message, { cause: e }) } form.append(relPath, stream, { contentType: 'application/octet-stream', @@ -98,15 +101,15 @@ export async function createUploadRequest( } return response - } catch (error) { + } catch (e) { if (hooks?.onResponse) { hooks.onResponse({ method, url, duration: Date.now() - startTime, - error: error as Error, + error: e as Error, }) } - throw error + throw e } } diff --git a/src/http-client.ts b/src/http-client.ts index fead0d52..724c0c22 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -1,4 +1,5 @@ import { debugLog } from '@socketsecurity/lib/debug' +import { isError } from '@socketsecurity/lib/errors' import { httpRequest } from '@socketsecurity/lib/http-request' import { jsonParse } from '@socketsecurity/lib/json/parse' import { perfTimer } from '@socketsecurity/lib/performance' @@ -81,17 +82,17 @@ export async function createDeleteRequest( } return response - } catch (error) { + } catch (e) { if (hooks?.onResponse) { hooks.onResponse({ method, url, duration: Date.now() - startTime, - error: error as Error, + error: e as Error, }) } - throw error + throw e } } @@ -140,7 +141,7 @@ export async function createGetRequest( } return response - } catch (error) { + } catch (e) { stopTimer({ error: true }) if (hooks?.onResponse) { @@ -148,11 +149,11 @@ export async function createGetRequest( method, url, duration: Date.now() - startTime, - error: error as Error, + error: e as Error, }) } - throw error + throw e } } @@ -210,7 +211,7 @@ export async function createRequestWithJson( } return response - } catch (error) { + } catch (e) { stopTimer({ error: true }) if (hooks?.onResponse) { @@ -218,11 +219,11 @@ export async function createRequestWithJson( method, url, duration: Date.now() - startTime, - error: error as Error, + error: e as Error, }) } - throw error + throw e } } @@ -301,7 +302,7 @@ export async function getResponseJson( throw enhancedError } /* c8 ignore start - Error instanceof check and unknown error handling for JSON parsing edge cases. */ - if (e instanceof Error) { + if (isError(e)) { throw e } const unknownError = new Error('Unknown JSON parsing error', { @@ -315,9 +316,9 @@ export async function getResponseJson( throw unknownError /* c8 ignore stop */ } - } catch (error) { + } catch (e) { stopTimer({ error: true }) - throw error + throw e } } diff --git a/test/unit/http-client.test.mts b/test/unit/http-client.test.mts index a4b32f92..d36339f5 100644 --- a/test/unit/http-client.test.mts +++ b/test/unit/http-client.test.mts @@ -12,6 +12,8 @@ import { reshapeArtifactForPublicPolicy, } from '../../src/http-client.js' +import { isError } from '@socketsecurity/lib/errors' + import type { HttpResponse } from '@socketsecurity/lib/http-request' import type { Server } from 'node:http' @@ -295,9 +297,9 @@ describe('HTTP Client - Error Handling', () => { try { await createGetRequest(invalidUrl, '/test', { timeout: 100 }) expect.fail('Should have thrown an error') - } catch (error) { - expect(error).toBeDefined() - expect(error instanceof Error).toBe(true) + } catch (e) { + expect(e).toBeDefined() + expect(isError(e)).toBe(true) } }) @@ -315,9 +317,9 @@ describe('HTTP Client - Error Handling', () => { }, ) expect.fail('Should have thrown an error') - } catch (error) { - expect(error).toBeDefined() - expect(error instanceof Error).toBe(true) + } catch (e) { + expect(e).toBeDefined() + expect(isError(e)).toBe(true) } }) @@ -328,9 +330,9 @@ describe('HTTP Client - Error Handling', () => { }) await getResponseJson(response) expect.fail('Should have thrown a JSON parsing error') - } catch (error) { - expect(error).toBeDefined() - expect(error instanceof SyntaxError).toBe(true) + } catch (e) { + expect(e).toBeDefined() + expect(e instanceof SyntaxError).toBe(true) } }) })