From 3ef471dffdf8fa335e06cdc48ba3dc1fac81b513 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 6 Jul 2026 16:03:12 -0500 Subject: [PATCH 1/3] Harden request config validation - validate hook lists consistently for defaults and requests - report ConfigError for invalid methods and hook configuration - preserve onError visibility for normalization and rebuild failures - cap retained ParseError body text and cover the public type export --- src/internal/client-defaults.ts | 43 +++++-------- src/internal/execute-request.ts | 48 ++++++++++++-- src/internal/hooks.ts | 59 +++++++++++++++++ src/internal/normalize-request.ts | 30 ++------- src/internal/parse-response.ts | 2 +- src/types.ts | 4 ++ test/client-defaults.test.ts | 27 ++++++++ test/hooks-and-retries.test.ts | 102 ++++++++++++++++++++++++++++++ test/normalize-request.test.ts | 50 +++++++++++++++ test/parse-response.test.ts | 22 +++++++ test/type-signatures.ts | 8 ++- 11 files changed, 336 insertions(+), 59 deletions(-) create mode 100644 src/internal/hooks.ts diff --git a/src/internal/client-defaults.ts b/src/internal/client-defaults.ts index 7262911..728117a 100644 --- a/src/internal/client-defaults.ts +++ b/src/internal/client-defaults.ts @@ -3,6 +3,12 @@ import type { Hooks, RetryOptions, } from '../types.js' +import { + mergeHooks, + normalizeAfterResponseHooks, + normalizeBeforeRequestHooks, + normalizeOnErrorHooks, +} from './hooks.js' export function mergeClientDefaults( parent: ClientDefaults, @@ -81,20 +87,7 @@ function mergeHookDefaults( parent: ClientDefaults, child: ClientDefaults, ): void { - const hooks = { - beforeRequest: [ - ...(parent.hooks?.beforeRequest ?? []), - ...(child.hooks?.beforeRequest ?? []), - ], - afterResponse: [ - ...(parent.hooks?.afterResponse ?? []), - ...(child.hooks?.afterResponse ?? []), - ], - onError: [ - ...(parent.hooks?.onError ?? []), - ...(child.hooks?.onError ?? []), - ], - } + const hooks = mergeHooks(parent.hooks, child.hooks) if (hooks.beforeRequest.length + hooks.afterResponse.length + hooks.onError.length > 0) { merged.hooks = hooks @@ -196,19 +189,17 @@ function copyRetryOnMethods( function snapshotHooks(hooks: Hooks): Hooks { const snapshot: Hooks = {} - copyHookList(snapshot, 'beforeRequest', hooks.beforeRequest) - copyHookList(snapshot, 'afterResponse', hooks.afterResponse) - copyHookList(snapshot, 'onError', hooks.onError) + if (hooks.beforeRequest !== undefined) { + snapshot.beforeRequest = normalizeBeforeRequestHooks(hooks) + } - return snapshot -} + if (hooks.afterResponse !== undefined) { + snapshot.afterResponse = normalizeAfterResponseHooks(hooks) + } -function copyHookList( - snapshot: Hooks, - key: Key, - hooks: Hooks[Key], -): void { - if (hooks !== undefined) { - snapshot[key] = hooks.slice() as Hooks[Key] + if (hooks.onError !== undefined) { + snapshot.onError = normalizeOnErrorHooks(hooks) } + + return snapshot } diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index 8495012..f77c6cf 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -23,6 +23,7 @@ import { createBeforeRequestContext, type ExecutionBeforeRequestContext, } from './normalize-request.js' +import { normalizeOnErrorHooks } from './hooks.js' import { parseResponse } from './parse-response.js' import { getRetryDelay, @@ -44,7 +45,24 @@ export async function executeRequest( ): Promise { // Determine retry bounds from a normalized first pass, then rebuild the // full execution context per attempt so hook mutations do not leak across retries. - const initialContext = createBeforeRequestContext(input, defaults, options) + let initialContext: ExecutionBeforeRequestContext + try { + initialContext = createBeforeRequestContext(input, defaults, options) + } catch (error) { + let onErrorHooks: OnErrorHook[] + try { + onErrorHooks = getInitialOnErrorHooks(defaults, options) + } catch { + throw error + } + + await runOnErrorHooks({ + input, + error, + }, onErrorHooks) + throw error + } + const maxAttempts = initialContext._internalOptions.retry === false ? 1 @@ -53,10 +71,20 @@ export async function executeRequest( let lastError: HttpClientError | undefined for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { - const context = - attempt === 1 - ? initialContext - : createBeforeRequestContext(input, defaults, options, attempt) + let context: ExecutionBeforeRequestContext + if (attempt === 1) { + context = initialContext + } else { + try { + context = createBeforeRequestContext(input, defaults, options, attempt) + } catch (error) { + await runOnErrorHooks({ + input, + error, + }, initialContext._internalOptions.hooks.onError) + throw error + } + } try { try { @@ -206,6 +234,16 @@ async function runOnErrorHooks( } } +function getInitialOnErrorHooks( + defaults: ClientDefaults, + options: RequestOptions, +): OnErrorHook[] { + return [ + ...normalizeOnErrorHooks(defaults.hooks), + ...normalizeOnErrorHooks(options.hooks), + ] +} + class RetrySignal { readonly error: HttpClientError diff --git a/src/internal/hooks.ts b/src/internal/hooks.ts new file mode 100644 index 0000000..48a8b43 --- /dev/null +++ b/src/internal/hooks.ts @@ -0,0 +1,59 @@ +import { ConfigError } from '../errors.js' +import type { + AfterResponseHook, + BeforeRequestHook, + Hooks, + OnErrorHook, +} from '../types.js' + +type HookList = BeforeRequestHook[] | AfterResponseHook[] | OnErrorHook[] + +export function mergeHooks( + defaultHooks?: Hooks, + requestHooks?: Hooks, +): Required { + return { + beforeRequest: [ + ...normalizeBeforeRequestHooks(defaultHooks), + ...normalizeBeforeRequestHooks(requestHooks), + ], + afterResponse: [ + ...normalizeAfterResponseHooks(defaultHooks), + ...normalizeAfterResponseHooks(requestHooks), + ], + onError: [ + ...normalizeOnErrorHooks(defaultHooks), + ...normalizeOnErrorHooks(requestHooks), + ], + } +} + +export function normalizeBeforeRequestHooks(hooks?: Hooks): BeforeRequestHook[] { + return normalizeHookList(hooks?.beforeRequest, 'beforeRequest') as BeforeRequestHook[] +} + +export function normalizeAfterResponseHooks(hooks?: Hooks): AfterResponseHook[] { + return normalizeHookList(hooks?.afterResponse, 'afterResponse') as AfterResponseHook[] +} + +export function normalizeOnErrorHooks(hooks?: Hooks): OnErrorHook[] { + return normalizeHookList(hooks?.onError, 'onError') as OnErrorHook[] +} + +function normalizeHookList(value: unknown, key: keyof Hooks): HookList { + if (value === undefined) { + return [] + } + + if (!Array.isArray(value)) { + throw new ConfigError(`\`hooks.${key}\` must be an array of functions`) + } + + for (const hook of value) { + if (typeof hook !== 'function') { + throw new ConfigError(`\`hooks.${key}\` must be an array of functions`) + } + } + + return value.slice() as HookList +} diff --git a/src/internal/normalize-request.ts b/src/internal/normalize-request.ts index 94fd8c6..dc582ac 100644 --- a/src/internal/normalize-request.ts +++ b/src/internal/normalize-request.ts @@ -2,7 +2,6 @@ import { ConfigError } from '../errors.js' import type { BeforeRequestContext, ClientDefaults, - Hooks, NormalizedRequestOptions, PrimitiveQueryValue, QueryInput, @@ -11,6 +10,7 @@ import type { RequestOptions, ResponseType, } from '../types.js' +import { mergeHooks } from './hooks.js' import { createHookRequestOptions } from './hook-options.js' import { REQUEST_METHODS, normalizeRetry } from './retry-policy.js' @@ -22,12 +22,6 @@ const RESPONSE_TYPES = new Set([ 'raw', ]) -const EMPTY_HOOKS: Required = { - beforeRequest: [], - afterResponse: [], - onError: [], -} - const DEFAULT_PARSE_JSON = (text: string): unknown => JSON.parse(text) export interface ExecutionBeforeRequestContext extends BeforeRequestContext { @@ -243,27 +237,11 @@ function mergeHeaders( return headers } -function mergeHooks( - defaultHooks?: Hooks, - requestHooks?: Hooks, -): Required { - return { - beforeRequest: [ - ...(defaultHooks?.beforeRequest ?? EMPTY_HOOKS.beforeRequest), - ...(requestHooks?.beforeRequest ?? EMPTY_HOOKS.beforeRequest), - ], - afterResponse: [ - ...(defaultHooks?.afterResponse ?? EMPTY_HOOKS.afterResponse), - ...(requestHooks?.afterResponse ?? EMPTY_HOOKS.afterResponse), - ], - onError: [ - ...(defaultHooks?.onError ?? EMPTY_HOOKS.onError), - ...(requestHooks?.onError ?? EMPTY_HOOKS.onError), - ], +function normalizeMethod(method: unknown): RequestMethod { + if (typeof method !== 'string') { + throw new ConfigError('`method` must be a string') } -} -function normalizeMethod(method: string): RequestMethod { return method.toUpperCase() as RequestMethod } diff --git a/src/internal/parse-response.ts b/src/internal/parse-response.ts index 45b4d0e..faeb39a 100644 --- a/src/internal/parse-response.ts +++ b/src/internal/parse-response.ts @@ -69,7 +69,7 @@ async function parseJsonResponse( throw new ParseError({ response, responseType: 'json', - bodyText, + bodyText: truncateBodyText(bodyText, MAX_ERROR_BODY_TEXT_CHARS), cause, }) } diff --git a/src/types.ts b/src/types.ts index 5d1c98c..2f0ae2f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -210,6 +210,10 @@ export interface Hooks { onError?: OnErrorHook[] } +/** + * @deprecated Internal execution metadata. Prefer public request, client, and + * hook option types for consumer code. + */ export interface NormalizedRequestOptions { method: RequestMethod headers: Headers diff --git a/test/client-defaults.test.ts b/test/client-defaults.test.ts index e8c6ec2..dbabd51 100644 --- a/test/client-defaults.test.ts +++ b/test/client-defaults.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict' import test from 'node:test' +import { ConfigError } from '../src/errors.js' import { mergeClientDefaults, snapshotClientDefaults, @@ -93,6 +94,32 @@ test('snapshotClientDefaults preserves property insertion order', () => { ]) }) +test('snapshotClientDefaults rejects invalid hook defaults', () => { + assert.throws( + () => + snapshotClientDefaults({ + hooks: { + beforeRequest: (() => undefined) as never, + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`hooks.beforeRequest` must be an array of functions', + ) + + assert.throws( + () => + snapshotClientDefaults({ + hooks: { + onError: new Array(1) as never, + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`hooks.onError` must be an array of functions', + ) +}) + test('mergeClientDefaults lets child scalar defaults override parent defaults', () => { const parentParser = (text: string) => ({ parent: text }) const childParser = (text: string) => ({ text }) diff --git a/test/hooks-and-retries.test.ts b/test/hooks-and-retries.test.ts index 3e6bfbe..4708952 100644 --- a/test/hooks-and-retries.test.ts +++ b/test/hooks-and-retries.test.ts @@ -651,6 +651,67 @@ test('retry attempts rebuild POST json bodies after the first attempt', async () } }) +test('onError observes retry attempt request rebuild failures before rethrow', async () => { + const originalFetch = globalThis.fetch + const observedErrors: unknown[] = [] + let attempts = 0 + let stringifyCalls = 0 + + const payload = { + toJSON() { + stringifyCalls += 1 + if (stringifyCalls === 2) { + throw new Error('cannot replay payload') + } + return { ok: true } + }, + } + + globalThis.fetch = async () => { + attempts += 1 + return new Response('retry', { + status: 503, + statusText: 'Service Unavailable', + }) + } + + try { + await assert.rejects( + () => + request('https://api.example.com/users', { + method: 'POST', + json: payload, + hooks: { + onError: [ + async (context) => { + observedErrors.push(context.error) + }, + ], + }, + retry: { + attempts: 2, + backoffMs: 1, + maxBackoffMs: 1, + multiplier: 1, + retryOnStatuses: [503], + retryOnMethods: ['POST'], + }, + }), + (error) => + error instanceof Error && + error.message === 'cannot replay payload', + ) + + assert.equal(attempts, 1) + assert.equal(stringifyCalls, 2) + assert.equal(observedErrors.length, 1) + assert.ok(observedErrors[0] instanceof Error) + assert.equal((observedErrors[0] as Error).message, 'cannot replay payload') + } finally { + globalThis.fetch = originalFetch + } +}) + test('retryable HTTP responses do not read body text before retrying', async () => { const originalText = Response.prototype.text let attempts = 0 @@ -1357,3 +1418,44 @@ test('request-level beforeRequest hook can override client header values', async globalThis.fetch = originalFetch } }) + +test('onError observes request normalization failures before rethrow', async () => { + const observedErrors: unknown[] = [] + + await assert.rejects( + () => + request('https://api.example.com/users', { + retry: { + attempts: 0, + }, + hooks: { + onError: [ + async (context) => { + observedErrors.push(context.error) + }, + ], + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`retry.attempts` must be a positive integer', + ) + + assert.equal(observedErrors.length, 1) + assert.ok(observedErrors[0] instanceof ConfigError) +}) + +test('invalid onError hooks do not mask request normalization failures', async () => { + await assert.rejects( + () => + request('https://api.example.com/users', { + method: 123 as never, + hooks: { + onError: [undefined as never], + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`method` must be a string', + ) +}) diff --git a/test/normalize-request.test.ts b/test/normalize-request.test.ts index 130c4b0..2338bdc 100644 --- a/test/normalize-request.test.ts +++ b/test/normalize-request.test.ts @@ -104,6 +104,18 @@ test('normalizeRequestOptions rejects body plus json', () => { ) }) +test('normalizeRequestOptions rejects non-string request methods', () => { + assert.throws( + () => + normalizeRequestOptions({}, { + method: 123 as never, + }), + (error) => + error instanceof ConfigError && + error.message === '`method` must be a string', + ) +}) + test('normalizeRequestOptions rejects request bodies for GET and HEAD', () => { assert.throws( () => @@ -154,6 +166,44 @@ test('normalizeRequestOptions rejects non-function parseJson values', () => { ) }) +test('normalizeRequestOptions rejects invalid hook configuration', () => { + assert.throws( + () => + normalizeRequestOptions({}, { + hooks: { + beforeRequest: (() => undefined) as never, + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`hooks.beforeRequest` must be an array of functions', + ) + + assert.throws( + () => + normalizeRequestOptions({}, { + hooks: { + onError: [undefined as never], + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`hooks.onError` must be an array of functions', + ) + + assert.throws( + () => + normalizeRequestOptions({}, { + hooks: { + afterResponse: new Array(1) as never, + }, + }), + (error) => + error instanceof ConfigError && + error.message === '`hooks.afterResponse` must be an array of functions', + ) +}) + test('normalizeRequestOptions rejects invalid retry values', () => { assert.throws( () => diff --git a/test/parse-response.test.ts b/test/parse-response.test.ts index 6586b87..3f8dc8c 100644 --- a/test/parse-response.test.ts +++ b/test/parse-response.test.ts @@ -44,6 +44,28 @@ test('parseResponse throws ParseError for invalid non-empty json', async () => { ) }) +test('parseResponse truncates large ParseError body text', async () => { + const response = new Response(`{${'x'.repeat(20_000)}`, { + headers: { + 'Content-Type': 'application/json', + }, + }) + + await assert.rejects( + () => + parseResponse({ + response, + responseType: 'json', + parseJson: JSON.parse, + }), + (error) => + error instanceof ParseError && + typeof error.bodyText === 'string' && + error.bodyText.length < 20_000 && + error.bodyText.endsWith('...[truncated]'), + ) +}) + test('parseResponse throws HttpError for non-2xx responses', async () => { const request = new Request('https://api.example.com/users') const response = new Response('missing', { diff --git a/test/type-signatures.ts b/test/type-signatures.ts index 3e827b2..43f3910 100644 --- a/test/type-signatures.ts +++ b/test/type-signatures.ts @@ -1,6 +1,12 @@ -import { createClient, request } from '../src/index.js' +import { + createClient, + request, + type NormalizedRequestOptions, +} from '../src/index.js' const client = createClient() +type PublicNormalizedRequestOptions = NormalizedRequestOptions +void (undefined as unknown as PublicNormalizedRequestOptions) const jsonPromise: Promise<{ ok: boolean } | undefined> = request<{ ok: boolean }>( 'https://api.example.com/users', From 21d5c0d0753cd3158b3c61b8d3e707bca47c7243 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 6 Jul 2026 16:27:08 -0500 Subject: [PATCH 2/3] Prepare v1.0.6 release --- CHANGELOG.md | 7 +++++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 342e3ca..d7803f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ Entries describe repository versions. A version is publicly released only after the matching `vX.Y.Z` tag publishes to npm and a GitHub Release exists. +## 1.0.6 + +- validate hook configuration consistently for client defaults and request options +- report invalid methods and hook lists as `ConfigError` instead of raw runtime errors +- route request normalization and retry rebuild failures through `onError` before rethrowing +- cap retained `ParseError.bodyText` diagnostics for large invalid JSON responses + ## 1.0.5 - add `redactHeaders()` for safe application-owned diagnostics without built-in logging diff --git a/package-lock.json b/package-lock.json index b8343da..f4d8f34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@gavoryn/clearfetch", - "version": "1.0.5", + "version": "1.0.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@gavoryn/clearfetch", - "version": "1.0.5", + "version": "1.0.6", "license": "MIT", "devDependencies": { "@types/node": "^24.5.2", diff --git a/package.json b/package.json index d4ba82b..047b737 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@gavoryn/clearfetch", - "version": "1.0.5", + "version": "1.0.6", "description": "A dependency-free, fetch-native HTTP client for modern JavaScript and TypeScript runtimes.", "type": "module", "sideEffects": false, From 5451877bb5163cbf73b8a9b56e18fda438cde288 Mon Sep 17 00:00:00 2001 From: "brian.j.murdock@gmail.com" Date: Mon, 6 Jul 2026 16:57:24 -0500 Subject: [PATCH 3/3] Clarify error observability behavior --- README.md | 13 ++++++++----- src/internal/execute-request.ts | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b76c787..76fe35a 100644 --- a/README.md +++ b/README.md @@ -214,10 +214,11 @@ const api = createClient({ }) ``` -`beforeRequest` hook failures and request-construction failures propagate as-is -and are observable through `onError` before being re-thrown. `afterResponse` hooks -receive a cloned `Response`, so reading the body there does not consume the -response used for normal parsing or `HttpError` creation. +`beforeRequest` hook failures, request-normalization failures, retry rebuild +failures, and request-construction failures propagate as-is and are observable +through `onError` before being re-thrown. `afterResponse` hooks receive a +cloned `Response`, so reading the body there does not consume the response used +for normal parsing or `HttpError` creation. Hook scope is intentionally narrow: @@ -325,11 +326,13 @@ If you need end-to-end runtime safety, validate parsed data with a schema librar - Non-2xx responses throw `HttpError`. - `HttpError.bodyText` capture is bounded and may be truncated for very large payloads. +- `ParseError.bodyText` capture is also bounded and may be truncated for very large invalid JSON payloads. - `HttpError.response` remains available for status, headers, and metadata, but its body may already be consumed or canceled by diagnostic `bodyText` capture. - JSON mode returns `undefined` for empty response bodies. - In JSON mode, successful empty bodies resolve as `T | undefined`. - No default timeout is applied. Requests run until completion or external abort unless `timeout` is configured. -- Hook and request-construction failures are not wrapped as `NetworkError`. +- Invalid request configuration, including invalid hook lists, fails fast with `ConfigError`. +- Hook, request-normalization, retry rebuild, and request-construction failures are not wrapped as `NetworkError`. - `afterResponse` receives a cloned `Response` for safe inspection. - Relative request inputs require `baseURL`. - `beforeRequest` may override the URL only with a final absolute URL. diff --git a/src/internal/execute-request.ts b/src/internal/execute-request.ts index f77c6cf..e160cfa 100644 --- a/src/internal/execute-request.ts +++ b/src/internal/execute-request.ts @@ -51,6 +51,8 @@ export async function executeRequest( } catch (error) { let onErrorHooks: OnErrorHook[] try { + // Request normalization failed before a context exists, so only recover + // valid `onError` hooks and avoid letting invalid hook config mask it. onErrorHooks = getInitialOnErrorHooks(defaults, options) } catch { throw error