diff --git a/src/http-request.ts b/src/http-request.ts index a04f72b..6bd2820 100644 --- a/src/http-request.ts +++ b/src/http-request.ts @@ -30,7 +30,7 @@ function getFs() { return _fs as typeof import('node:fs') } -import type { IncomingMessage } from 'http' +import type { IncomingHttpHeaders, IncomingMessage } from 'http' import type { Logger } from './logger.js' @@ -74,6 +74,38 @@ function getHttps() { return _https as typeof import('node:https') } +/** + * Information passed to the onRequest hook before each request attempt. + */ +export interface HttpHookRequestInfo { + headers: Record + method: string + timeout: number + url: string +} + +/** + * Information passed to the onResponse hook after each request attempt. + */ +export interface HttpHookResponseInfo { + duration: number + error?: Error | undefined + headers?: IncomingHttpHeaders | undefined + method: string + status?: number | undefined + statusText?: string | undefined + url: string +} + +/** + * Lifecycle hooks for observing HTTP request/response events. + * Hooks fire per-attempt (retries produce multiple hook calls). + */ +export interface HttpHooks { + onRequest?: ((info: HttpHookRequestInfo) => void) | undefined + onResponse?: ((info: HttpHookResponseInfo) => void) | undefined +} + /** * Configuration options for HTTP/HTTPS requests. */ @@ -136,6 +168,11 @@ export interface HttpRequestOptions { * ``` */ followRedirects?: boolean | undefined + /** + * Lifecycle hooks for observing request/response events. + * Hooks fire per-attempt — retries and redirects each trigger separate hook calls. + */ + hooks?: HttpHooks | undefined /** * HTTP headers to send with the request. * A `User-Agent` header is automatically added if not provided. @@ -167,6 +204,14 @@ export interface HttpRequestOptions { * ``` */ maxRedirects?: number | undefined + /** + * Maximum response body size in bytes. Responses exceeding this limit + * will be rejected with an error. Prevents memory exhaustion from + * unexpectedly large responses. + * + * @default undefined (no limit) + */ + maxResponseSize?: number | undefined /** * HTTP method to use for the request. * @@ -281,7 +326,7 @@ export interface HttpResponse { * console.log(response.headers['set-cookie']) // May be string[] * ``` */ - headers: Record + headers: IncomingHttpHeaders /** * Parse response body as JSON. * Type parameter `T` allows specifying the expected JSON structure. @@ -346,6 +391,12 @@ export interface HttpResponse { * ``` */ text(): string + /** + * The underlying Node.js IncomingMessage for advanced use cases + * (e.g., streaming, custom header inspection). Only available when + * the response was not consumed by the convenience methods. + */ + rawResponse?: IncomingMessage | undefined } /** @@ -891,8 +942,47 @@ async function httpDownloadAttempt( }) } +/** + * Build an enriched error message based on the error code. + * Generic guidance (no product-specific branding). + */ +export function enrichErrorMessage( + url: string, + method: string, + error: NodeJS.ErrnoException, +): string { + const code = error.code + let message = `${method} request failed: ${url}` + if (code === 'ECONNREFUSED') { + message += + '\n→ Connection refused. Server is unreachable.\n→ Check: Network connectivity and firewall settings.' + } else if (code === 'ENOTFOUND') { + message += + '\n→ DNS lookup failed. Cannot resolve hostname.\n→ Check: Internet connection and DNS settings.' + } else if (code === 'ETIMEDOUT') { + message += + '\n→ Connection timed out. Network or server issue.\n→ Try: Check network connectivity and retry.' + } else if (code === 'ECONNRESET') { + message += + '\n→ Connection reset by server. Possible network interruption.\n→ Try: Retry the request.' + } else if (code === 'EPIPE') { + message += + '\n→ Broken pipe. Server closed connection unexpectedly.\n→ Check: Authentication credentials and permissions.' + } else if ( + code === 'CERT_HAS_EXPIRED' || + code === 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' + ) { + message += + '\n→ SSL/TLS certificate error.\n→ Check: System time and date are correct.\n→ Try: Update CA certificates on your system.' + } else if (code) { + message += `\n→ Error code: ${code}` + } + return message +} + /** * Single HTTP request attempt (used internally by httpRequest with retry logic). + * Supports hooks (fire per-attempt), maxResponseSize, and rawResponse. * @private */ async function httpRequestAttempt( @@ -904,21 +994,28 @@ async function httpRequestAttempt( ca, followRedirects = true, headers = {}, + hooks, maxRedirects = 5, + maxResponseSize, method = 'GET', timeout = 30_000, } = { __proto__: null, ...options } as HttpRequestOptions + const startTime = Date.now() + const mergedHeaders = { + 'User-Agent': 'socket-registry/1.0', + ...headers, + } + + hooks?.onRequest?.({ method, url, headers: mergedHeaders, timeout }) + return await new Promise((resolve, reject) => { const parsedUrl = new URL(url) const isHttps = parsedUrl.protocol === 'https:' const httpModule = isHttps ? getHttps() : getHttp() const requestOptions: Record = { - headers: { - 'User-Agent': 'socket-registry/1.0', - ...headers, - }, + headers: mergedHeaders, hostname: parsedUrl.hostname, method, path: parsedUrl.pathname + parsedUrl.search, @@ -926,16 +1023,23 @@ async function httpRequestAttempt( timeout, } - // Pass custom CA certificates for TLS connections. if (ca && isHttps) { requestOptions['ca'] = ca } + const emitResponse = (info: Partial) => { + hooks?.onResponse?.({ + duration: Date.now() - startTime, + method, + url, + ...info, + }) + } + /* c8 ignore start - External HTTP/HTTPS request */ const request = httpModule.request( requestOptions, (res: IncomingMessage) => { - // Handle redirects if ( followRedirects && res.statusCode && @@ -943,6 +1047,12 @@ async function httpRequestAttempt( res.statusCode < 400 && res.headers.location ) { + emitResponse({ + headers: res.headers, + status: res.statusCode, + statusText: res.statusMessage, + }) + if (maxRedirects <= 0) { reject( new Error( @@ -952,12 +1062,10 @@ async function httpRequestAttempt( return } - // Follow redirect const redirectUrl = res.headers.location.startsWith('http') ? res.headers.location : new URL(res.headers.location, url).toString() - // Reject HTTPS-to-HTTP downgrade redirects. const redirectParsed = new URL(redirectUrl) if (isHttps && redirectParsed.protocol !== 'https:') { reject( @@ -974,7 +1082,9 @@ async function httpRequestAttempt( ca, followRedirects, headers, + hooks, maxRedirects: maxRedirects - 1, + maxResponseSize, method, timeout, }), @@ -982,9 +1092,22 @@ async function httpRequestAttempt( return } - // Collect response data const chunks: Buffer[] = [] + let totalBytes = 0 + res.on('data', (chunk: Buffer) => { + totalBytes += chunk.length + if (maxResponseSize && totalBytes > maxResponseSize) { + res.destroy() + const sizeMB = (totalBytes / (1024 * 1024)).toFixed(2) + const maxMB = (maxResponseSize / (1024 * 1024)).toFixed(2) + const err = new Error( + `Response exceeds maximum size limit (${sizeMB}MB > ${maxMB}MB)`, + ) + emitResponse({ error: err }) + reject(err) + return + } chunks.push(chunk) }) @@ -1003,14 +1126,12 @@ async function httpRequestAttempt( ) }, body: responseBody, - headers: res.headers as Record< - string, - string | string[] | undefined - >, + headers: res.headers, json(): T { return JSON.parse(responseBody.toString('utf8')) as T }, ok, + rawResponse: res, status: res.statusCode || 0, statusText: res.statusMessage || '', text(): string { @@ -1018,45 +1139,42 @@ async function httpRequestAttempt( }, } + emitResponse({ + headers: res.headers, + status: res.statusCode, + statusText: res.statusMessage, + }) + resolve(response) }) res.on('error', (error: Error) => { + emitResponse({ error }) reject(error) }) }, ) request.on('error', (error: Error) => { - const code = (error as NodeJS.ErrnoException).code - let message = `HTTP request failed for ${url}: ${error.message}\n` - - if (code === 'ENOTFOUND') { - message += - 'DNS lookup failed. Check the hostname and your network connection.' - } else if (code === 'ECONNREFUSED') { - message += - 'Connection refused. Verify the server is running and accessible.' - } else if (code === 'ETIMEDOUT') { - message += - 'Request timed out. Check your network or increase the timeout value.' - } else if (code === 'ECONNRESET') { - message += - 'Connection reset. The server may have closed the connection unexpectedly.' - } else { - message += - 'Check your network connection and verify the URL is correct.' - } - - reject(new Error(message, { cause: error })) + const message = enrichErrorMessage( + url, + method, + error as NodeJS.ErrnoException, + ) + const enhanced = new Error(message, { cause: error }) + emitResponse({ error: enhanced }) + reject(enhanced) }) request.on('timeout', () => { request.destroy() - reject(new Error(`Request timed out after ${timeout}ms`)) + const err = new Error( + `${method} request timed out after ${timeout}ms: ${url}\n→ Server did not respond in time.\n→ Try: Increase timeout or check network connectivity.`, + ) + emitResponse({ error: err }) + reject(err) }) - // Send body if present if (body) { request.write(body) } @@ -1384,7 +1502,9 @@ export async function httpRequest( ca, followRedirects = true, headers = {}, + hooks, maxRedirects = 5, + maxResponseSize, method = 'GET', retries = 0, retryDelay = 1000, @@ -1401,7 +1521,9 @@ export async function httpRequest( ca, followRedirects, headers, + hooks, maxRedirects, + maxResponseSize, method, timeout, }) diff --git a/test/unit/http-request.test.mts b/test/unit/http-request.test.mts index 9957637..604b080 100644 --- a/test/unit/http-request.test.mts +++ b/test/unit/http-request.test.mts @@ -18,6 +18,7 @@ import path from 'node:path' import { Writable } from 'node:stream' import { + enrichErrorMessage, fetchChecksums, httpDownload, httpJson, @@ -25,6 +26,10 @@ import { httpText, parseChecksums, } from '@socketsecurity/lib/http-request' +import type { + HttpHookRequestInfo, + HttpHookResponseInfo, +} from '@socketsecurity/lib/http-request' import { Logger } from '@socketsecurity/lib/logger' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { runWithTempDir } from './utils/temp-file-helper' @@ -163,6 +168,13 @@ beforeAll(async () => { // Empty checksums file (only comments). res.writeHead(200, { 'Content-Type': 'text/plain' }) res.end('# This file has no checksums\n\n') + } else if (url === '/large-body') { + const content = 'X'.repeat(10_000) + res.writeHead(200, { + 'Content-Length': String(content.length), + 'Content-Type': 'text/plain', + }) + res.end(content) } else if (url === '/post-success') { if (req.method === 'POST') { res.writeHead(201, { 'Content-Type': 'application/json' }) @@ -428,7 +440,7 @@ describe('http-request', () => { retries: 2, retryDelay: 10, }), - ).rejects.toThrow(/HTTP request failed/) + ).rejects.toThrow(/request failed/) expect(attemptCount).toBe(3) // Initial attempt + 2 retries } finally { await new Promise(resolve => { @@ -440,7 +452,7 @@ describe('http-request', () => { it('should handle network errors', async () => { await expect( httpRequest('http://localhost:1/nonexistent', { timeout: 100 }), - ).rejects.toThrow(/HTTP request failed/) + ).rejects.toThrow(/request failed/) }) it('should handle invalid URLs gracefully', async () => { @@ -498,7 +510,7 @@ describe('http-request', () => { try { await expect( httpRequest(`http://localhost:${testPort}/`), - ).rejects.toThrow(/HTTP request failed/) + ).rejects.toThrow(/request failed/) } finally { await new Promise(resolve => { testServer.close(() => resolve()) @@ -1804,4 +1816,305 @@ abc123def456789012345678901234567890123456789012345678901234abcd expect(response.text()).toBe('Plain text response') }) }) + + describe('hooks', () => { + it('should call onRequest with method, url, headers, and timeout', async () => { + const requestInfos: HttpHookRequestInfo[] = [] + await httpRequest(`${httpBaseUrl}/json`, { + headers: { 'X-Custom': 'test-value' }, + hooks: { + onRequest: info => requestInfos.push(info), + }, + }) + expect(requestInfos).toHaveLength(1) + expect(requestInfos[0]!.method).toBe('GET') + expect(requestInfos[0]!.url).toBe(`${httpBaseUrl}/json`) + expect(requestInfos[0]!.timeout).toBe(30_000) + expect(requestInfos[0]!.headers['User-Agent']).toBe('socket-registry/1.0') + expect(requestInfos[0]!.headers['X-Custom']).toBe('test-value') + }) + + it('should call onResponse with status, headers, and duration', async () => { + const responseInfos: HttpHookResponseInfo[] = [] + await httpRequest(`${httpBaseUrl}/json`, { + hooks: { + onResponse: info => responseInfos.push(info), + }, + }) + expect(responseInfos).toHaveLength(1) + expect(responseInfos[0]!.method).toBe('GET') + expect(responseInfos[0]!.url).toBe(`${httpBaseUrl}/json`) + expect(responseInfos[0]!.status).toBe(200) + expect(responseInfos[0]!.statusText).toBe('OK') + expect(responseInfos[0]!.duration).toBeGreaterThanOrEqual(0) + expect(responseInfos[0]!.error).toBeUndefined() + expect(responseInfos[0]!.headers?.['content-type']).toContain( + 'application/json', + ) + }) + + it('should call onResponse with error on timeout', async () => { + const responseInfos: HttpHookResponseInfo[] = [] + await httpRequest(`${httpBaseUrl}/timeout`, { + timeout: 50, + hooks: { + onResponse: info => responseInfos.push(info), + }, + }).catch(() => {}) + expect(responseInfos).toHaveLength(1) + expect(responseInfos[0]!.error).toBeDefined() + }) + + it('should fire hooks per-attempt on retries', async () => { + const requestInfos: HttpHookRequestInfo[] = [] + const responseInfos: HttpHookResponseInfo[] = [] + + let attemptCount = 0 + const testServer = http.createServer((_req, _res) => { + attemptCount++ + _res.socket?.destroy() + }) + + await new Promise(resolve => { + testServer.listen(0, () => resolve()) + }) + const address = testServer.address() + const testPort = address && typeof address === 'object' ? address.port : 0 + + try { + await httpRequest(`http://localhost:${testPort}/`, { + retries: 1, + retryDelay: 10, + hooks: { + onRequest: info => requestInfos.push(info), + onResponse: info => responseInfos.push(info), + }, + }).catch(() => {}) + + expect(attemptCount).toBe(2) + expect(requestInfos).toHaveLength(2) + expect(responseInfos).toHaveLength(2) + for (const info of responseInfos) { + expect(info.error).toBeDefined() + } + } finally { + await new Promise(resolve => { + testServer.close(() => resolve()) + }) + } + }) + + it('should fire hooks on redirect hops with correct status codes', async () => { + const requestInfos: HttpHookRequestInfo[] = [] + const responseInfos: HttpHookResponseInfo[] = [] + + await httpRequest(`${httpBaseUrl}/redirect`, { + hooks: { + onRequest: info => requestInfos.push(info), + onResponse: info => responseInfos.push(info), + }, + }) + + expect(requestInfos).toHaveLength(2) + expect(responseInfos).toHaveLength(2) + expect(responseInfos[0]!.status).toBe(302) + expect(responseInfos[1]!.status).toBe(200) + }) + + it('should report POST method in hook info', async () => { + const requestInfos: HttpHookRequestInfo[] = [] + await httpRequest(`${httpBaseUrl}/echo-body`, { + method: 'POST', + body: 'test', + hooks: { + onRequest: info => requestInfos.push(info), + }, + }) + expect(requestInfos[0]!.method).toBe('POST') + }) + + it('should work with empty hooks object', async () => { + const response = await httpRequest(`${httpBaseUrl}/json`, { hooks: {} }) + expect(response.ok).toBe(true) + }) + + it('should pass hooks through httpJson and httpText', async () => { + const jsonInfos: HttpHookResponseInfo[] = [] + await httpJson(`${httpBaseUrl}/json`, { + hooks: { onResponse: info => jsonInfos.push(info) }, + }) + expect(jsonInfos).toHaveLength(1) + expect(jsonInfos[0]!.status).toBe(200) + + const textInfos: HttpHookResponseInfo[] = [] + await httpText(`${httpBaseUrl}/text`, { + hooks: { onResponse: info => textInfos.push(info) }, + }) + expect(textInfos).toHaveLength(1) + expect(textInfos[0]!.status).toBe(200) + }) + }) + + describe('maxResponseSize', () => { + it('should reject responses exceeding limit with size info', async () => { + try { + await httpRequest(`${httpBaseUrl}/large-body`, { + maxResponseSize: 50, + }) + expect.unreachable('should have thrown') + } catch (e) { + const msg = (e as Error).message + expect(msg).toMatch(/exceeds maximum size limit/) + expect(msg).toMatch(/MB.*>.*MB/) + } + }) + + it('should allow responses within limit', async () => { + const response = await httpRequest(`${httpBaseUrl}/json`, { + maxResponseSize: 1_000_000, + }) + expect(response.ok).toBe(true) + }) + + it('should allow response exactly at limit', async () => { + const probe = await httpRequest(`${httpBaseUrl}/json`) + const exactSize = probe.body.length + + const response = await httpRequest(`${httpBaseUrl}/json`, { + maxResponseSize: exactSize, + }) + expect(response.ok).toBe(true) + expect(response.body.length).toBe(exactSize) + }) + + it('should treat 0 as no limit', async () => { + const response = await httpRequest(`${httpBaseUrl}/json`, { + maxResponseSize: 0, + }) + expect(response.ok).toBe(true) + }) + + it('should enforce after redirect', async () => { + await expect( + httpRequest(`${httpBaseUrl}/redirect`, { + maxResponseSize: 5, + }), + ).rejects.toThrow(/exceeds maximum size limit/) + }) + + it('should work with httpJson and httpText', async () => { + await expect( + httpJson(`${httpBaseUrl}/json`, { maxResponseSize: 5 }), + ).rejects.toThrow(/exceeds maximum size limit/) + + await expect( + httpText(`${httpBaseUrl}/text`, { maxResponseSize: 5 }), + ).rejects.toThrow(/exceeds maximum size limit/) + }) + + it('should fire onResponse hook with error on size limit', async () => { + const responseInfos: HttpHookResponseInfo[] = [] + await httpRequest(`${httpBaseUrl}/large-body`, { + maxResponseSize: 50, + hooks: { + onResponse: info => responseInfos.push(info), + }, + }).catch(() => {}) + + expect(responseInfos.length).toBeGreaterThanOrEqual(1) + const sizeError = responseInfos.find(info => + info.error?.message?.includes('exceeds maximum size limit'), + ) + expect(sizeError).toBeDefined() + }) + }) + + describe('rawResponse', () => { + it('should expose IncomingMessage with status and headers', async () => { + const response = await httpRequest(`${httpBaseUrl}/json`) + expect(response.rawResponse).toBeDefined() + expect(response.rawResponse!.statusCode).toBe(200) + expect(response.rawResponse!.headers['content-type']).toContain( + 'application/json', + ) + }) + + it('should be from final response after redirect', async () => { + const response = await httpRequest(`${httpBaseUrl}/redirect`) + expect(response.rawResponse).toBeDefined() + expect(response.rawResponse!.statusCode).toBe(200) + }) + + it('should be available on non-2xx responses', async () => { + const r404 = await httpRequest(`${httpBaseUrl}/not-found`) + expect(r404.rawResponse!.statusCode).toBe(404) + + const r500 = await httpRequest(`${httpBaseUrl}/server-error`) + expect(r500.rawResponse!.statusCode).toBe(500) + }) + }) + + describe('enrichErrorMessage', () => { + it('should enrich each known error code', () => { + const cases: Array<[string, string]> = [ + ['ECONNREFUSED', 'Connection refused'], + ['ENOTFOUND', 'DNS lookup failed'], + ['ETIMEDOUT', 'Connection timed out'], + ['ECONNRESET', 'Connection reset'], + ['EPIPE', 'Broken pipe'], + ['CERT_HAS_EXPIRED', 'SSL/TLS certificate error'], + ['UNABLE_TO_VERIFY_LEAF_SIGNATURE', 'SSL/TLS certificate error'], + ] + for (const [code, expected] of cases) { + const err = Object.assign(new Error('test'), { + code, + }) as NodeJS.ErrnoException + const msg = enrichErrorMessage('http://example.com', 'GET', err) + expect(msg).toContain(expected) + } + }) + + it('should include method, url, and error code in message', () => { + const err = Object.assign(new Error('fail'), { + code: 'ESOMETHING', + }) as NodeJS.ErrnoException + const msg = enrichErrorMessage('http://my-server:8080/api', 'DELETE', err) + expect(msg).toContain('DELETE request failed') + expect(msg).toContain('http://my-server:8080/api') + expect(msg).toContain('Error code: ESOMETHING') + }) + + it('should handle errors without a code', () => { + const err = new Error('generic error') as NodeJS.ErrnoException + const msg = enrichErrorMessage('http://example.com', 'GET', err) + expect(msg).toContain('GET request failed') + expect(msg).not.toContain('Error code:') + }) + }) + + describe('enriched error messages — integration', () => { + it('should include method and url in timeout errors', async () => { + try { + await httpRequest(`${httpBaseUrl}/timeout`, { timeout: 50 }) + expect.unreachable('should have thrown') + } catch (e) { + const msg = (e as Error).message + expect(msg).toContain('GET') + expect(msg).toContain('timed out') + expect(msg).toContain(`${httpBaseUrl}/timeout`) + } + }) + + it('should include method, url, and cause chain on connection errors', async () => { + try { + await httpRequest('http://localhost:1/no-server', { timeout: 100 }) + expect.unreachable('should have thrown') + } catch (e) { + const msg = (e as Error).message + expect(msg).toContain('request failed') + expect(msg).toContain('localhost:1') + expect((e as Error).cause).toBeDefined() + } + }) + }) })