Skip to content

Commit 93a83a3

Browse files
committed
fix(stack): preserve non-Error rejection detail on the wasm-inline entry
`withResult`'s default `ensureError` REPLACES any non-Error throw with `new Error('Something went wrong')`, discarding the original value (@byteslice/result@0.2.0, dist/result.js:27). But that is only the FALLBACK — the library prefers an `onException` hook: const error = hooks?.onException?.(ex) ?? ensureError(ex) Nothing in this repo passes it. On the native entry that rarely bites, because the FFI throws real `ProtectError` instances. On WASM it is a live hazard: wasm-bindgen rejects with the raw `JsValue` the Rust side produced (`throw takeFromExternrefTable0(...)` in the generated glue), and the WASM build exports no `ProtectError` class at all — so a genuine FFI failure can arrive as a bare string or object. That made the Result conversion in the previous commit a partial REGRESSION: the old throwing behaviour at least propagated the raw value to the caller, whereas `withResult` was silently swallowing it. Fixed by supplying `onException`. The six call sites now go through one `wasmResult()` seam that binds both the failure shape and the hook, so a method added later cannot omit either — an omission would not fail a build, it would just quietly degrade failure messages, which is the whole bug. `toError` preserves strings as-is, JSON-serializes objects (so `{code, detail}` survives rather than becoming "[object Object]"), and falls back to `String()` for cycles and other unserializable values. +2 tests covering string and object preservation, plus the cyclic fallback. 833 pass; 0 biome errors; zero new tsc errors (diffed against main's error set, not counted).
1 parent bbd5b18 commit 93a83a3

2 files changed

Lines changed: 98 additions & 28 deletions

File tree

packages/stack/__tests__/wasm-inline-result-contract.test.ts

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,43 @@ describe('wasm-inline Result contract — failure path', () => {
120120
})
121121
})
122122

123-
// A non-Error throw still produces a well-formed `{ failure }`, but its
124-
// message is LOST: `withResult`'s `ensureError` replaces any non-Error with
125-
// `new Error('Something went wrong')` before `onError` ever sees it
126-
// (@byteslice/result@0.2.0, `dist/result.js:27`). That is a library-wide
127-
// characteristic — the native entry's `(error as Error).message` has it too
128-
// — not something this entry can fix locally. Pinned so the behaviour is
129-
// documented rather than surprising, and so the #532 bump to 0.5.0 shows up
130-
// here as a failing test if the library starts passing the value through.
131-
it('still returns a well-formed failure for a non-Error throw', async () => {
132-
ffi.encrypt.mockRejectedValueOnce('just a string')
123+
// Non-Error rejections must keep their detail. `withResult`'s default
124+
// `ensureError` would replace them with `new Error('Something went wrong')`,
125+
// discarding the value (@byteslice/result@0.2.0, `dist/result.js:27`) — so
126+
// this entry passes the `onException` hook, which takes precedence.
127+
//
128+
// This is not hypothetical on WASM: wasm-bindgen rejects with the raw
129+
// `JsValue` from Rust (`throw takeFromExternrefTable0(...)`), and the WASM
130+
// build exports no `ProtectError` class, so a real FFI failure can arrive as
131+
// a bare string or object. Losing it would be worse than the throwing
132+
// behaviour this entry had before, which propagated the raw value.
133+
it.each([
134+
['a string', 'boom from rust', 'boom from rust'],
135+
[
136+
'an object',
137+
{ code: 'EQL_X', detail: 'bad domain' },
138+
'{"code":"EQL_X","detail":"bad domain"}',
139+
],
140+
])('preserves the detail of %s rejection', async (_label, thrown, expected) => {
141+
ffi.encrypt.mockRejectedValueOnce(thrown)
142+
143+
const c = await client()
144+
const result = await c.encrypt('a@b.com', {
145+
table: users,
146+
column: users.email,
147+
})
148+
149+
expect(result.failure?.type).toBe('EncryptionError')
150+
expect(result.failure?.message).toBe(expected)
151+
expect(result.failure?.message).not.toBe('Something went wrong')
152+
})
153+
154+
it('falls back to String() for a value JSON cannot serialize', async () => {
155+
// A cycle makes JSON.stringify throw; the catch must still yield a
156+
// well-formed failure rather than propagating a second error.
157+
const cyclic: Record<string, unknown> = { a: 1 }
158+
cyclic.self = cyclic
159+
ffi.encrypt.mockRejectedValueOnce(cyclic)
133160

134161
const c = await client()
135162
const result = await c.encrypt('a@b.com', {
@@ -138,6 +165,6 @@ describe('wasm-inline Result contract — failure path', () => {
138165
})
139166

140167
expect(result.failure?.type).toBe('EncryptionError')
141-
expect(result.failure?.message).toBe('Something went wrong')
168+
expect(result.failure?.message).toBe('[object Object]')
142169
})
143170
})

packages/stack/src/wasm-inline.ts

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -350,16 +350,59 @@ function toFailure(
350350
): (error: unknown) => EncryptionError {
351351
return (error: unknown) => ({
352352
type,
353-
// In practice always an `Error`: `withResult`'s `ensureError` replaces any
354-
// non-Error throw with `new Error('Something went wrong')` before this
355-
// runs, discarding the original value (@byteslice/result@0.2.0). The
356-
// narrowing is kept anyway — it is correct on its own terms, and the #532
357-
// bump to 0.5.0 may start passing the raw value through.
358353
message: error instanceof Error ? error.message : String(error),
359354
code: getErrorCode(error),
360355
})
361356
}
362357

358+
/**
359+
* Coerce a rejection into an `Error` WITHOUT losing what it said.
360+
*
361+
* `withResult`'s built-in `ensureError` replaces any non-`Error` throw with
362+
* `new Error('Something went wrong')`, discarding the original value entirely
363+
* (@byteslice/result@0.2.0, `dist/result.js:27`). It is only the fallback,
364+
* though — `withResult` prefers the `onException` hook:
365+
*
366+
* ```js
367+
* const error = hooks?.onException?.(ex) ?? ensureError(ex)
368+
* ```
369+
*
370+
* Supplying it matters more here than on the native entry. wasm-bindgen
371+
* rejects with the raw `JsValue` the Rust side produced (`throw
372+
* takeFromExternrefTable0(...)` in the generated glue), and the WASM build
373+
* exports no `ProtectError` class, so a genuine FFI failure can arrive as a
374+
* plain string or object rather than an `Error`. Without this hook its message
375+
* would be replaced by boilerplate — strictly worse than the throwing
376+
* behaviour this entry had before, which at least propagated the raw value.
377+
*/
378+
function toError(ex: unknown): Error {
379+
if (ex instanceof Error) return ex
380+
if (typeof ex === 'string') return new Error(ex)
381+
try {
382+
// Objects are the other shape wasm-bindgen hands back, so serialize rather
383+
// than settle for "[object Object]". `JSON.stringify` returns undefined for
384+
// a symbol/function and throws on a cycle — `String` covers both.
385+
return new Error(JSON.stringify(ex) ?? String(ex))
386+
} catch {
387+
return new Error(String(ex))
388+
}
389+
}
390+
391+
/**
392+
* `withResult` bound to this entry's conventions: the repo-wide failure shape
393+
* ({@link toFailure}) plus the {@link toError} hook.
394+
*
395+
* One seam, so a method added later cannot silently omit either. Omitting them
396+
* would not fail a build — it would quietly degrade failure messages, which is
397+
* precisely the bug this helper exists to prevent.
398+
*/
399+
function wasmResult<T>(
400+
operation: () => Promise<T>,
401+
type: EncryptionError['type'],
402+
): Promise<Result<T, EncryptionError>> {
403+
return withResult(operation, toFailure(type), { onException: toError })
404+
}
405+
363406
/**
364407
* Guard the positional contract the bulk methods rely on.
365408
*
@@ -448,7 +491,7 @@ export class WasmEncryptionClient {
448491
plaintext: WasmPlaintext,
449492
opts: EncryptOptions,
450493
): Promise<Result<Encrypted, EncryptionError>> {
451-
return withResult(async () => {
494+
return wasmResult(async () => {
452495
const ffiOpts = {
453496
plaintext,
454497
table: opts.table.tableName,
@@ -460,21 +503,21 @@ export class WasmEncryptionClient {
460503
// biome-ignore lint/plugin: the opts cross the serde boundary, whose shape protect-ffi types as `any`
461504
ffiOpts as never,
462505
)) as Encrypted
463-
}, toFailure(EncryptionErrorTypes.EncryptionError))
506+
}, EncryptionErrorTypes.EncryptionError)
464507
}
465508

466509
async decrypt(
467510
encrypted: Encrypted,
468511
): Promise<Result<WasmPlaintext, EncryptionError>> {
469-
return withResult(
512+
return wasmResult(
470513
async () =>
471514
(await wasmDecrypt(
472515
// biome-ignore lint/plugin: the FFI handle is an opaque wasm-bindgen pointer with no JS-side type
473516
this.client as never,
474517
// biome-ignore lint/plugin: the opts cross the serde boundary, whose shape protect-ffi types as `any`
475518
{ ciphertext: encrypted } as never,
476519
)) as WasmPlaintext,
477-
toFailure(EncryptionErrorTypes.DecryptionError),
520+
EncryptionErrorTypes.DecryptionError,
478521
)
479522
}
480523

@@ -557,15 +600,15 @@ export class WasmEncryptionClient {
557600
plaintext: WasmPlaintext,
558601
opts: WasmEncryptQueryOptions,
559602
): Promise<Result<EncryptedQuery | null, EncryptionError>> {
560-
return withResult(async () => {
603+
return wasmResult(async () => {
561604
if (plaintext === null || plaintext === undefined) return null
562605
return (await wasmEncryptQuery(
563606
// biome-ignore lint/plugin: the FFI handle is an opaque wasm-bindgen pointer with no JS-side type
564607
this.client as never,
565608
// biome-ignore lint/plugin: the term crosses the serde boundary, whose shape protect-ffi types as `any`
566609
toFfiQueryTerm(plaintext, opts) as never,
567610
)) as EncryptedQuery
568-
}, toFailure(EncryptionErrorTypes.EncryptionError))
611+
}, EncryptionErrorTypes.EncryptionError)
569612
}
570613

571614
/**
@@ -598,7 +641,7 @@ export class WasmEncryptionClient {
598641
async encryptQueryBulk(
599642
terms: readonly WasmQueryTerm[],
600643
): Promise<Result<Array<EncryptedQuery | null>, EncryptionError>> {
601-
return withResult(async () => {
644+
return wasmResult(async () => {
602645
const live: Array<{ term: WasmQueryTerm; at: number }> = []
603646
terms.forEach((term, at) => {
604647
if (term.value !== null && term.value !== undefined)
@@ -626,7 +669,7 @@ export class WasmEncryptionClient {
626669
if (slot) out[slot.at] = value
627670
})
628671
return out
629-
}, toFailure(EncryptionErrorTypes.EncryptionError))
672+
}, EncryptionErrorTypes.EncryptionError)
630673
}
631674

632675
/**
@@ -676,7 +719,7 @@ export class WasmEncryptionClient {
676719
async bulkEncrypt(
677720
items: readonly WasmBulkPlaintext[],
678721
): Promise<Result<Array<Encrypted | null>, EncryptionError>> {
679-
return withResult(async () => {
722+
return wasmResult(async () => {
680723
const live: Array<{ item: WasmBulkPlaintext; at: number }> = []
681724
items.forEach((item, at) => {
682725
if (item.plaintext !== null && item.plaintext !== undefined)
@@ -710,7 +753,7 @@ export class WasmEncryptionClient {
710753
if (slot) out[slot.at] = value
711754
})
712755
return out
713-
}, toFailure(EncryptionErrorTypes.EncryptionError))
756+
}, EncryptionErrorTypes.EncryptionError)
714757
}
715758

716759
/**
@@ -753,7 +796,7 @@ export class WasmEncryptionClient {
753796
async bulkDecrypt(
754797
ciphertexts: readonly (Encrypted | null | undefined)[],
755798
): Promise<Result<Array<WasmPlaintext | null>, EncryptionError>> {
756-
return withResult(async () => {
799+
return wasmResult(async () => {
757800
const live: Array<{ ciphertext: Encrypted; at: number }> = []
758801
ciphertexts.forEach((ciphertext, at) => {
759802
if (ciphertext !== null && ciphertext !== undefined)
@@ -795,7 +838,7 @@ export class WasmEncryptionClient {
795838
)
796839
}
797840
return out
798-
}, toFailure(EncryptionErrorTypes.DecryptionError))
841+
}, EncryptionErrorTypes.DecryptionError)
799842
}
800843
}
801844

0 commit comments

Comments
 (0)