Skip to content

Commit 38b7bca

Browse files
jdaltonjdalton
authored
chore(sdk): adopt Error Messages doctrine + error helpers (supersedes #612) (#610)
* docs(claude): add Error Messages doctrine + references doc Add the fleet Error Messages section to CLAUDE.md, tuned for an SDK library context: - Four ingredients (What / Where / Saw vs. wanted / Fix). - Audience tiers with emphasis on library API (terse, stable message text) since SDK errors are caught by callers who may match on them. - Five baseline rules. - Two short SDK-flavored examples (orgSlug required, AuthError shape). Add `docs/references/error-messages.md` with cross-fleet worked examples and anti-patterns so CLAUDE.md stays tight. * docs(claude): reference joinAnd / joinOr helpers in Error Messages Point readers at @socketsecurity/lib/arrays' list-formatting helpers from CLAUDE.md (one-line rule) and the worked-examples references doc (new "Formatting lists of values" section). * chore: bump @socketsecurity/lib to 5.24.0 and adopt error helpers - package.json: 5.21.0 → 5.24.0. - src/http-client.ts: replace `e instanceof Error` check inside the JSON-parse fallback with `isError(e)` for cross-realm safety. - docs/references/error-messages.md: pick up the new "Working with caught values" section from socket-repo-template. Small codemod footprint — most of the SDK's error flow already runs through the shared `ResponseError` / `APIError` classes rather than raw `catch (e)`. Pre-commit tests skipped (DISABLE_PRECOMMIT_TEST): the single failing test (`resolveAbsPaths > should resolve array of relative paths to absolute`) asserts a literal `"socket-sdk-js/package.json"` substring which breaks in a `socket-sdk-js-errmsg` worktree — pre-existing assumption, unrelated to this change. `pnpm run check` (lint + type) passes clean. * style: rename catch (err|error) identifiers to catch (e) Mechanical rename across 3 files (src/file-upload.ts, src/http-client.ts, test/unit/http-client.test.mts) — 9 catch clauses converted. tsgo --noEmit clean. * chore(sdk): use isErrnoException helper for file-upload errno checks Replace the ad-hoc `e as NodeJS.ErrnoException` cast in file-upload.ts with the @socketsecurity/lib/errors isErrnoException type guard. Same runtime behavior — narrows e to NodeJS.ErrnoException only when the value is genuinely an errno exception (Error subclass with a string .code) — but removes an unchecked type assertion and matches the error-helper adoption pattern elsewhere in this PR. * docs(claude): trim Error Messages section, push rules + examples to refs CLAUDE.md has a hard size ceiling and the Error Messages section had drifted to 32 lines — duplicating content that already lives in docs/references/error-messages.md. Slim CLAUDE.md to 11 lines: the four-ingredient core, the SDK-tier stance (one sentence), and pointers to the helpers + refs doc. Every rule / length tier / worked example still has a home, just in the one place instead of two. Move the two SDK-specific Examples (orgSlug required, 401 rejected) into the refs doc's Library API errors table so nothing's lost. Saves ~1 KB / 18 lines in CLAUDE.md. * test(http-client): use isError helper instead of instanceof Error Matches the pattern adopted in src/http-client.ts — tests should exercise the same Error-detection helper the production code uses. Addresses jmsjtu's review feedback on #610.
1 parent b9f7a13 commit 38b7bca

5 files changed

Lines changed: 226 additions & 35 deletions

File tree

‎CLAUDE.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ Features: TypeScript support, API client, package analysis, security scanning, o
9797
- **Check all**: `pnpm check`
9898
- **Coverage**: `pnpm run cover`
9999

100+
## ERROR MESSAGES
101+
102+
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:
103+
104+
1. **What** — the rule that was broken, not the fallout (`must be non-empty`, not `invalid`).
105+
2. **Where** — exact method, argument, field, or URL.
106+
3. **Saw vs. wanted** — the bad value and the allowed shape or set.
107+
4. **Fix** — one concrete action, imperative voice (`pass an org slug`, not `the org slug was missing`).
108+
109+
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')`.
110+
111+
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`.
112+
113+
See `docs/references/error-messages.md` for length tiers (validator / programmatic), the full rule list, worked examples, anti-patterns, and helper signatures.
114+
100115
## Agents & Skills
101116

102117
- `/security-scan` — AgentShield + zizmor security audit

‎docs/references/error-messages.md‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
# Error Messages — Worked Examples
2+
3+
Companion to the `## Error Messages` section of `CLAUDE.md`. That section
4+
holds the rules; this file holds longer examples and anti-patterns that
5+
would bloat CLAUDE.md if inlined.
6+
7+
## The four ingredients
8+
9+
Every message needs, in order:
10+
11+
1. **What** — the rule that was broken.
12+
2. **Where** — the exact file, line, key, field, or CLI flag.
13+
3. **Saw vs. wanted** — the bad value and the allowed shape or set.
14+
4. **Fix** — one concrete action, in imperative voice.
15+
16+
## Library API errors (terse)
17+
18+
Callers may match on the message text, so stability matters. Aim for one
19+
sentence.
20+
21+
| ✗ / ✓ | Message | Notes |
22+
| ----- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- |
23+
| ✗ | `Error: invalid component` | No rule, no saw, no where. |
24+
| ✗ | `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. |
25+
| ✓ | `npm "name" component is required` | Rule + where + implied saw (missing). Six words. |
26+
| ✗ | `Error: bad name` | No rule. |
27+
| ✓ | `name "__proto__" cannot start with an underscore` | Rule, where (`name`), saw (`__proto__`), fix implied. |
28+
| ✗ | `Error: invalid argument` | No where, no rule, no fix. |
29+
| ✓ | `orgSlug is required` | Rule + where (`orgSlug`), saw (missing), implies fix. |
30+
| ✗ | `Error: request failed` | No status, no hint what to check. |
31+
| ✓ | `Socket API rejected the token (401); check SOCKET_API_TOKEN` | Rule (401), where (token), fix (check env var). |
32+
33+
## Validator / config / build-tool errors (verbose)
34+
35+
The reader is looking at a file and wants to fix the record without
36+
re-running the tool. Give each ingredient its own words.
37+
38+
✗ `Error: invalid tour config`
39+
40+
✓ `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 /<slug>/part/3 at publish time.`
41+
42+
Breakdown:
43+
44+
- **What**: `is missing "filename"` — the rule is "each part has a filename".
45+
- **Where**: `tour.json: part 3 ("Parsing & Normalization")` — file + record + human label.
46+
- **Saw vs. wanted**: saw = missing; wanted = a single-word lowercase filename, with `"parsing"` as a concrete model.
47+
- **Fix**: `Add … to this part` — imperative, specific.
48+
49+
The trailing `to route /<slug>/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").
50+
51+
## Programmatic errors (terse, rule only)
52+
53+
Internal assertions and invariant checks. No end user will read them;
54+
terse keeps the assertion readable when you skim the code.
55+
56+
- ✓ `assert(queue.length > 0)` with message `queue drained before worker exit`
57+
- ✓ `pool size must be positive`
58+
- ✗ `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.
59+
60+
## Common anti-patterns
61+
62+
**"Invalid X" with no rule.**
63+
64+
- ✗ `Invalid filename 'My Part'`
65+
- ✓ `filename 'My Part' must be [a-z]+ (lowercase, no spaces)`
66+
67+
**Passive voice on the fix.**
68+
69+
- ✗ `"filename" was missing`
70+
- ✓ `add "filename" to part 3`
71+
72+
**Naming only one side of a collision.**
73+
74+
- ✗ `duplicate key "foo"` (which record won, which lost?)
75+
- ✓ `duplicate key "foo" in config.json (lines 12 and 47) — rename one`
76+
77+
**Silently auto-correcting.**
78+
79+
- ✗ Stripping a trailing slash from a URL and continuing. The next run will hit the same bug; nothing learned.
80+
- ✓ `url "https://api/" has a trailing slash — remove it`.
81+
82+
**Bloat that restates the rule.**
83+
84+
- ✗ `The value provided for "timeout" is invalid because timeouts must be positive numbers and the value you provided was not a positive number.`
85+
- ✓ `timeout must be a positive number (saw: -5)`
86+
87+
## Formatting lists of values
88+
89+
When the error needs to show an allowed set, a list of conflicting
90+
records, or multiple missing fields, use the list formatters from
91+
`@socketsecurity/lib/arrays` rather than hand-joining with commas:
92+
93+
- `joinAnd(['a', 'b', 'c'])` → `"a, b, and c"` — for conjunctions ("missing foo, bar, and baz")
94+
- `joinOr(['npm', 'pypi', 'maven'])` → `"npm, pypi, or maven"` — for disjunctions ("must be one of: …")
95+
96+
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"`).
97+
98+
- ✗ `--reach-ecosystems must be one of: npm, pypi, maven (saw: "foo")` — hand-joined, breaks if the list has one or two entries.
99+
- ✓ `` `--reach-ecosystems must be one of: ${joinOr(ALLOWED)} (saw: "foo")` ``
100+
- ✗ `missing keys: filename slug title` — no separators, no grammar.
101+
- ✓ `` `missing keys: ${joinAnd(missing)}` `` → `"missing keys: filename, slug, and title"`
102+
103+
Use `joinOr` whenever the error is "must be one of X", `joinAnd` whenever it's "all of X are required / missing / in conflict".
104+
105+
## Working with caught values
106+
107+
`catch (e)` binds `unknown`. The helpers in `@socketsecurity/lib/errors` cover the four patterns that recur everywhere:
108+
109+
```ts
110+
import {
111+
errorMessage,
112+
errorStack,
113+
isError,
114+
isErrnoException,
115+
} from '@socketsecurity/lib/errors'
116+
```
117+
118+
### `isError(value)` — replaces `value instanceof Error`
119+
120+
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.
121+
122+
- ✗ `if (e instanceof Error) { … }`
123+
- ✓ `if (isError(e)) { … }`
124+
125+
### `isErrnoException(value)` — replaces `'code' in err` guards
126+
127+
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`.
128+
129+
- ✗ `if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') { … }`
130+
- ✓ `if (isErrnoException(e) && e.code === 'ENOENT') { … }`
131+
132+
### `errorMessage(value)` — replaces the `instanceof Error ? e.message : String(e)` pattern
133+
134+
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.
135+
136+
That last bullet is the important one: **every `|| 'Unknown error'` fallback in the fleet should collapse into a single `errorMessage(e)` call.**
137+
138+
- ✗ `` `Failed: ${e instanceof Error ? e.message : String(e)}` ``
139+
- ✗ `` `Failed: ${(e as Error)?.message ?? 'Unknown error'}` ``
140+
- ✗ `` `Failed: ${e instanceof Error ? e.message : 'Unknown error'}` ``
141+
- ✓ `` `Failed: ${errorMessage(e)}` ``
142+
143+
When you want to preserve the cause chain upstream (recommended), pair it with `{ cause }`:
144+
145+
```ts
146+
try {
147+
await readConfig(path)
148+
} catch (e) {
149+
throw new Error(`Failed to read ${path}: ${errorMessage(e)}`, { cause: e })
150+
}
151+
```
152+
153+
### `errorStack(value)` — cause-aware stack, or `undefined`
154+
155+
Returns the cause-walking stack for Errors; returns `undefined` for non-Errors so logger calls stay safe:
156+
157+
```ts
158+
logger.error(`rebuild failed: ${errorMessage(e)}`, { stack: errorStack(e) })
159+
```
160+
161+
## Voice & tone
162+
163+
- Imperative for the fix: `rename`, `add`, `remove`, `set`.
164+
- Present tense for the rule: `must be`, `cannot`, `is required`.
165+
- No apology ("Sorry, …"), no blame ("You provided …"). State the rule and the fix.
166+
- Don't end with "please"; it doesn't add information and it makes the message feel longer than it is.
167+
168+
## Bloat check
169+
170+
Before shipping a message, cross out any word that, if removed, leaves the information intact. If only rhythm or politeness disappears, drop it.

‎src/file-upload.ts‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import path from 'node:path'
33

44
import FormData from 'form-data'
55

6+
import { isErrnoException } from '@socketsecurity/lib/errors'
67
import { httpRequest } from '@socketsecurity/lib/http-request'
78
import { normalizePath } from '@socketsecurity/lib/paths/normalize'
89

@@ -27,19 +28,21 @@ export function createRequestBodyForFilepaths(
2728
try {
2829
stream = createReadStream(absPath, { highWaterMark: 1024 * 1024 })
2930
/* c8 ignore next 13 - createReadStream throws synchronously only for type validation errors; file system errors (ENOENT, EISDIR) are emitted asynchronously */
30-
} catch (error) {
31-
const err = error as NodeJS.ErrnoException
31+
} catch (e) {
3232
let message = `Failed to read file: ${absPath}`
33-
if (err.code === 'ENOENT') {
34-
message += '\n→ File does not exist. Check the file path and try again.'
35-
} else if (err.code === 'EACCES') {
36-
message += `\n→ Permission denied. Run: chmod +r "${absPath}"`
37-
} else if (err.code === 'EISDIR') {
38-
message += '\n→ Expected a file but found a directory.'
39-
} else if (err.code) {
40-
message += `\n→ Error code: ${err.code}`
33+
if (isErrnoException(e)) {
34+
if (e.code === 'ENOENT') {
35+
message +=
36+
'\n→ File does not exist. Check the file path and try again.'
37+
} else if (e.code === 'EACCES') {
38+
message += `\n→ Permission denied. Run: chmod +r "${absPath}"`
39+
} else if (e.code === 'EISDIR') {
40+
message += '\n→ Expected a file but found a directory.'
41+
} else if (e.code) {
42+
message += `\n→ Error code: ${e.code}`
43+
}
4144
}
42-
throw new Error(message, { cause: error })
45+
throw new Error(message, { cause: e })
4346
}
4447
form.append(relPath, stream, {
4548
contentType: 'application/octet-stream',
@@ -98,15 +101,15 @@ export async function createUploadRequest(
98101
}
99102

100103
return response
101-
} catch (error) {
104+
} catch (e) {
102105
if (hooks?.onResponse) {
103106
hooks.onResponse({
104107
method,
105108
url,
106109
duration: Date.now() - startTime,
107-
error: error as Error,
110+
error: e as Error,
108111
})
109112
}
110-
throw error
113+
throw e
111114
}
112115
}

‎src/http-client.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { debugLog } from '@socketsecurity/lib/debug'
2+
import { isError } from '@socketsecurity/lib/errors'
23
import { httpRequest } from '@socketsecurity/lib/http-request'
34
import { jsonParse } from '@socketsecurity/lib/json/parse'
45
import { perfTimer } from '@socketsecurity/lib/performance'
@@ -81,17 +82,17 @@ export async function createDeleteRequest(
8182
}
8283

8384
return response
84-
} catch (error) {
85+
} catch (e) {
8586
if (hooks?.onResponse) {
8687
hooks.onResponse({
8788
method,
8889
url,
8990
duration: Date.now() - startTime,
90-
error: error as Error,
91+
error: e as Error,
9192
})
9293
}
9394

94-
throw error
95+
throw e
9596
}
9697
}
9798

@@ -140,19 +141,19 @@ export async function createGetRequest(
140141
}
141142

142143
return response
143-
} catch (error) {
144+
} catch (e) {
144145
stopTimer({ error: true })
145146

146147
if (hooks?.onResponse) {
147148
hooks.onResponse({
148149
method,
149150
url,
150151
duration: Date.now() - startTime,
151-
error: error as Error,
152+
error: e as Error,
152153
})
153154
}
154155

155-
throw error
156+
throw e
156157
}
157158
}
158159

@@ -210,19 +211,19 @@ export async function createRequestWithJson(
210211
}
211212

212213
return response
213-
} catch (error) {
214+
} catch (e) {
214215
stopTimer({ error: true })
215216

216217
if (hooks?.onResponse) {
217218
hooks.onResponse({
218219
method,
219220
url,
220221
duration: Date.now() - startTime,
221-
error: error as Error,
222+
error: e as Error,
222223
})
223224
}
224225

225-
throw error
226+
throw e
226227
}
227228
}
228229

@@ -301,7 +302,7 @@ export async function getResponseJson(
301302
throw enhancedError
302303
}
303304
/* c8 ignore start - Error instanceof check and unknown error handling for JSON parsing edge cases. */
304-
if (e instanceof Error) {
305+
if (isError(e)) {
305306
throw e
306307
}
307308
const unknownError = new Error('Unknown JSON parsing error', {
@@ -315,9 +316,9 @@ export async function getResponseJson(
315316
throw unknownError
316317
/* c8 ignore stop */
317318
}
318-
} catch (error) {
319+
} catch (e) {
319320
stopTimer({ error: true })
320-
throw error
321+
throw e
321322
}
322323
}
323324

‎test/unit/http-client.test.mts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
reshapeArtifactForPublicPolicy,
1313
} from '../../src/http-client.js'
1414

15+
import { isError } from '@socketsecurity/lib/errors'
16+
1517
import type { HttpResponse } from '@socketsecurity/lib/http-request'
1618
import type { Server } from 'node:http'
1719

@@ -295,9 +297,9 @@ describe('HTTP Client - Error Handling', () => {
295297
try {
296298
await createGetRequest(invalidUrl, '/test', { timeout: 100 })
297299
expect.fail('Should have thrown an error')
298-
} catch (error) {
299-
expect(error).toBeDefined()
300-
expect(error instanceof Error).toBe(true)
300+
} catch (e) {
301+
expect(e).toBeDefined()
302+
expect(isError(e)).toBe(true)
301303
}
302304
})
303305

@@ -315,9 +317,9 @@ describe('HTTP Client - Error Handling', () => {
315317
},
316318
)
317319
expect.fail('Should have thrown an error')
318-
} catch (error) {
319-
expect(error).toBeDefined()
320-
expect(error instanceof Error).toBe(true)
320+
} catch (e) {
321+
expect(e).toBeDefined()
322+
expect(isError(e)).toBe(true)
321323
}
322324
})
323325

@@ -328,9 +330,9 @@ describe('HTTP Client - Error Handling', () => {
328330
})
329331
await getResponseJson(response)
330332
expect.fail('Should have thrown a JSON parsing error')
331-
} catch (error) {
332-
expect(error).toBeDefined()
333-
expect(error instanceof SyntaxError).toBe(true)
333+
} catch (e) {
334+
expect(e).toBeDefined()
335+
expect(e instanceof SyntaxError).toBe(true)
334336
}
335337
})
336338
})

0 commit comments

Comments
 (0)