From 0de7f066e44d8ed14cab962005c9e11fd57a33b5 Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Fri, 17 Jul 2026 11:48:12 +0000 Subject: [PATCH] refactor: use native `Promise.withResolvers` and extract a shared abort-race helper Node.js 22 ships `Promise.withResolvers` natively, so the local polyfill can go away (`lib` is bumped to ES2024 to match). The connect/login methods all repeated the same idiom: create a promise that rejects on signal abort, race pending work against it, and remove the abort listener in a `finally` block. This is now captured in a single `withAbortRace` helper. The helper also marks the abort rejection as handled, closing a small unhandled-rejection window when a signal aborts while no race is pending. Co-Authored-By: Claude Fable 5 --- src/connection.ts | 114 +++++++++++++++------------------------------- tsconfig.json | 2 +- 2 files changed, 37 insertions(+), 79 deletions(-) diff --git a/src/connection.ts b/src/connection.ts index b6bc8a72f..0ff2f90e3 100644 --- a/src/connection.ts +++ b/src/connection.ts @@ -877,20 +877,32 @@ interface RoutingData { } /** - * Helper function, equivalent to `Promise.withResolvers()`. + * Runs the given function, providing it with a promise that will reject + * with the given signal's abort reason once the signal is aborted. * - * @returns An object with the properties `promise`, `resolve`, and `reject`. + * The function can race this promise against any asynchronous work it + * performs (potentially multiple times) to make itself abortable. + * + * The rejection is always considered handled, and the signal's abort + * listener is removed once the function settles. */ -function withResolvers() { - let resolve: (value: T | PromiseLike) => void; - let reject: (reason?: any) => void; +async function withAbortRace(signal: AbortSignal, func: (signalAborted: Promise) => Promise): Promise { + signal.throwIfAborted(); + + const { promise: signalAborted, reject } = Promise.withResolvers(); - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); + // Prevent unhandled rejections if the signal is aborted while + // `func` is not currently racing against `signalAborted`. + signalAborted.catch(() => {}); - return { promise, resolve: resolve!, reject: reject! }; + const onAbort = () => { reject(signal.reason); }; + signal.addEventListener('abort', onAbort, { once: true }); + + try { + return await func(signalAborted); + } finally { + signal.removeEventListener('abort', onAbort); + } } /** @@ -2216,7 +2228,7 @@ class Connection extends EventEmitter { servername: this.config.options.serverName ? this.config.options.serverName : serverName, }; - const { promise, resolve, reject } = withResolvers(); + const { promise, resolve, reject } = Promise.withResolvers(); const encryptsocket = tls.connect(encryptOptions); try { @@ -3333,14 +3345,7 @@ class Connection extends EventEmitter { * @private */ async performTlsNegotiation(preloginPayload: PreloginPayload, signal: AbortSignal) { - signal.throwIfAborted(); - - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + await withAbortRace(signal, async (signalAborted) => { if (preloginPayload.fedAuthRequired === 1) { this.fedAuthRequired = true; } @@ -3357,22 +3362,13 @@ class Connection extends EventEmitter { signalAborted ]); } - } finally { - signal.removeEventListener('abort', onAbort); - } + }); } async readPreloginResponse(signal: AbortSignal): Promise { - signal.throwIfAborted(); - let messageBuffer = Buffer.alloc(0); - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + await withAbortRace(signal, async (signalAborted) => { const message = await Promise.race([ this.messageIo.readMessage().catch((err) => { throw this.wrapSocketError(err); @@ -3399,9 +3395,7 @@ class Connection extends EventEmitter { await iterator.return(); } } - } finally { - signal.removeEventListener('abort', onAbort); - } + }); const preloginPayload = new PreloginPayload(messageBuffer); this.debug.payload(function() { @@ -3449,7 +3443,7 @@ class Connection extends EventEmitter { const closeSignal = this.closeController!.signal; closeSignal.throwIfAborted(); - const { promise, resolve, reject } = withResolvers(); + const { promise, resolve, reject } = Promise.withResolvers(); const onAbort = () => { reject(closeSignal.reason); }; closeSignal.addEventListener('abort', onAbort, { once: true }); @@ -3471,14 +3465,7 @@ class Connection extends EventEmitter { * @private */ async performSentLogin7WithStandardLogin(signal: AbortSignal): Promise { - signal.throwIfAborted(); - - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + return await withAbortRace(signal, async (signalAborted) => { const message = await Promise.race([ this.messageIo.readMessage().catch((err) => { throw this.wrapSocketError(err); @@ -3500,23 +3487,14 @@ class Connection extends EventEmitter { } else { throw new ConnectionError('Login failed.', 'ELOGIN'); } - } finally { - signal.removeEventListener('abort', onAbort); - } + }); } /** * @private */ async performSentLogin7WithNTLMLogin(signal: AbortSignal): Promise { - signal.throwIfAborted(); - - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + return await withAbortRace(signal, async (signalAborted) => { while (true) { const message = await Promise.race([ this.messageIo.readMessage().catch((err) => { @@ -3556,23 +3534,14 @@ class Connection extends EventEmitter { throw new ConnectionError('Login failed.', 'ELOGIN'); } } - } finally { - signal.removeEventListener('abort', onAbort); - } + }); } /** * @private */ async performSentLogin7WithFedAuth(signal: AbortSignal): Promise { - signal.throwIfAborted(); - - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + return await withAbortRace(signal, async (signalAborted) => { const message = await Promise.race([ this.messageIo.readMessage().catch((err) => { throw this.wrapSocketError(err); @@ -3662,23 +3631,14 @@ class Connection extends EventEmitter { } else { throw new ConnectionError('Login failed.', 'ELOGIN'); } - } finally { - signal.removeEventListener('abort', onAbort); - } + }); } /** * @private */ async performLoggedInSendingInitialSql(signal: AbortSignal) { - signal.throwIfAborted(); - - const { promise: signalAborted, reject } = withResolvers(); - - const onAbort = () => { reject(signal.reason); }; - signal.addEventListener('abort', onAbort, { once: true }); - - try { + await withAbortRace(signal, async (signalAborted) => { this.sendInitialSql(); const message = await Promise.race([ @@ -3693,9 +3653,7 @@ class Connection extends EventEmitter { once(tokenStreamParser, 'end'), signalAborted ]); - } finally { - signal.removeEventListener('abort', onAbort); - } + }); } } diff --git a/tsconfig.json b/tsconfig.json index 6c3d07bd5..76863de0c 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,7 @@ "strict": true, "esModuleInterop": true, "allowSyntheticDefaultImports": true, - "lib": [ "ES2022" ], + "lib": [ "ES2024" ], "skipLibCheck": true, "resolveJsonModule": true, "exactOptionalPropertyTypes": true,