Skip to content

Commit

Permalink
refactor!: cleanup AbortSignal handling (#1654)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: tedious now requires Node.js version 18.17 or later.
  • Loading branch information
arthurschreiber authored Aug 22, 2024
1 parent 5463652 commit 669ab44
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 87 deletions.
5 changes: 3 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"url": "https://github.com/tediousjs/tedious.git"
},
"engines": {
"node": ">=18"
"node": ">=18.17"
},
"publishConfig": {
"tag": "next",
Expand Down
24 changes: 8 additions & 16 deletions src/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@ import net from 'net';
import dns, { type LookupAddress } from 'dns';

import url from 'node:url';
import AbortError from './errors/abort-error';

type LookupFunction = (hostname: string, options: dns.LookupAllOptions, callback: (err: NodeJS.ErrnoException | null, addresses: dns.LookupAddress[]) => void) => void;

export async function connectInParallel(options: { host: string, port: number, localAddress?: string | undefined }, lookup: LookupFunction, signal: AbortSignal) {
if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

const addresses = await lookupAllAddresses(options.host, lookup, signal);

Expand Down Expand Up @@ -61,7 +58,7 @@ export async function connectInParallel(options: { host: string, port: number, l
socket.destroy();
}

reject(new AbortError());
reject(signal.reason);
};

for (let i = 0, len = addresses.length; i < len; i++) {
Expand All @@ -80,9 +77,7 @@ export async function connectInParallel(options: { host: string, port: number, l
}

export async function connectInSequence(options: { host: string, port: number, localAddress?: string | undefined }, lookup: LookupFunction, signal: AbortSignal) {
if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

const errors: any[] = [];
const addresses = await lookupAllAddresses(options.host, lookup, signal);
Expand All @@ -102,7 +97,7 @@ export async function connectInSequence(options: { host: string, port: number, l

socket.destroy();

reject(new AbortError());
reject(signal.reason);
};

const onError = (err: Error) => {
Expand Down Expand Up @@ -131,9 +126,8 @@ export async function connectInSequence(options: { host: string, port: number, l
socket.on('connect', onConnect);
});
} catch (err) {
if (err instanceof Error && err.name === 'AbortError') {
throw err;
}
// If the signal was aborted, re-throw the error.
signal.throwIfAborted();

errors.push(err);

Expand All @@ -148,9 +142,7 @@ export async function connectInSequence(options: { host: string, port: number, l
* Look up all addresses for the given hostname.
*/
export async function lookupAllAddresses(host: string, lookup: LookupFunction, signal: AbortSignal): Promise<dns.LookupAddress[]> {
if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

if (net.isIPv6(host)) {
return [{ address: host, family: 6 }];
Expand All @@ -159,7 +151,7 @@ export async function lookupAllAddresses(host: string, lookup: LookupFunction, s
} else {
return await new Promise<LookupAddress[]>((resolve, reject) => {
const onAbort = () => {
reject(new AbortError());
reject(signal.reason);
};

signal.addEventListener('abort', onAbort);
Expand Down
10 changes: 0 additions & 10 deletions src/errors/abort-error.ts

This file was deleted.

10 changes: 0 additions & 10 deletions src/errors/timeout-error.ts

This file was deleted.

17 changes: 7 additions & 10 deletions src/instance-lookup.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import dns from 'dns';

import AbortError from './errors/abort-error';
import { sendMessage } from './sender';
import { withTimeout } from './utils/with-timeout';

const SQL_SERVER_BROWSER_PORT = 1434;
const TIMEOUT = 2 * 1000;
Expand Down Expand Up @@ -46,21 +44,20 @@ export async function instanceLookup(options: { server: string, instanceName: st

const signal = options.signal;

if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

let response;

const request = Buffer.from([0x02]);

for (let i = 0; i <= retries; i++) {
const timeoutSignal = AbortSignal.timeout(timeout);

try {
response = await withTimeout(timeout, async (signal) => {
const request = Buffer.from([0x02]);
return await sendMessage(options.server, port, lookup, signal, request);
}, signal);
response = await sendMessage(options.server, port, lookup, AbortSignal.any([ signal, timeoutSignal ]), request);
} catch (err) {
// If the current attempt timed out, continue with the next
if (!signal.aborted && err instanceof Error && err.name === 'TimeoutError') {
if (timeoutSignal.aborted) {
continue;
}

Expand Down
14 changes: 4 additions & 10 deletions src/sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ import dns from 'dns';
import net from 'net';
import url from 'node:url';

import AbortError from './errors/abort-error';

type LookupFunction = (hostname: string, options: dns.LookupAllOptions, callback: (err: NodeJS.ErrnoException | null, addresses: dns.LookupAddress[]) => void) => void;

export async function sendInParallel(addresses: dns.LookupAddress[], port: number, request: Buffer, signal: AbortSignal) {
if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

return await new Promise<Buffer>((resolve, reject) => {
const sockets: dgram.Socket[] = [];
Expand Down Expand Up @@ -38,7 +34,7 @@ export async function sendInParallel(addresses: dns.LookupAddress[], port: numbe
const onAbort = () => {
clearSockets();

reject(new AbortError());
reject(signal.reason);
};

const clearSockets = () => {
Expand All @@ -64,9 +60,7 @@ export async function sendInParallel(addresses: dns.LookupAddress[], port: numbe
}

export async function sendMessage(host: string, port: number, lookup: LookupFunction, signal: AbortSignal, request: Buffer) {
if (signal.aborted) {
throw new AbortError();
}
signal.throwIfAborted();

let addresses: dns.LookupAddress[];

Expand All @@ -77,7 +71,7 @@ export async function sendMessage(host: string, port: number, lookup: LookupFunc
} else {
addresses = await new Promise<dns.LookupAddress[]>((resolve, reject) => {
const onAbort = () => {
reject(new AbortError());
reject(signal.reason);
};

const domainInASCII = url.domainToASCII(host);
Expand Down
28 changes: 0 additions & 28 deletions src/utils/with-timeout.ts

This file was deleted.

0 comments on commit 669ab44

Please sign in to comment.