refactoring: phase 0 (#41)

Minor changes

refactor request lifecycle architecture
introduce execution context
add AbortSignal support (internal + partial public)
extract request metadata and controller helpers
improve testability of core modules

Author: romanindev

.changeset/nine-wasps-fetch.md

@@ -0,0 +1,11 @@
+---
+'@dfsync/client': minor
+---
+
+Minor changes
+
+- refactor request lifecycle architecture
+- introduce execution context
+- add AbortSignal support (internal + partial public)
+- extract request metadata and controller helpers
+- improve testability of core modules

.gitignore

@@ -140,6 +140,7 @@ vite.config.ts.timestamp-*
 
 .vitest
 .DS_Store
+.idea/
 
 .tmp/
 smoke/**/node_modules/

packages/client/src/core/apply-request-metadata.ts

@@ -0,0 +1,5 @@
+import type { ExecutionContext } from './execution-context';
+
+export function applyRequestMetadata(execution: ExecutionContext): void {
+  execution.headers['x-request-id'] = execution.headers['x-request-id'] ?? execution.requestId;
+}

packages/client/src/core/create-request-controller.ts

@@ -0,0 +1,44 @@
+export type RequestController = {
+  signal: AbortSignal;
+  cleanup: () => void;
+};
+
+type CreateRequestControllerParams = {
+  timeout: number;
+  signal?: AbortSignal | undefined;
+};
+
+export function createRequestController(params: CreateRequestControllerParams): RequestController {
+  const timeoutController = new AbortController();
+
+  const timeoutId = setTimeout(() => {
+    timeoutController.abort();
+  }, params.timeout);
+
+  if (!params.signal) {
+    return {
+      signal: timeoutController.signal,
+      cleanup: () => {
+        clearTimeout(timeoutId);
+      },
+    };
+  }
+
+  if (params.signal.aborted) {
+    timeoutController.abort();
+  }
+
+  const abortOnExternalSignal = () => {
+    timeoutController.abort();
+  };
+
+  params.signal.addEventListener('abort', abortOnExternalSignal, { once: true });
+
+  return {
+    signal: timeoutController.signal,
+    cleanup: () => {
+      clearTimeout(timeoutId);
+      params.signal?.removeEventListener('abort', abortOnExternalSignal);
+    },
+  };
+}

packages/client/src/core/execution-context.ts

@@ -0,0 +1,36 @@
+import type { HeadersMap } from '../types/common';
+import type { RequestConfig } from '../types/request';
+
+export type ExecutionContext = {
+  request: RequestConfig;
+  url: URL;
+  headers: HeadersMap;
+  attempt: number;
+
+  // future lifecycle fields
+  requestId: string;
+  startedAt: number;
+};
+
+type CreateExecutionContextParams = {
+  request: RequestConfig;
+  url: URL;
+  headers: HeadersMap;
+  attempt: number;
+};
+
+function generateRequestId(): string {
+  return Math.random().toString(36).slice(2);
+}
+
+export function createExecutionContext(params: CreateExecutionContextParams): ExecutionContext {
+  return {
+    request: params.request,
+    url: params.url,
+    headers: params.headers,
+    attempt: params.attempt,
+
+    requestId: generateRequestId(),
+    startedAt: Date.now(),
+  };
+}

packages/client/src/core/hook-context.ts

@@ -0,0 +1,33 @@
+import type { AfterResponseContext, BeforeRequestContext, ErrorContext } from '../types/hooks';
+import type { ExecutionContext } from './execution-context';
+
+export function createBeforeRequestContext(execution: ExecutionContext): BeforeRequestContext {
+  return {
+    request: execution.request,
+    url: execution.url,
+    headers: execution.headers,
+  };
+}
+
+export function createAfterResponseContext<T>(
+  execution: ExecutionContext,
+  response: Response,
+  data: T,
+): AfterResponseContext<T> {
+  return {
+    request: execution.request,
+    url: execution.url,
+    headers: execution.headers,
+    response,
+    data,
+  };
+}
+
+export function createErrorContext(execution: ExecutionContext, error: Error): ErrorContext {
+  return {
+    request: execution.request,
+    url: execution.url,
+    headers: execution.headers,
+    error,
+  };
+}

packages/client/src/core/normalize-error.ts

@@ -0,0 +1,15 @@
+import { HttpError } from '../errors/http-error';
+import { NetworkError } from '../errors/network-error';
+import { TimeoutError } from '../errors/timeout-error';
+
+export function normalizeError(error: unknown, timeout: number): Error {
+  if (error instanceof HttpError) {
+    return error;
+  }
+
+  if (error instanceof Error && error.name === 'AbortError') {
+    return new TimeoutError(timeout, error);
+  }
+
+  return new NetworkError('Network request failed', error);
+}

packages/client/src/core/request.ts

@@ -1,61 +1,31 @@
 import { HttpError } from '../errors/http-error';
 import { NetworkError } from '../errors/network-error';
-import { TimeoutError } from '../errors/timeout-error';
 import type { HeadersMap } from '../types/common';
-import type { ClientConfig, RetryConfig } from '../types/config';
+import type { ClientConfig } from '../types/config';
 import type { RequestConfig } from '../types/request';
 import { applyAuth } from './apply-auth';
 import { buildUrl } from './build-url';
+import { applyRequestMetadata } from './apply-request-metadata';
+import { createExecutionContext } from './execution-context';
+import { createRequestController } from './create-request-controller';
+import {
+  createAfterResponseContext,
+  createBeforeRequestContext,
+  createErrorContext,
+} from './hook-context';
 import { getRetryDelay } from './get-retry-delay';
+import { normalizeError } from './normalize-error';
 import { parseResponse } from './parse-response';
+import { resolveRuntimeConfig } from './resolve-runtime-config';
 import { runHooks, runHooksSafely } from './run-hooks';
 import { shouldRetry } from './should-retry';
-
-const DEFAULT_TIMEOUT = 5000;
-
-const DEFAULT_RETRY: Required<RetryConfig> = {
-  attempts: 0,
-  backoff: 'exponential',
-  baseDelayMs: 300,
-  retryOn: ['network-error', '5xx'],
-  retryMethods: ['GET', 'PUT', 'DELETE'],
-};
-
-function normalizeError(error: unknown, timeout: number): Error {
-  if (error instanceof HttpError) {
-    return error;
-  }
-
-  if (error instanceof Error && error.name === 'AbortError') {
-    return new TimeoutError(timeout, error);
-  }
-
-  return new NetworkError('Network request failed', error);
-}
-
-function sleep(ms: number): Promise<void> {
-  return new Promise((resolve) => {
-    setTimeout(resolve, ms);
-  });
-}
+import { sleep } from './sleep';
 
 export async function request<T>(
   clientConfig: ClientConfig,
   requestConfig: RequestConfig,
 ): Promise<T> {
-  const fetchImpl = clientConfig.fetch ?? globalThis.fetch;
-
-  if (!fetchImpl) {
-    throw new Error('No fetch implementation available');
-  }
-
-  const timeout = requestConfig.timeout ?? clientConfig.timeout ?? DEFAULT_TIMEOUT;
-
-  const retry: Required<RetryConfig> = {
-    ...DEFAULT_RETRY,
-    ...(clientConfig.retry ?? {}),
-    ...(requestConfig.retry ?? {}),
-  };
+  const { fetchImpl, timeout, retry } = resolveRuntimeConfig(clientConfig, requestConfig);
 
   const url = new URL(buildUrl(clientConfig.baseUrl, requestConfig.path, requestConfig.query));
 
@@ -68,50 +38,55 @@ export async function request<T>(
       ...(requestConfig.headers ?? {}),
     };
 
-    await applyAuth({
-      auth: clientConfig.auth,
+    const execution = createExecutionContext({
       request: requestConfig,
       url,
       headers,
+      attempt,
     });
 
-    await runHooks(clientConfig.hooks?.beforeRequest, {
-      request: requestConfig,
-      url,
-      headers,
+    applyRequestMetadata(execution);
+
+    await applyAuth({
+      auth: clientConfig.auth,
+      request: execution.request,
+      url: execution.url,
+      headers: execution.headers,
     });
 
+    await runHooks(clientConfig.hooks?.beforeRequest, createBeforeRequestContext(execution));
+
     let body: BodyInit | undefined;
 
-    if (requestConfig.body !== undefined) {
-      if (typeof requestConfig.body === 'string') {
-        body = requestConfig.body;
+    if (execution.request.body !== undefined) {
+      if (typeof execution.request.body === 'string') {
+        body = execution.request.body;
       } else {
-        headers['content-type'] = headers['content-type'] ?? 'application/json';
-        body = JSON.stringify(requestConfig.body);
+        execution.headers['content-type'] = execution.headers['content-type'] ?? 'application/json';
+        body = JSON.stringify(execution.request.body);
       }
     }
 
-    const controller = new AbortController();
-    const timeoutId = setTimeout(() => {
-      controller.abort();
-    }, timeout);
+    const requestController = createRequestController({
+      timeout,
+      signal: execution.request.signal,
+    });
 
     let response: Response;
     let data: unknown;
 
     try {
       const init: RequestInit = {
-        method: requestConfig.method,
-        headers,
-        signal: controller.signal,
+        method: execution.request.method,
+        headers: execution.headers,
+        signal: requestController.signal,
       };
 
       if (body !== undefined) {
         init.body = body;
       }
 
-      response = await fetchImpl(url.toString(), init);
+      response = await fetchImpl(execution.url.toString(), init);
       data = await parseResponse(response);
 
       if (!response.ok) {
@@ -122,42 +97,34 @@ export async function request<T>(
       lastError = error;
 
       const canRetry = shouldRetry({
-        attempt,
-        method: requestConfig.method,
+        attempt: execution.attempt,
+        method: execution.request.method,
         retry,
         error,
       });
 
       if (!canRetry) {
-        await runHooksSafely(clientConfig.hooks?.onError, {
-          request: requestConfig,
-          url,
-          headers,
-          error,
-        });
+        await runHooksSafely(clientConfig.hooks?.onError, createErrorContext(execution, error));
 
         throw error;
       }
 
       const delay = getRetryDelay({
-        attempt: attempt + 1,
+        attempt: execution.attempt + 1,
         backoff: retry.backoff,
         baseDelayMs: retry.baseDelayMs,
       });
 
       await sleep(delay);
       continue;
     } finally {
-      clearTimeout(timeoutId);
+      requestController.cleanup();
     }
 
-    await runHooks(clientConfig.hooks?.afterResponse, {
-      request: requestConfig,
-      url,
-      headers,
-      response,
-      data,
-    });
+    await runHooks(
+      clientConfig.hooks?.afterResponse,
+      createAfterResponseContext(execution, response, data),
+    );
 
     return data as T;
   }

packages/client/src/core/resolve-runtime-config.ts

@@ -0,0 +1,39 @@
+import type { ClientConfig, RetryConfig } from '../types/config';
+import type { RequestConfig } from '../types/request';
+
+const DEFAULT_TIMEOUT = 5000;
+
+const DEFAULT_RETRY: Required<RetryConfig> = {
+  attempts: 0,
+  backoff: 'exponential',
+  baseDelayMs: 300,
+  retryOn: ['network-error', '5xx'],
+  retryMethods: ['GET', 'PUT', 'DELETE'],
+};
+
+export type ResolvedRuntimeConfig = {
+  fetchImpl: typeof globalThis.fetch;
+  timeout: number;
+  retry: Required<RetryConfig>;
+};
+
+export function resolveRuntimeConfig(
+  clientConfig: ClientConfig,
+  requestConfig: RequestConfig,
+): ResolvedRuntimeConfig {
+  const fetchImpl = clientConfig.fetch ?? globalThis.fetch;
+
+  if (!fetchImpl) {
+    throw new Error('No fetch implementation available');
+  }
+
+  return {
+    fetchImpl,
+    timeout: requestConfig.timeout ?? clientConfig.timeout ?? DEFAULT_TIMEOUT,
+    retry: {
+      ...DEFAULT_RETRY,
+      ...(clientConfig.retry ?? {}),
+      ...(requestConfig.retry ?? {}),
+    },
+  };
+}