Skip to content

Commit a1e5e2d

Browse files
clutz-bot[bot]clutz-bot[bot]
committed
Fix promise hang when AbortSignal is aborted
When an AbortSignal was aborted, the promise returned by once() would remain permanently pending, causing any await usage to hang indefinitely. Now the promise rejects with an AbortError when the signal is aborted, including the case where the signal is already aborted at call time. Also sets up changesets for the 2.x release line with legacy dist-tag.
1 parent b71b6e8 commit a1e5e2d

8 files changed

Lines changed: 144 additions & 9 deletions

File tree

‎.changeset/README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Changesets
2+
3+
Hello and welcome! This folder has been automatically generated by `@changesets/cli`, a build tool that works
4+
with multi-package repos, or single-package repos to help you version and publish your code. You can
5+
find the full documentation for it [in our repository](https://github.com/changesets/changesets)
6+
7+
We have a quick list of common questions to get you started engaging with this project in
8+
[our documentation](https://github.com/changesets/changesets/blob/main/docs/common-questions.md)

‎.changeset/config.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"$schema": "https://unpkg.com/@changesets/config@3.1.1/schema.json",
3+
"changelog": "@changesets/cli/changelog",
4+
"commit": false,
5+
"fixed": [],
6+
"linked": [],
7+
"access": "public",
8+
"baseBranch": "2.x",
9+
"updateInternalDependencies": "patch",
10+
"ignore": []
11+
}

‎.changeset/fix-abort-signal.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tootallnate/once": patch
3+
---
4+
5+
Fix promise hang when AbortSignal is aborted

‎.github/workflows/release.yml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: Release
2+
3+
on:
4+
push:
5+
branches:
6+
- 2.x
7+
8+
concurrency: ${{ github.workflow }}-${{ github.ref }}
9+
10+
jobs:
11+
release:
12+
name: Release
13+
runs-on: ubuntu-latest
14+
steps:
15+
- name: Checkout Repo
16+
uses: actions/checkout@v4
17+
18+
- name: Setup Node.js
19+
uses: actions/setup-node@v4
20+
with:
21+
node-version: 18
22+
23+
- name: Install Dependencies
24+
run: npm install
25+
26+
- name: Create Release Pull Request or Publish
27+
id: changesets
28+
uses: changesets/action@v1
29+
with:
30+
publish: npm run release
31+
title: "Version Packages (2.x)"
32+
commit: "Version Packages (2.x)"
33+
env:
34+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
35+
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

‎package.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111
"prebuild": "rimraf dist",
1212
"build": "tsc",
1313
"test": "jest",
14-
"prepublishOnly": "npm run build"
14+
"prepublishOnly": "npm run build",
15+
"changeset": "changeset",
16+
"version": "changeset version",
17+
"release": "npm run build && changeset publish --tag legacy"
1518
},
1619
"repository": {
1720
"type": "git",
@@ -24,6 +27,7 @@
2427
"url": "https://github.com/TooTallNate/once/issues"
2528
},
2629
"devDependencies": {
30+
"@changesets/cli": "^2.29.4",
2731
"@types/jest": "^27.0.2",
2832
"@types/node": "^12.12.11",
2933
"abort-controller": "^3.0.0",

‎src/index.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export default function once<
1515
): Promise<EventListenerParameters<Emitter, Event>> {
1616
return new Promise((resolve, reject) => {
1717
function cleanup() {
18-
signal?.removeEventListener('abort', cleanup);
18+
signal?.removeEventListener('abort', onAbort);
1919
emitter.removeListener(name, onEvent);
2020
emitter.removeListener('error', onError);
2121
}
@@ -27,7 +27,17 @@ export default function once<
2727
cleanup();
2828
reject(err);
2929
}
30-
signal?.addEventListener('abort', cleanup);
30+
function onAbort() {
31+
cleanup();
32+
const err = new Error('The operation was aborted');
33+
err.name = 'AbortError';
34+
reject(err);
35+
}
36+
if (signal?.aborted) {
37+
onAbort();
38+
return;
39+
}
40+
signal?.addEventListener('abort', onAbort);
3141
emitter.on(name, onEvent);
3242
emitter.on('error', onError);
3343
});

‎src/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export type EventListenerParameters<
3030
export type WithDefault<T, D> = [T] extends [never] ? D : T;
3131

3232
export interface AbortSignal {
33+
aborted: boolean;
3334
addEventListener: (name: string, listener: (...args: any[]) => any) => void;
3435
removeEventListener: (
3536
name: string,

‎test/once.test.ts‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,19 +99,80 @@ describe('once()', () => {
9999
await new Promise((r) => process.nextTick(r));
100100

101101
expect(wasResolved).toEqual(true);
102+
});
103+
104+
it('should reject with AbortError when signal is aborted', async () => {
105+
const emitter = new EventEmitter();
106+
const controller = new AbortController();
107+
const { signal } = controller;
102108

103-
// Reset
104-
wasResolved = false;
109+
const promise = once(emitter, 'foo', { signal });
105110

106-
once(emitter, 'foo', { signal }).then(onResolve, onResolve);
111+
controller.abort();
112+
113+
try {
114+
await promise;
115+
throw new Error('Should not happen');
116+
} catch (err: any) {
117+
expect(err.name).toEqual('AbortError');
118+
expect(err.message).toEqual('The operation was aborted');
119+
}
120+
});
107121

108-
// This time abort
122+
it('should reject immediately if signal is already aborted', async () => {
123+
const emitter = new EventEmitter();
124+
const controller = new AbortController();
125+
controller.abort();
126+
127+
const { signal } = controller;
128+
129+
try {
130+
await once(emitter, 'foo', { signal });
131+
throw new Error('Should not happen');
132+
} catch (err: any) {
133+
expect(err.name).toEqual('AbortError');
134+
expect(err.message).toEqual('The operation was aborted');
135+
}
136+
});
137+
138+
it('should not resolve with event after being aborted', async () => {
139+
const emitter = new EventEmitter();
140+
const controller = new AbortController();
141+
const { signal } = controller;
142+
143+
let wasResolved = false;
144+
let wasRejected = false;
145+
146+
const promise = once(emitter, 'foo', { signal }).then(
147+
() => { wasResolved = true; },
148+
() => { wasRejected = true; }
149+
);
150+
151+
// Abort first, then emit
109152
controller.abort();
110153
emitter.emit('foo');
111154

112-
// Promise is fulfilled on next tick, so wait a bit
113-
await new Promise((r) => process.nextTick(r));
155+
await promise;
114156

115157
expect(wasResolved).toEqual(false);
158+
expect(wasRejected).toEqual(true);
159+
});
160+
161+
it('should clean up listeners on the emitter when aborted', async () => {
162+
const emitter = new EventEmitter();
163+
const controller = new AbortController();
164+
const { signal } = controller;
165+
166+
once(emitter, 'foo', { signal }).catch(() => {});
167+
168+
expect(emitter.listenerCount('foo')).toEqual(1);
169+
expect(emitter.listenerCount('error')).toEqual(1);
170+
171+
controller.abort();
172+
173+
await new Promise((r) => process.nextTick(r));
174+
175+
expect(emitter.listenerCount('foo')).toEqual(0);
176+
expect(emitter.listenerCount('error')).toEqual(0);
116177
});
117178
});

0 commit comments

Comments
 (0)