Skip to content

Commit 0a46c7b

Browse files
authored
chore: merge docs cleanup into main
2 parents 9d3d721 + 0ca3376 commit 0a46c7b

20 files changed

+1127
-328
lines changed

ERROR_HANDLING_GUIDE.md

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,43 @@ const { RecorderBusyError, RecorderBusyErr } = defineErrors({
7676
RecorderBusyErr() // no args
7777
```
7878

79-
**Reason-only (single descriptive field):**
79+
**Cause-wrapping (carries the raw caught error):**
8080
```typescript
8181
const { PlaySoundError, PlaySoundErr } = defineErrors({
82-
PlaySoundError: ({ reason }: { reason: string }) => ({
83-
message: `Failed to play sound: ${reason}`,
84-
reason,
82+
PlaySoundError: ({ cause }: { cause: unknown }) => ({
83+
message: `Failed to play sound: ${extractErrorMessage(cause)}`,
84+
cause,
8585
}),
8686
});
8787

88-
PlaySoundErr({ reason: extractErrorMessage(error) })
88+
PlaySoundErr({ cause: error })
8989
```
9090

91+
Accept `cause: unknown` (the raw caught error) and call `extractErrorMessage` inside the factory's message template — not at the call site. This keeps call sites clean (`{ cause: error }`) and centralizes message extraction where the message is composed. The anti-pattern to avoid is **string literal unions** (`'a' | 'b' | 'c'`) acting as sub-discriminants. See [Anti-Pattern: Discriminated Union Inputs in Error Factories](#anti-pattern-discriminated-union-inputs-in-error-factories) below.
92+
93+
#### Why `extractErrorMessage` belongs inside the constructor
94+
95+
**Anti-pattern — transforming at the call site:**
96+
```typescript
97+
// Every call site must remember to call extractErrorMessage
98+
try { playAudio(); } catch (error) {
99+
return PlaySoundErr({ reason: extractErrorMessage(error) });
100+
}
101+
```
102+
103+
**Preferred — constructor accepts raw `cause`:**
104+
```typescript
105+
// Call site just passes the raw error
106+
try { playAudio(); } catch (error) {
107+
return PlaySoundErr({ cause: error });
108+
}
109+
```
110+
111+
- **The constructor owns the message template — it should also own the transformation.** `extractErrorMessage` is a message-formatting concern; it belongs where the message string is assembled, not scattered across every call site.
112+
- **Raw `cause: unknown` preserves the original error for programmatic access.** Downstream code can inspect, log, or re-wrap the actual error object — not just a lossy string summary.
113+
- **Call sites stay minimal:** `{ cause: error }` instead of `{ reason: extractErrorMessage(error) }`.
114+
- **Mirrors Rust's `#[from]`**, where the enum variant handles the conversion from the source error type — callers just use `?`.
115+
91116
**Structured (multiple fields):**
92117
```typescript
93118
const responseErrors = defineErrors({
@@ -107,7 +132,7 @@ ResponseErr({ status: 404 })
107132
- `name` + `message` are the only built-in fields
108133
- Additional fields spread **flat** on the error object (no nested `context` bag)
109134
- `message` is always returned from the factory function — it is part of the return value, not a separate input
110-
- `cause` is not special — if needed, it's just another field
135+
- `cause: unknown` is the recommended way to wrap caught errors — call `extractErrorMessage(cause)` inside the message template, not at the call site
111136
- Only `name` is a reserved key — prevented by `NoReservedKeys` at compile time
112137
- Multiple errors can be grouped in a single `defineErrors` call, or defined individually
113138

@@ -155,6 +180,70 @@ export function createClipboardServiceExtension(): ClipboardService {
155180
};
156181
}
157182

183+
## Anti-Pattern: Discriminated Union Inputs in Error Factories
184+
185+
When a `defineErrors` variant's input contains a string literal union field — such as `reason: 'a' | 'b' | 'c'` or `operation: 'read' | 'write'` — that field is acting as a **sub-discriminant**, duplicating the role that variant names already serve. This is a code smell. Split into separate variants instead.
186+
187+
### Problems
188+
189+
**1. Double narrowing.** Consumers must first narrow on `error.name`, then narrow again on the sub-discriminant field. This defeats the purpose of the tagged union pattern, where a single `switch` on `name` should give you everything you need:
190+
191+
```typescript
192+
// Consumers are forced into two levels of narrowing
193+
if (error.name === 'InvalidAccelerator') {
194+
if (error.reason === 'invalid_format') {
195+
// now we finally know what happened
196+
}
197+
}
198+
```
199+
200+
**2. Dishonest types.** Fields start becoming optional because "some reasons don't use that field". The type says `accelerator?: string`, but the real contract is "required when reason is `'invalid_format'`, meaningless otherwise." TypeScript cannot express this conditional relationship within a single variant, so the type lies about the shape of the data.
201+
202+
**3. Message lookup tables.** A `const messages = { ... }` object inside the factory function is a strong signal that the variant is doing too much. Each lookup entry is really its own error with its own message template — it should be its own variant.
203+
204+
### Example
205+
206+
Beforea single variant with a string literal union acting as a sub-discriminant:
207+
208+
```typescript
209+
const ShortcutError = defineErrors({
210+
InvalidAccelerator: (input: {
211+
reason: 'invalid_format' | 'no_key_code' | 'multiple_key_codes';
212+
accelerator?: string;
213+
}) => {
214+
const messages = {
215+
invalid_format: `Invalid format: '${input.accelerator}'`,
216+
no_key_code: 'No valid key code found',
217+
multiple_key_codes: 'Multiple key codes not allowed',
218+
};
219+
return { message: messages[input.reason], ...input };
220+
},
221+
});
222+
```
223+
224+
Aftereach case becomes its own variant with honest, minimal types:
225+
226+
```typescript
227+
const ShortcutError = defineErrors({
228+
InvalidFormat: ({ accelerator }: { accelerator: string }) => ({
229+
message: `Invalid accelerator format: '${accelerator}'`,
230+
accelerator,
231+
}),
232+
NoKeyCode: () => ({
233+
message: 'No valid key code found in pressed keys',
234+
}),
235+
MultipleKeyCodes: () => ({
236+
message: 'Multiple key codes not allowed in accelerator',
237+
}),
238+
});
239+
```
240+
241+
Consumers now get full type information from a single `switch` on `error.name`, with no optional fields and no second level of narrowing.
242+
243+
### Exception
244+
245+
If the string literal field is genuinely metadata for logging or telemetryand no consumer ever switches on itkeeping it as a field is fine. The test is straightforward: **does any consumer narrow on this field?** If yes, it should be a variant name. If no consumer ever branches on it, it is metadata, not a discriminant, and a field is the right place for it.
246+
158247
## Error Classification Framework
159248

160249
### By Origin: New vs. Bubbled-Up Errors

NAMING_CONVENTION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,6 @@ if (isErr(result)) {
115115
3. **Be specific**: `Authentication` vs generic `Service`
116116
4. **Group by domain**: `UserError.Validation`, `HttpError.Timeout`, `DbError.Connection`
117117
5. **Namespace as the "Error" suffix**: The variable name carries the "Error" suffix (`UserError`), individual variants do not
118+
6. **No string literal unions as sub-discriminants**: If a variant input has a field like `reason: 'a' | 'b' | 'c'`, each literal should be its own variant instead. The variant name is the discriminant — fields should carry data, not act as a second tag to switch on
118119

119120
This convention provides clear semantics and helps developers understand the distinction between the error data itself and the data structures that contain it.

0 commit comments

Comments
 (0)