Skip to content

Commit e96119a

Browse files
committed
Address comments
1 parent 1cf7f1f commit e96119a

File tree

3 files changed

+32
-14
lines changed

3 files changed

+32
-14
lines changed

.github/workflows/linux-arm64-build-and-test.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,3 @@ jobs:
6060
uname -a
6161
source /opt/ros/${{ matrix.ros_distribution }}/setup.bash
6262
npm i
63-
npm run lint
64-
npm test
65-
npm run clean

lib/client.js

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,45 @@ const DistroUtils = require('./distro.js');
1919
const Entity = require('./entity.js');
2020
const debug = require('debug')('rclnodejs:client');
2121

22-
// Polyfill for AbortSignal.any() for Node.js <= 18.20
22+
// Polyfill for AbortSignal.any() for Node.js <= 20.3.0
2323
// AbortSignal.any() was added in Node.js 20.3.0
2424
// See https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static
2525
if (!AbortSignal.any) {
2626
AbortSignal.any = function (signals) {
27+
// Filter out null/undefined values and validate inputs
28+
const validSignals = Array.isArray(signals)
29+
? signals.filter((signal) => signal != null)
30+
: [];
31+
32+
// If no valid signals, return a never-aborting signal
33+
if (validSignals.length === 0) {
34+
return new AbortController().signal;
35+
}
36+
2737
const controller = new AbortController();
38+
const listeners = [];
39+
40+
// Cleanup function to remove all event listeners
41+
const cleanup = () => {
42+
listeners.forEach(({ signal, listener }) => {
43+
signal.removeEventListener('abort', listener);
44+
});
45+
};
2846

29-
for (const signal of signals) {
47+
for (const signal of validSignals) {
3048
if (signal.aborted) {
49+
cleanup();
3150
controller.abort(signal.reason);
32-
break;
51+
return controller.signal;
3352
}
3453

35-
signal.addEventListener(
36-
'abort',
37-
() => {
38-
controller.abort(signal.reason);
39-
},
40-
{ once: true }
41-
);
54+
const listener = () => {
55+
cleanup();
56+
controller.abort(signal.reason);
57+
};
58+
59+
signal.addEventListener('abort', listener);
60+
listeners.push({ signal, listener });
4261
}
4362

4463
return controller.signal;

test/test-async-client.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,9 @@ describe('Client async functionality', function () {
260260
});
261261

262262
it('should handle zero and negative timeouts', async function () {
263-
// Skip this test on Node.js < 18.20 where AbortSignal.timeout(0) throws RangeError
263+
// Skip this test on Node.js < 18.20.0: AbortSignal.timeout() was added in 17.3.0,
264+
// but support for zero timeout values was only fixed in
265+
// 18.20.0 (prior versions throw RangeError for AbortSignal.timeout(0))
264266
const [major, minor] = process.versions.node.split('.').map(Number);
265267
if (major < 18 || (major === 18 && minor < 20)) {
266268
this.skip();

0 commit comments

Comments
 (0)