Skip to content

Commit e01a6ca

Browse files
polRkpolRk
committed
Simplify abortable implementation and use native AbortSignal methods
- Replace custom DOMException with native `signal.throwIfAborted()` - Reject with signal reason instead of creating new exceptions - Add `linkSignals()` utility for managing multiple abort signals - Update tests to expect actual abort reasons Signed-off-by: Vladislav Polyakov <polRk@ydb.tech>
1 parent 270cc7d commit e01a6ca

File tree

6 files changed

+122
-34
lines changed

6 files changed

+122
-34
lines changed

‎.changeset/abortable-simplify.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@ydbjs/abortable': minor
3+
---
4+
5+
Add linkSignals utility and simplify abortable implementation
6+
7+
- Add new `linkSignals` function to combine multiple AbortSignal instances
8+
- Simplify abortable implementation to use native AbortSignal API
9+
- Improve type safety and reduce code complexity
10+
- Add comprehensive tests for signal linking and cleanup

‎packages/abortable/src/index.test.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ test('throws immediately when signal already aborted', async () => {
2828
let promise = Promise.resolve('success')
2929
let result = abortable(controller.signal, promise)
3030

31-
await expect(result).rejects.toThrow('AbortError')
31+
await expect(result).rejects.toThrow('aborted')
3232
})
3333

3434
test('throws immediately when signal aborted with reason', async () => {
@@ -39,7 +39,7 @@ test('throws immediately when signal aborted with reason', async () => {
3939
let promise = Promise.resolve('success')
4040
let result = abortable(controller.signal, promise)
4141

42-
await expect(result).rejects.toThrow('AbortError')
42+
await expect(result).rejects.toThrow('Custom abort reason')
4343
})
4444

4545
test('aborts when signal aborted during promise execution', async () => {
@@ -53,7 +53,7 @@ test('aborts when signal aborted during promise execution', async () => {
5353
// Abort after a short delay
5454
setTimeout(() => controller.abort(), 10)
5555

56-
await expect(result).rejects.toThrow('AbortError')
56+
await expect(result).rejects.toThrow('aborted')
5757
})
5858

5959
test('resolves when promise completes before abort signal', async () => {
@@ -87,7 +87,7 @@ test('handles multiple abort listeners correctly', async () => {
8787

8888
setTimeout(() => controller.abort(), 10)
8989

90-
await expect(result).rejects.toThrow('AbortError')
90+
await expect(result).rejects.toThrow('aborted')
9191
expect(listenerCalled).eq(true)
9292
})
9393

@@ -113,7 +113,7 @@ test('handles promise that never resolves with abort', async () => {
113113
// Immediately abort
114114
controller.abort()
115115

116-
await expect(result).rejects.toThrow('AbortError')
116+
await expect(result).rejects.toThrow('aborted')
117117
})
118118

119119
test('handles promise rejection after abort signal', async () => {
@@ -127,11 +127,11 @@ test('handles promise rejection after abort signal', async () => {
127127
// Abort immediately
128128
controller.abort()
129129

130-
// Should reject with AbortError, not the promise error
131-
await expect(result).rejects.toThrow('AbortError')
130+
// Should reject with aborted, not the promise error
131+
await expect(result).rejects.toThrow('aborted')
132132
})
133133

134-
test('creates AbortError when signal aborted with reason', async () => {
134+
test('creates aborted when signal aborted with reason', async () => {
135135
let controller = new AbortController()
136136
let customReason = new Error('Custom cancellation')
137137

@@ -143,7 +143,7 @@ test('creates AbortError when signal aborted with reason', async () => {
143143

144144
setTimeout(() => controller.abort(customReason), 10)
145145

146-
await expect(result).rejects.toThrow('AbortError')
146+
await expect(result).rejects.toThrow('Custom cancellation')
147147
})
148148

149149
test('handles concurrent abortable calls with same signal', async () => {
@@ -158,8 +158,8 @@ test('handles concurrent abortable calls with same signal', async () => {
158158
// Abort after starting both
159159
setTimeout(() => controller.abort(), 10)
160160

161-
await expect(result1).rejects.toThrow('AbortError')
162-
await expect(result2).rejects.toThrow('AbortError')
161+
await expect(result1).rejects.toThrow('aborted')
162+
await expect(result2).rejects.toThrow('aborted')
163163
})
164164

165165
test('works with different promise types', async () => {
@@ -191,7 +191,7 @@ test('handles abort without reason', async () => {
191191

192192
setTimeout(() => controller.abort(), 10)
193193

194-
await expect(result).rejects.toThrow('AbortError')
194+
await expect(result).rejects.toThrow('aborted')
195195
})
196196

197197
test('handles promise that resolves to undefined', async () => {
@@ -223,7 +223,7 @@ test('handles extremely fast abort timing', async () => {
223223
// Abort immediately without delay
224224
controller.abort()
225225

226-
await expect(result).rejects.toThrow('AbortError')
226+
await expect(result).rejects.toThrow('aborted')
227227
})
228228

229229
test('handles abort signal from different controller', async () => {
@@ -240,6 +240,6 @@ test('handles abort signal from different controller', async () => {
240240
// Only abort controller1
241241
setTimeout(() => controller1.abort(), 10)
242242

243-
await expect(result1).rejects.toThrow('AbortError')
243+
await expect(result1).rejects.toThrow('aborted')
244244
await expect(result2).resolves.eq('success')
245245
})

‎packages/abortable/src/index.ts‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,15 @@
11
export async function abortable<T>(signal: AbortSignal, promise: Promise<T>): Promise<T> {
2-
if (signal.aborted) {
3-
throw new DOMException('AbortError', {
4-
name: 'AbortError',
5-
cause: signal.reason,
6-
})
7-
}
2+
signal.throwIfAborted()
83

94
let abortHandler: () => void
10-
11-
return Promise.race<T>([
12-
promise,
13-
new Promise((_, reject) => {
14-
let reason = new DOMException('AbortError', {
15-
name: 'AbortError',
16-
cause: signal.reason,
17-
})
18-
abortHandler = () => reject(reason)
19-
signal.addEventListener('abort', abortHandler, { once: true })
20-
}),
21-
]).finally(() => {
22-
signal.removeEventListener('abort', abortHandler)
5+
let abortPromise = new Promise<T>((_, reject) => {
6+
abortHandler = () => reject(signal.reason)
7+
signal.addEventListener('abort', abortHandler, { once: true })
238
})
9+
10+
try {
11+
return await Promise.race<T>([promise, abortPromise])
12+
} finally {
13+
if (abortHandler!) signal.removeEventListener('abort', abortHandler)
14+
}
2415
}

‎packages/abortable/src/signals.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { getEventListeners } from 'node:events'
2+
import { expect, test } from 'vitest'
3+
import { linkSignals } from './signals.ts'
4+
5+
test('cleans up listeners after using block', () => {
6+
let root = new AbortController().signal
7+
8+
for (let i = 0; i < 10_000; i++) {
9+
using _ = linkSignals(root)
10+
}
11+
12+
expect(getEventListeners(root, 'abort').length).toBe(0)
13+
})
14+
15+
test('aborts immediately when parent already aborted', () => {
16+
let root = new AbortController()
17+
root.abort('dead')
18+
19+
using linked = linkSignals(root.signal)
20+
21+
expect(linked.signal.aborted).toBe(true)
22+
expect(linked.signal.reason).toBe('dead')
23+
})
24+
25+
test('handles multiple signals and undefined', () => {
26+
let sig1 = new AbortController().signal
27+
let sig2 = new AbortController().signal
28+
29+
{
30+
using _ = linkSignals(sig1, sig2, undefined)
31+
expect(getEventListeners(sig1, 'abort').length).toBe(1)
32+
expect(getEventListeners(sig2, 'abort').length).toBe(1)
33+
}
34+
35+
expect(getEventListeners(sig1, 'abort').length).toBe(0)
36+
expect(getEventListeners(sig2, 'abort').length).toBe(0)
37+
})
38+
39+
test('aborts linked signal on dispose', () => {
40+
let signal: AbortSignal
41+
42+
{
43+
using linked = linkSignals(new AbortController().signal)
44+
signal = linked.signal
45+
expect(signal.aborted).toBe(false)
46+
}
47+
48+
expect(signal.aborted).toBe(true)
49+
})

‎packages/abortable/src/signals.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
export interface LinkedSignal extends Disposable {
2+
signal: AbortSignal
3+
}
4+
5+
export function linkSignals(...signals: (AbortSignal | undefined)[]): LinkedSignal {
6+
let ac = new AbortController()
7+
let activeSignals = signals.filter((s): s is AbortSignal => !!s)
8+
9+
for (let signal of activeSignals) {
10+
if (signal.aborted) {
11+
ac.abort(signal.reason)
12+
13+
return {
14+
signal: ac.signal,
15+
[Symbol.dispose]: () => {},
16+
}
17+
}
18+
}
19+
20+
let onAbort = (e: Event) => ac.abort((e.target as AbortSignal).reason)
21+
22+
for (let signal of activeSignals) {
23+
signal.addEventListener('abort', onAbort, { once: true })
24+
}
25+
26+
return {
27+
signal: ac.signal,
28+
[Symbol.dispose]() {
29+
for (let signal of activeSignals) {
30+
signal.removeEventListener('abort', onAbort)
31+
}
32+
33+
if (!ac.signal.aborted) {
34+
ac.abort()
35+
}
36+
},
37+
}
38+
}

‎packages/auth/tests/static.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ test('respects abort signal', async () => {
189189
let controller = new AbortController()
190190
setTimeout(() => controller.abort(), 1)
191191

192-
await expect(provider.getToken(false, controller.signal)).rejects.toThrow('AbortError')
192+
await expect(provider.getToken(false, controller.signal)).rejects.toThrow('aborted')
193193
})
194194

195195
test('handles invalid credentials gracefully', async () => {

0 commit comments

Comments
 (0)