From d6c0af35fa52c8b4ebe820f8276b6177bea9fc67 Mon Sep 17 00:00:00 2001 From: Philipp Kunz Date: Mon, 28 Apr 2025 15:30:08 +0000 Subject: [PATCH] fix(core): Improve logging and error handling by introducing custom error classes and a global logging interface while refactoring network diagnostics methods. --- changelog.md | 8 +++ ts/00_commitinfo_data.ts | 2 +- ts/errors.ts | 20 ++++++ ts/logging.ts | 30 ++++++++ ts/smartnetwork.classes.cloudflarespeed.ts | 84 +++++++++++----------- ts/smartnetwork.classes.smartnetwork.ts | 32 ++++----- 6 files changed, 115 insertions(+), 61 deletions(-) create mode 100644 ts/errors.ts create mode 100644 ts/logging.ts diff --git a/changelog.md b/changelog.md index 41d0587..6291ea5 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,13 @@ # Changelog +## 2025-04-28 - 3.0.5 - fix(core) +Improve logging and error handling by introducing custom error classes and a global logging interface while refactoring network diagnostics methods. + +- Added custom error classes (NetworkError, TimeoutError) for network operations. +- Introduced a global logging interface to replace direct console logging. +- Updated CloudflareSpeed and SmartNetwork classes to use getLogger for improved error reporting. +- Disabled connection pooling in HTTP requests to prevent listener accumulation. + ## 2025-04-28 - 3.0.4 - fix(ci/config) Improve CI workflows, update project configuration, and clean up code formatting diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 4401ddb..dcb9ecf 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@push.rocks/smartnetwork', - version: '3.0.4', + version: '3.0.5', description: 'A toolkit for network diagnostics including speed tests, port availability checks, and more.' } diff --git a/ts/errors.ts b/ts/errors.ts new file mode 100644 index 0000000..27e08e0 --- /dev/null +++ b/ts/errors.ts @@ -0,0 +1,20 @@ +/** + * Custom error classes for network operations + */ +export class NetworkError extends Error { + public code?: string; + constructor(message?: string, code?: string) { + super(message); + this.name = 'NetworkError'; + this.code = code; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +export class TimeoutError extends NetworkError { + constructor(message?: string) { + super(message, 'ETIMEOUT'); + this.name = 'TimeoutError'; + Object.setPrototypeOf(this, new.target.prototype); + } +} \ No newline at end of file diff --git a/ts/logging.ts b/ts/logging.ts new file mode 100644 index 0000000..a0ad8c0 --- /dev/null +++ b/ts/logging.ts @@ -0,0 +1,30 @@ +/** + * Injectable logging interface and global logger + */ +export interface Logger { + /** Debug-level messages */ + debug?(...args: unknown[]): void; + /** Informational messages */ + info(...args: unknown[]): void; + /** Warning messages */ + warn?(...args: unknown[]): void; + /** Error messages */ + error(...args: unknown[]): void; +} + +let globalLogger: Logger = console; + +/** + * Replace the global logger implementation + * @param logger Custom logger adhering to Logger interface + */ +export function setLogger(logger: Logger): void { + globalLogger = logger; +} + +/** + * Retrieve the current global logger + */ +export function getLogger(): Logger { + return globalLogger; +} \ No newline at end of file diff --git a/ts/smartnetwork.classes.cloudflarespeed.ts b/ts/smartnetwork.classes.cloudflarespeed.ts index f7ecf94..0cde8fd 100644 --- a/ts/smartnetwork.classes.cloudflarespeed.ts +++ b/ts/smartnetwork.classes.cloudflarespeed.ts @@ -1,4 +1,6 @@ import * as plugins from './smartnetwork.plugins.js'; +import { getLogger } from './logging.js'; +import { NetworkError, TimeoutError } from './errors.js'; import * as stats from './helpers/stats.js'; export class CloudflareSpeed { @@ -49,7 +51,7 @@ export class CloudflareSpeed { measurements.push(response[4] - response[0] - response[6]); }, (error) => { - console.log(`Error: ${error}`); + getLogger().error('Error measuring latency:', error); }, ); } @@ -73,7 +75,7 @@ export class CloudflareSpeed { measurements.push(await this.measureSpeed(bytes, transferTime)); }, (error) => { - console.log(`Error: ${error}`); + getLogger().error('Error measuring download chunk:', error); }, ); } @@ -91,7 +93,7 @@ export class CloudflareSpeed { measurements.push(await this.measureSpeed(bytes, transferTime)); }, (error) => { - console.log(`Error: ${error}`); + getLogger().error('Error measuring upload chunk:', error); }, ); } @@ -104,15 +106,16 @@ export class CloudflareSpeed { } public async fetchServerLocations(): Promise<{ [key: string]: string }> { - const res = JSON.parse(await this.get('speed.cloudflare.com', '/locations')); - - return res.reduce((data: any, optionsArg: { iata: string; city: string }) => { - // Bypass prettier "no-assign-param" rules - const data1 = data; - - data1[optionsArg.iata] = optionsArg.city; - return data1; - }, {}); + const res = JSON.parse( + await this.get('speed.cloudflare.com', '/locations'), + ) as Array<{ iata: string; city: string }>; + return res.reduce( + (data: Record, optionsArg) => { + data[optionsArg.iata] = optionsArg.city; + return data; + }, + {} as Record, + ); } public async get(hostname: string, path: string): Promise { @@ -122,6 +125,8 @@ export class CloudflareSpeed { hostname, path, method: 'GET', + // disable connection pooling to avoid listener accumulation + agent: false, }, (res) => { const body: Array = []; @@ -135,9 +140,9 @@ export class CloudflareSpeed { reject(e); } }); - req.on('error', (err) => { - reject(err); - }); + req.on('error', (err: Error & { code?: string }) => { + reject(new NetworkError(err.message, err.code)); + }); }, ); @@ -179,33 +184,36 @@ export class CloudflareSpeed { return new Promise((resolve, reject) => { started = plugins.perfHooks.performance.now(); - const req = plugins.https.request(options, (res) => { + // disable connection pooling to avoid listener accumulation across requests + const reqOptions = { ...options, agent: false }; + const req = plugins.https.request(reqOptions, (res) => { res.once('readable', () => { ttfb = plugins.perfHooks.performance.now(); }); res.on('data', () => {}); res.on('end', () => { ended = plugins.perfHooks.performance.now(); - resolve([ - started, - dnsLookup, - tcpHandshake, - sslHandshake, - ttfb, - ended, - parseFloat(res.headers['server-timing'].slice(22) as any), - ]); + resolve([ + started, + dnsLookup, + tcpHandshake, + sslHandshake, + ttfb, + ended, + parseFloat((res.headers['server-timing'] as string).slice(22)), + ]); }); }); - req.on('socket', (socket) => { - socket.on('lookup', () => { + // Listen for timing events once per new socket + req.once('socket', (socket) => { + socket.once('lookup', () => { dnsLookup = plugins.perfHooks.performance.now(); }); - socket.on('connect', () => { + socket.once('connect', () => { tcpHandshake = plugins.perfHooks.performance.now(); }); - socket.on('secureConnect', () => { + socket.once('secureConnect', () => { sslHandshake = plugins.perfHooks.performance.now(); }); }); @@ -238,20 +246,14 @@ export class CloudflareSpeed { text .split('\n') .map((i) => { - const j = i.split('='); - - return [j[0], j[1]]; + const parts = i.split('='); + return [parts[0], parts[1]]; }) - .reduce((data: any, [k, v]) => { + .reduce((data: Record, [k, v]) => { if (v === undefined) return data; - - // Bypass prettier "no-assign-param" rules - const data1 = data; - // Object.fromEntries is only supported by Node.js 12 or newer - data1[k] = v; - - return data1; - }, {}); + data[k] = v; + return data; + }, {} as Record); return this.get('speed.cloudflare.com', '/cdn-cgi/trace').then(parseCfCdnCgiTrace); } diff --git a/ts/smartnetwork.classes.smartnetwork.ts b/ts/smartnetwork.classes.smartnetwork.ts index 61ce825..dff302e 100644 --- a/ts/smartnetwork.classes.smartnetwork.ts +++ b/ts/smartnetwork.classes.smartnetwork.ts @@ -1,6 +1,7 @@ import * as plugins from './smartnetwork.plugins.js'; import { CloudflareSpeed } from './smartnetwork.classes.cloudflarespeed.js'; +import { getLogger } from './logging.js'; /** * SmartNetwork simplifies actions within the network @@ -37,11 +38,7 @@ export class SmartNetwork { // test IPv4 space const ipv4Test = net.createServer(); - ipv4Test.once('error', (err: any) => { - if (err.code !== 'EADDRINUSE') { - doneIpV4.resolve(false); - return; - } + ipv4Test.once('error', () => { doneIpV4.resolve(false); }); ipv4Test.once('listening', () => { @@ -56,11 +53,7 @@ export class SmartNetwork { // test IPv6 space const ipv6Test = net.createServer(); - ipv6Test.once('error', function (err: any) { - if (err.code !== 'EADDRINUSE') { - doneIpV6.resolve(false); - return; - } + ipv6Test.once('error', () => { doneIpV6.resolve(false); }); ipv6Test.once('listening', () => { @@ -87,14 +80,15 @@ export class SmartNetwork { const domainPart = domainArg.split(':')[0]; const port = portArg ? portArg : parseInt(domainArg.split(':')[1], 10); - plugins.isopen(domainPart, port, (response: any) => { - console.log(response); - if (response[port.toString()].isOpen) { - done.resolve(true); - } else { - done.resolve(false); - } - }); + plugins.isopen( + domainPart, + port, + (response: Record) => { + getLogger().debug(response); + const portInfo = response[port.toString()]; + done.resolve(Boolean(portInfo?.isOpen)); + }, + ); const result = await done.promise; return result; } @@ -110,7 +104,7 @@ export class SmartNetwork { }> { const defaultGatewayName = await plugins.systeminformation.networkInterfaceDefault(); if (!defaultGatewayName) { - console.log('Cannot determine default gateway'); + getLogger().warn?.('Cannot determine default gateway'); return null; } const gateways = await this.getGateways();