Skip to content

Commit 735d293

Browse files
committed
fix(result): short-circuit AsyncResult.all and guard chain against sync throws
- all: iterate sequentially and bail on first Fail; preserves order and avoids waiting on later items - chain: catch synchronous exceptions and convert to Fail; use toPromise() to avoid accessing private state - tests: add coverage for chain throw handling and short-circuit behavior; adjust timing for stability Refs: PR #209 review feedback (@patrickmichalina: "yes please").
1 parent 6529d9e commit 735d293

File tree

2 files changed

+48
-10
lines changed

2 files changed

+48
-10
lines changed

src/result/async-result.spec.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,17 @@ describe(AsyncResult.name, () => {
6161
expect(final.unwrap()).toBe(5)
6262
})
6363

64+
it('chain should convert sync throws in the callback into Fail (no rejection)', async () => {
65+
const boom = (n: number): AsyncResult<number, string> => {
66+
void n
67+
throw 'oops'
68+
}
69+
const ar = AsyncResult.ok<number, string>(1).chain(boom)
70+
const final = await ar.toPromise()
71+
expect(final.isFail()).toBe(true)
72+
expect(final.unwrapFail()).toBe('oops')
73+
})
74+
6475
it('all should collect Ok values and fail on the first failure', async () => {
6576
const a = AsyncResult.ok<number, string>(1)
6677
const b = AsyncResult.fromPromise<number, string>(Promise.resolve(2))
@@ -76,6 +87,31 @@ describe(AsyncResult.name, () => {
7687
expect(allFail.unwrapFail()).toBe('oops')
7788
})
7889

90+
it('all should short-circuit on first Fail (does not wait for later items)', async () => {
91+
let thirdCompleted = false
92+
const a = AsyncResult.ok<number, string>(1)
93+
const b = AsyncResult.fromPromise<number, string>(
94+
new Promise<number>((_, reject) => setTimeout(() => reject('boom'), 10))
95+
)
96+
const c = AsyncResult.fromPromise<number, string>(
97+
new Promise<number>((resolve) => setTimeout(() => {
98+
thirdCompleted = true
99+
resolve(3)
100+
}, 1000))
101+
)
102+
103+
const startedAt = Date.now()
104+
const res = await AsyncResult.all([a, b, c]).toPromise()
105+
const elapsed = Date.now() - startedAt
106+
107+
expect(res.isFail()).toBe(true)
108+
expect(res.unwrapFail()).toBe('boom')
109+
// Should finish well before the third completes (1000ms)
110+
expect(elapsed).toBeLessThan(500)
111+
// And third should not be marked completed yet at the moment of resolution
112+
expect(thirdCompleted).toBe(false)
113+
})
114+
79115
it('fromResult and fromResultPromise should wrap existing Results', async () => {
80116
const syncOk = AsyncResult.fromResult(ok<number, string>(1))
81117
const okFinal = await syncOk.toPromise()
@@ -128,13 +164,13 @@ describe(AsyncResult.name, () => {
128164
})
129165

130166
it('match and matchAsync should resolve with the proper branch', async () => {
131-
const m1 = await AsyncResult.ok<number, string>(2).match({ ok: n => n * 2, fail: _ => -1 })
167+
const m1 = await AsyncResult.ok<number, string>(2).match({ ok: n => n * 2, fail: (e) => { void e; return -1 } })
132168
expect(m1).toBe(4)
133169

134-
const m2 = await AsyncResult.ok<number, string>(3).matchAsync({ ok: async n => n * 3, fail: async _ => -1 })
170+
const m2 = await AsyncResult.ok<number, string>(3).matchAsync({ ok: async n => n * 3, fail: async (e) => { void e; return -1 } })
135171
expect(m2).toBe(9)
136172

137-
const m3 = await AsyncResult.fail<number, string>('x').matchAsync({ ok: async _ => 0, fail: async e => e.length })
173+
const m3 = await AsyncResult.fail<number, string>('x').matchAsync({ ok: async (n) => { void n; return 0 }, fail: async e => e.length })
138174
expect(m3).toBe(1)
139175
})
140176
})

src/result/async-result.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,16 @@ export class AsyncResult<TOk, TFail> {
4343
* collected values preserving order; otherwise returns the first Fail seen.
4444
*/
4545
static all<T, E>(items: ReadonlyArray<AsyncResult<T, E>>): AsyncResult<ReadonlyArray<T>, E> {
46-
const p = (async () => {
46+
const run = async (): Promise<IResult<ReadonlyArray<T>, E>> => {
4747
const acc: T[] = []
4848
for (const ar of items) {
4949
const r = await ar.toPromise()
5050
if (r.isFail()) return Result.fail<ReadonlyArray<T>, E>(r.unwrapFail())
5151
acc.push(r.unwrap())
5252
}
5353
return Result.ok<ReadonlyArray<T>, E>(acc)
54-
})()
55-
return new AsyncResult<ReadonlyArray<T>, E>(p)
54+
}
55+
return new AsyncResult<ReadonlyArray<T>, E>(run())
5656
}
5757

5858
// Core instance methods
@@ -92,11 +92,13 @@ export class AsyncResult<TOk, TFail> {
9292
}
9393

9494
chain<M>(fn: (val: TOk) => AsyncResult<M, TFail>): AsyncResult<M, TFail> {
95-
const p = this.promise.then(r => {
96-
if (r.isOk()) {
97-
return fn(r.unwrap()).promise
95+
const p = this.promise.then(async (r): Promise<IResult<M, TFail>> => {
96+
if (!r.isOk()) return Result.fail<M, TFail>(r.unwrapFail())
97+
try {
98+
return await fn(r.unwrap()).toPromise()
99+
} catch (e) {
100+
return Result.fail<M, TFail>(e as TFail)
98101
}
99-
return Promise.resolve(Result.fail<M, TFail>(r.unwrapFail()))
100102
})
101103
return new AsyncResult<M, TFail>(p)
102104
}

0 commit comments

Comments
 (0)