Skip to content

Commit dac2f59

Browse files
committed
docs(claude): add Error Messages doctrine; adopt isErrnoException in fs
- CLAUDE.md: add the fleet Error Messages section with the caught-value helpers (isError / isErrnoException / errorMessage / errorStack). Uses `./errors` / `./arrays` paths since socket-lib self-references. - docs/references/error-messages.md: add the worked-examples companion doc from socket-repo-template. - src/fs.ts: two EEXIST ignore paths now use isErrnoException(e) instead of the hand-rolled `typeof e !== 'object' || … || !('code' in e)` idiom. Same semantics, cross-realm-safe. - test/unit/bin.test.mts: two `error instanceof Error` checks become isError(error) — exercises the canonical helper path. Out of scope: - test/unit/errors.test.mts:152,259 intentionally assert `remoteErr instanceof Error === false` to prove cross-realm Errors slip through `instanceof`. Keeping those as-is (they test the motivating semantics of isError). - src/spawn.ts `'code' in result` checks use a numeric exit code, not an errno string; isErrnoException would wrongly return false.
1 parent 8773f22 commit dac2f59

4 files changed

Lines changed: 212 additions & 14 deletions

File tree

CLAUDE.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,46 @@ The umbrella rule: never run a git command that mutates state belonging to a pat
9797
- Offer to checkpoint before risky changes
9898
- Flag files >400 LOC for potential splitting
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.
103+
104+
Every message needs four ingredients, in order:
105+
106+
1. **What** — the rule that was broken (e.g. "must be lowercase"), not the fallout ("invalid").
107+
2. **Where** — the exact file, line, key, field, or CLI flag. Not "somewhere in config".
108+
3. **Saw vs. wanted** — the bad value and the allowed shape or set.
109+
4. **Fix** — one concrete action, in imperative voice (`rename the key to …`, not `the key was not renamed`).
110+
111+
Length depends on the audience:
112+
113+
- **Library API errors** (thrown from a published package): terse. Callers may match on the message text, so every word counts. All four ingredients often fit in one sentence — e.g. `name "__proto__" cannot start with an underscore` covers rule, where (`name`), saw (`__proto__`), and implies the fix.
114+
- **Validator / config / build-tool errors** (developer reading a terminal): verbose. Give each ingredient its own words so the reader can find the bad record without re-running the tool.
115+
- **Programmatic errors** (internal assertions, invariant checks): terse, rule only. No end user will see it; short keeps the check readable.
116+
117+
Rules for every message:
118+
119+
- Imperative voice for the fix — `add "filename" to part 3`, not `"filename" was missing`.
120+
- Never "invalid" on its own. `invalid filename 'My Part'` is fallout; `filename 'My Part' must be [a-z]+ (lowercase, no spaces)` is a rule.
121+
- On a collision, name **both** sides, not just the second one found.
122+
- Suggest, don't auto-correct. Silently fixing state hides the bug next time.
123+
- Bloat check: if removing a word keeps the information, drop it.
124+
- For allowed-set / conflict lists, use `joinAnd` / `joinOr` from `./arrays``must be one of: ${joinOr(allowed)}` reads better than a hand-formatted list.
125+
126+
Caught-value helpers from `./errors` (prefer these over hand-rolled checks):
127+
128+
- `isError(e)` — replaces `e instanceof Error`. Cross-realm-safe (ES2025 `Error.isError` with a shim fallback); catches Errors from worker threads and `vm` contexts that `instanceof` misses.
129+
- `isErrnoException(e)` — replaces `'code' in err` / `typeof err.code === 'string'` guards. Narrows to `NodeJS.ErrnoException` for syscall/libuv failures (`'ENOENT'`, `'EACCES'`, …).
130+
- `errorMessage(e)` — replaces every `e instanceof Error ? e.message : String(e)` and any fallback ending in `'Unknown error'`. Walks the `cause` chain, coerces primitives, and returns the shared `UNKNOWN_ERROR` sentinel when nothing else yields a usable string.
131+
- `errorStack(e)` — cause-aware stack for Errors, `undefined` otherwise. Use with `logger.error(msg, { stack: errorStack(e) })`.
132+
133+
Examples:
134+
135+
-`Error: invalid config` → ✓ `config.json: part 3 is missing "filename". Add a lowercase filename (e.g. "parsing").`
136+
-`Error: invalid component` → ✓ `npm "name" component is required`
137+
138+
See `docs/references/error-messages.md` for worked examples and anti-patterns.
139+
100140
## ABSOLUTE RULES
101141

102142
- Never create files unless necessary; always prefer editing existing files

docs/references/error-messages.md

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

src/fs.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import process from 'node:process'
77

88
import { isArray } from './arrays'
99
import { getAbortSignal } from './constants/process'
10+
import { isErrnoException } from './errors'
1011
import { defaultIgnore, getGlobMatcher } from './globs'
1112
import { jsonParse } from './json/parse'
1213
import { objectFreeze, type Remap } from './objects'
@@ -1493,12 +1494,7 @@ export async function safeMkdir(
14931494
await fs.promises.mkdir(path, opts)
14941495
} catch (e: unknown) {
14951496
// Ignore EEXIST (directory already exists); re-throw everything else.
1496-
if (
1497-
typeof e !== 'object' ||
1498-
e === null ||
1499-
!('code' in e) ||
1500-
(e as NodeJS.ErrnoException).code !== 'EEXIST'
1501-
) {
1497+
if (!isErrnoException(e) || e.code !== 'EEXIST') {
15021498
throw e
15031499
}
15041500
}
@@ -1543,12 +1539,7 @@ export function safeMkdirSync(
15431539
fs.mkdirSync(path, opts)
15441540
} catch (e: unknown) {
15451541
// Ignore EEXIST (directory already exists); re-throw everything else.
1546-
if (
1547-
typeof e !== 'object' ||
1548-
e === null ||
1549-
!('code' in e) ||
1550-
(e as NodeJS.ErrnoException).code !== 'EEXIST'
1551-
) {
1542+
if (!isErrnoException(e) || e.code !== 'EEXIST') {
15521543
throw e
15531544
}
15541545
}

test/unit/bin.test.mts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
whichReal,
2626
whichRealSync,
2727
} from '@socketsecurity/lib/bin'
28+
import { isError } from '@socketsecurity/lib/errors'
2829
import { describe, expect, it } from 'vitest'
2930
import { runWithTempDir } from './utils/temp-file-helper'
3031

@@ -227,7 +228,7 @@ describe('bin', () => {
227228
} catch (error) {
228229
// Skip if symlinks are not supported on this platform
229230
if (
230-
error instanceof Error &&
231+
isError(error) &&
231232
(error.message.includes('EPERM') ||
232233
error.message.includes('operation not permitted'))
233234
) {
@@ -709,7 +710,7 @@ exec node "$basedir/pnpm/bin/pnpm.cjs" "$@"
709710
await execBin('nonexistent-bin-12345')
710711
} catch (error) {
711712
expect(error).toBeInstanceOf(Error)
712-
if (error instanceof Error) {
713+
if (isError(error)) {
713714
expect((error as any).code).toBe('ENOENT')
714715
}
715716
}

0 commit comments

Comments
 (0)