Harden request config validation

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

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.

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,

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<Key extends keyof Hooks>(
-  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
 }

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,26 @@ export async function executeRequest<T = unknown>(
 ): Promise<T | Response | string | Blob | ArrayBuffer | undefined> {
   // 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 {
+      // 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
+    }
+
+    await runOnErrorHooks({
+      input,
+      error,
+    }, onErrorHooks)
+    throw error
+  }
+
   const maxAttempts =
     initialContext._internalOptions.retry === false
       ? 1
@@ -53,10 +73,20 @@ export async function executeRequest<T = unknown>(
   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 +236,16 @@ async function runOnErrorHooks(
   }
 }
 
+function getInitialOnErrorHooks(
+  defaults: ClientDefaults,
+  options: RequestOptions,
+): OnErrorHook[] {
+  return [
+    ...normalizeOnErrorHooks(defaults.hooks),
+    ...normalizeOnErrorHooks(options.hooks),
+  ]
+}
+
 class RetrySignal {
   readonly error: HttpClientError
 

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<Hooks> {
+  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
+}

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<ResponseType>([
   'raw',
 ])
 
-const EMPTY_HOOKS: Required<Hooks> = {
-  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<Hooks> {
-  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
 }
 

src/internal/parse-response.ts

@@ -69,7 +69,7 @@ async function parseJsonResponse<T>(
     throw new ParseError({
       response,
       responseType: 'json',
-      bodyText,
+      bodyText: truncateBodyText(bodyText, MAX_ERROR_BODY_TEXT_CHARS),
       cause,
     })
   }

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

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 })