diff --git a/packages/client/src/core/create-request-controller.ts b/packages/client/src/core/create-request-controller.ts index e6cf1dd..ce553bd 100644 --- a/packages/client/src/core/create-request-controller.ts +++ b/packages/client/src/core/create-request-controller.ts @@ -1,6 +1,9 @@ +export type RequestAbortReason = 'timeout' | 'external'; + export type RequestController = { signal: AbortSignal; cleanup: () => void; + getAbortReason: () => RequestAbortReason | undefined; }; type CreateRequestControllerParams = { @@ -11,7 +14,10 @@ type CreateRequestControllerParams = { export function createRequestController(params: CreateRequestControllerParams): RequestController { const timeoutController = new AbortController(); + let abortReason: RequestAbortReason | undefined; + const timeoutId = setTimeout(() => { + abortReason = 'timeout'; timeoutController.abort(); }, params.timeout); @@ -21,14 +27,17 @@ export function createRequestController(params: CreateRequestControllerParams): cleanup: () => { clearTimeout(timeoutId); }, + getAbortReason: () => abortReason, }; } if (params.signal.aborted) { + abortReason = 'external'; timeoutController.abort(); } const abortOnExternalSignal = () => { + abortReason = 'external'; timeoutController.abort(); }; @@ -40,5 +49,6 @@ export function createRequestController(params: CreateRequestControllerParams): clearTimeout(timeoutId); params.signal?.removeEventListener('abort', abortOnExternalSignal); }, + getAbortReason: () => abortReason, }; } diff --git a/packages/client/src/core/execution-context.ts b/packages/client/src/core/execution-context.ts index b93f427..2f8bd24 100644 --- a/packages/client/src/core/execution-context.ts +++ b/packages/client/src/core/execution-context.ts @@ -30,7 +30,7 @@ export function createExecutionContext(params: CreateExecutionContextParams): Ex headers: params.headers, attempt: params.attempt, - requestId: generateRequestId(), + requestId: params.request.requestId ?? generateRequestId(), startedAt: Date.now(), }; } diff --git a/packages/client/src/core/hook-context.ts b/packages/client/src/core/hook-context.ts index 259b163..44db6af 100644 --- a/packages/client/src/core/hook-context.ts +++ b/packages/client/src/core/hook-context.ts @@ -1,23 +1,31 @@ import type { AfterResponseContext, BeforeRequestContext, ErrorContext } from '../types/hooks'; import type { ExecutionContext } from './execution-context'; -export function createBeforeRequestContext(execution: ExecutionContext): BeforeRequestContext { +function createLifecycleContextBase( + execution: ExecutionContext, +): Omit { return { request: execution.request, url: execution.url, headers: execution.headers, + attempt: execution.attempt, + requestId: execution.requestId, + startedAt: execution.startedAt, + signal: execution.request.signal, }; } +export function createBeforeRequestContext(execution: ExecutionContext): BeforeRequestContext { + return createLifecycleContextBase(execution); +} + export function createAfterResponseContext( execution: ExecutionContext, response: Response, data: T, ): AfterResponseContext { return { - request: execution.request, - url: execution.url, - headers: execution.headers, + ...createLifecycleContextBase(execution), response, data, }; @@ -25,9 +33,7 @@ export function createAfterResponseContext( export function createErrorContext(execution: ExecutionContext, error: Error): ErrorContext { return { - request: execution.request, - url: execution.url, - headers: execution.headers, + ...createLifecycleContextBase(execution), error, }; } diff --git a/packages/client/src/core/normalize-error.ts b/packages/client/src/core/normalize-error.ts index 6450414..61aec65 100644 --- a/packages/client/src/core/normalize-error.ts +++ b/packages/client/src/core/normalize-error.ts @@ -1,13 +1,23 @@ import { HttpError } from '../errors/http-error'; import { NetworkError } from '../errors/network-error'; import { TimeoutError } from '../errors/timeout-error'; +import { RequestAbortedError } from '../errors/request-aborted-error'; +import type { RequestAbortReason } from './create-request-controller'; -export function normalizeError(error: unknown, timeout: number): Error { +export function normalizeError( + error: unknown, + timeout: number, + abortReason?: RequestAbortReason, +): Error { if (error instanceof HttpError) { return error; } if (error instanceof Error && error.name === 'AbortError') { + if (abortReason === 'external') { + return new RequestAbortedError(error); + } + return new TimeoutError(timeout, error); } diff --git a/packages/client/src/core/request.ts b/packages/client/src/core/request.ts index bc665ea..e98d1da 100644 --- a/packages/client/src/core/request.ts +++ b/packages/client/src/core/request.ts @@ -93,7 +93,7 @@ export async function request( throw new HttpError(response, data); } } catch (rawError) { - const error = normalizeError(rawError, timeout); + const error = normalizeError(rawError, timeout, requestController.getAbortReason()); lastError = error; const canRetry = shouldRetry({ diff --git a/packages/client/src/errors/request-aborted-error.ts b/packages/client/src/errors/request-aborted-error.ts new file mode 100644 index 0000000..dcc72f2 --- /dev/null +++ b/packages/client/src/errors/request-aborted-error.ts @@ -0,0 +1,8 @@ +import { DfsyncError } from './base-error'; + +export class RequestAbortedError extends DfsyncError { + constructor(cause?: unknown) { + super('Request was aborted', 'REQUEST_ABORTED', cause); + this.name = 'RequestAbortedError'; + } +} diff --git a/packages/client/src/types/hooks.ts b/packages/client/src/types/hooks.ts index 7fbe387..367708a 100644 --- a/packages/client/src/types/hooks.ts +++ b/packages/client/src/types/hooks.ts @@ -1,24 +1,24 @@ import type { HeadersMap } from './common'; import type { RequestConfig } from './request'; -export type BeforeRequestContext = { +type LifecycleContextBase = { request: RequestConfig; url: URL; headers: HeadersMap; + attempt: number; + requestId: string; + startedAt: number; + signal?: AbortSignal | undefined; }; -export type AfterResponseContext = { - request: RequestConfig; - url: URL; - headers: HeadersMap; +export type BeforeRequestContext = LifecycleContextBase; + +export type AfterResponseContext = LifecycleContextBase & { response: Response; data: T; }; -export type ErrorContext = { - request: RequestConfig; - url: URL; - headers: HeadersMap; +export type ErrorContext = LifecycleContextBase & { error: Error; }; diff --git a/packages/client/src/types/request.ts b/packages/client/src/types/request.ts index 30727d2..60d175a 100644 --- a/packages/client/src/types/request.ts +++ b/packages/client/src/types/request.ts @@ -12,6 +12,7 @@ export type RequestConfig = { timeout?: number; retry?: RetryConfig; signal?: AbortSignal; + requestId?: string; }; export type RequestOptionsWithoutBody = Omit; diff --git a/packages/client/tests/integration/abort-signal.test.ts b/packages/client/tests/integration/abort-signal.test.ts index d2ac1ab..0f23242 100644 --- a/packages/client/tests/integration/abort-signal.test.ts +++ b/packages/client/tests/integration/abort-signal.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it, vi } from 'vitest'; import { createClient } from '../../src/core/create-client'; -import { TimeoutError } from '../../src/errors/timeout-error'; +import { RequestAbortedError } from '../../src/errors/request-aborted-error'; import { getFirstFetchInit } from '../testUtils'; describe('client abort signal', () => { - it('throws TimeoutError when request is aborted via external signal', async () => { + it('throws RequestAbortedError when request is aborted via external signal', async () => { const fetchMock = vi.fn((_input: RequestInfo | URL, init?: RequestInit): Promise => { return new Promise((_resolve, reject) => { const rejectWithAbortError = () => { @@ -39,7 +39,7 @@ describe('client abort signal', () => { controller.abort(); - await expect(promise).rejects.toBeInstanceOf(TimeoutError); + await expect(promise).rejects.toBeInstanceOf(RequestAbortedError); }); it('passes the external signal to fetch', async () => { @@ -70,4 +70,43 @@ describe('client abort signal', () => { const init = getFirstFetchInit(fetchMock); expect(init?.signal).toBeDefined(); }); + + it('does not retry when request is aborted via external signal', async () => { + const fetchMock = vi.fn((_input: RequestInfo | URL, init?: RequestInit): Promise => { + return new Promise((_resolve, reject) => { + const rejectWithAbortError = () => { + const abortError = new Error('The operation was aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }; + + if (init?.signal?.aborted) { + rejectWithAbortError(); + return; + } + + init?.signal?.addEventListener('abort', rejectWithAbortError, { + once: true, + }); + }); + }); + + const client = createClient({ + baseUrl: 'https://api.test.com', + timeout: 1000, + fetch: fetchMock, + retry: { attempts: 2 }, + }); + + const controller = new AbortController(); + + const promise = client.get('/slow', { + signal: controller.signal, + }); + + controller.abort(); + + await expect(promise).rejects.toBeInstanceOf(RequestAbortedError); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/client/tests/integration/headers.test.ts b/packages/client/tests/integration/headers.test.ts index 0ee1ba4..147c95e 100644 --- a/packages/client/tests/integration/headers.test.ts +++ b/packages/client/tests/integration/headers.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { createClient } from '../../src/core/create-client'; +import { getFirstFetchInit } from '../testUtils'; describe('request headers', () => { it('adds default accept header', async () => { @@ -101,4 +102,89 @@ describe('request headers', () => { expect(headers['x-shared']).toBe('request'); expect(headers['x-api-key']).toBe('secret-key'); }); + + it('propagates requestId to x-request-id header', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + + const client = createClient({ + baseUrl: 'https://api.test.com', + fetch: fetchMock, + }); + + await client.get('/users', { + requestId: 'req_123', + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + + const firstCall = fetchMock.mock.calls[0]; + expect(firstCall).toBeDefined(); + + const [, init] = firstCall!; + const headers = init?.headers as Record; + + expect(headers['x-request-id']).toBe('req_123'); + }); + + it('prefers explicit x-request-id header over requestId option', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + + const client = createClient({ + baseUrl: 'https://api.test.com', + fetch: fetchMock, + }); + + await client.get('/users', { + requestId: 'req_from_option', + headers: { + 'x-request-id': 'req_from_header', + }, + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + + const firstCall = fetchMock.mock.calls[0]; + expect(firstCall).toBeDefined(); + + const [, init] = firstCall!; + const headers = init?.headers as Record; + + expect(headers['x-request-id']).toBe('req_from_header'); + }); + + it('adds generated x-request-id header when requestId is not provided', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + + const client = createClient({ + baseUrl: 'https://api.test.com', + fetch: fetchMock, + }); + + await client.get('/users'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + + const init = getFirstFetchInit(fetchMock); + const headers = init.headers as Record; + const requestId = headers['x-request-id']; + + expect(requestId).toBeDefined(); + expect(typeof requestId).toBe('string'); + expect(requestId!.length).toBeGreaterThan(0); + }); }); diff --git a/packages/client/tests/integration/hooks.test.ts b/packages/client/tests/integration/hooks.test.ts index 9f2e9a3..50b0fc4 100644 --- a/packages/client/tests/integration/hooks.test.ts +++ b/packages/client/tests/integration/hooks.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from 'vitest'; import { HttpError } from '../../src/errors/http-error'; import { NetworkError } from '../../src/errors/network-error'; import { TimeoutError } from '../../src/errors/timeout-error'; +import { RequestAbortedError } from '../../src/errors/request-aborted-error'; import { createClient } from '../../src/core/create-client'; import { getFirstMockCall } from '../testUtils'; @@ -320,4 +321,50 @@ describe('request hooks', () => { expect(firstCall).toBeDefined(); expect(firstCall![0].error).toBeInstanceOf(TimeoutError); }); + + it('runs onError for externally aborted requests', async () => { + const fetchMock = vi.fn((_input: RequestInfo | URL, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + const rejectWithAbortError = () => { + const abortError = new Error('The operation was aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }; + + if (init?.signal?.aborted) { + rejectWithAbortError(); + return; + } + + init?.signal?.addEventListener('abort', rejectWithAbortError, { + once: true, + }); + }); + }) as typeof fetch; + + const onError = vi.fn(); + + const client = createClient({ + baseUrl: 'https://api.example.com', + timeout: 1000, + fetch: fetchMock, + hooks: { onError }, + }); + + const controller = new AbortController(); + + const promise = client.get('/slow', { + signal: controller.signal, + }); + + controller.abort(); + + await expect(promise).rejects.toBeInstanceOf(RequestAbortedError); + + expect(onError).toHaveBeenCalledTimes(1); + + const firstCall = onError.mock.calls[0]; + expect(firstCall).toBeDefined(); + expect(firstCall![0].error).toBeInstanceOf(RequestAbortedError); + }); }); diff --git a/packages/client/tests/integration/retry.test.ts b/packages/client/tests/integration/retry.test.ts index 2609b9d..28e3903 100644 --- a/packages/client/tests/integration/retry.test.ts +++ b/packages/client/tests/integration/retry.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { createClient } from '../../src/core/create-client'; import { HttpError } from '../../src/errors/http-error'; import { NetworkError } from '../../src/errors/network-error'; +import { RequestAbortedError } from '../../src/errors/request-aborted-error'; describe('client retry', () => { it('retries on 503 and succeeds on the next attempt', async () => { @@ -225,4 +226,42 @@ describe('client retry', () => { expect(fetchMock).toHaveBeenCalledTimes(2); expect(beforeRequest).toHaveBeenCalledTimes(2); }); + + it('does not retry externally aborted requests', async () => { + const fetchMock = vi.fn((_input: RequestInfo | URL, init?: RequestInit): Promise => { + return new Promise((_resolve, reject) => { + const rejectWithAbortError = () => { + const abortError = new Error('The operation was aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }; + + if (init?.signal?.aborted) { + rejectWithAbortError(); + return; + } + + init?.signal?.addEventListener('abort', rejectWithAbortError, { + once: true, + }); + }); + }); + + const client = createClient({ + baseUrl: 'https://api.test.com', + fetch: fetchMock, + retry: { attempts: 2 }, + }); + + const controller = new AbortController(); + + const promise = client.get('/users', { + signal: controller.signal, + }); + + controller.abort(); + + await expect(promise).rejects.toBeInstanceOf(RequestAbortedError); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/client/tests/integration/timeout.test.ts b/packages/client/tests/integration/timeout.test.ts index 0986729..80e4118 100644 --- a/packages/client/tests/integration/timeout.test.ts +++ b/packages/client/tests/integration/timeout.test.ts @@ -21,6 +21,9 @@ describe('client timeout', () => { }); await expect(client.get('/slow')).rejects.toBeInstanceOf(TimeoutError); + await expect(client.get('/slow')).rejects.toMatchObject({ + name: 'TimeoutError', + }); }); it('request timeout overrides client timeout', async () => { @@ -41,5 +44,8 @@ describe('client timeout', () => { }); await expect(client.get('/slow', { timeout: 10 })).rejects.toBeInstanceOf(TimeoutError); + await expect(client.get('/slow')).rejects.toMatchObject({ + name: 'TimeoutError', + }); }); }); diff --git a/packages/client/tests/unit/create-request-controller.test.ts b/packages/client/tests/unit/create-request-controller.test.ts index 4736c52..112bf1e 100644 --- a/packages/client/tests/unit/create-request-controller.test.ts +++ b/packages/client/tests/unit/create-request-controller.test.ts @@ -19,6 +19,7 @@ describe('createRequestController', () => { vi.advanceTimersByTime(100); expect(controller.signal.aborted).toBe(true); + expect(controller.getAbortReason()).toBe('timeout'); controller.cleanup(); }); @@ -35,6 +36,7 @@ describe('createRequestController', () => { vi.advanceTimersByTime(100); expect(controller.signal.aborted).toBe(false); + expect(controller.getAbortReason()).toBeUndefined(); }); it('aborts when the external signal is aborted', () => { @@ -50,6 +52,7 @@ describe('createRequestController', () => { externalController.abort(); expect(controller.signal.aborted).toBe(true); + expect(controller.getAbortReason()).toBe('external'); controller.cleanup(); }); @@ -64,6 +67,24 @@ describe('createRequestController', () => { }); expect(controller.signal.aborted).toBe(true); + expect(controller.getAbortReason()).toBe('external'); + + controller.cleanup(); + }); + + it('has no abort reason before timeout or external abort', () => { + vi.useFakeTimers(); + + const controller = createRequestController({ + timeout: 100, + }); + + expect(controller.getAbortReason()).toBeUndefined(); + + vi.advanceTimersByTime(99); + + expect(controller.signal.aborted).toBe(false); + expect(controller.getAbortReason()).toBeUndefined(); controller.cleanup(); }); diff --git a/packages/client/tests/unit/execution-context.test.ts b/packages/client/tests/unit/execution-context.test.ts index e56c78c..0e8b1a6 100644 --- a/packages/client/tests/unit/execution-context.test.ts +++ b/packages/client/tests/unit/execution-context.test.ts @@ -5,6 +5,36 @@ import type { HeadersMap } from '../../src/types/common'; import type { RequestConfig } from '../../src/types/request'; describe('createExecutionContext', () => { + it('uses request.requestId when provided', () => { + const execution = createExecutionContext({ + request: { + method: 'GET', + path: '/users', + requestId: 'req_custom_123', + }, + url: new URL('https://api.example.com/users'), + headers: {}, + attempt: 0, + }); + + expect(execution.requestId).toBe('req_custom_123'); + }); + + it('generates requestId when request.requestId is not provided', () => { + const execution = createExecutionContext({ + request: { + method: 'GET', + path: '/users', + }, + url: new URL('https://api.example.com/users'), + headers: {}, + attempt: 0, + }); + + expect(typeof execution.requestId).toBe('string'); + expect(execution.requestId.length).toBeGreaterThan(0); + }); + it('creates execution context with request lifecycle metadata', () => { const request: RequestConfig = { method: 'GET', @@ -25,10 +55,11 @@ describe('createExecutionContext', () => { attempt: 2, }); - expect(execution.request).toBe(request); - expect(execution.url).toBe(url); - expect(execution.headers).toBe(headers); expect(execution.attempt).toBe(2); + expect(execution.request.method).toBe('GET'); + expect(execution.request.path).toBe('/users'); + expect(execution.url.toString()).toBe('https://api.example.com/users'); + expect(execution.headers).toBe(headers); expect(execution.requestId).toEqual(expect.any(String)); expect(execution.requestId.length).toBeGreaterThan(0); diff --git a/packages/client/tests/unit/hook-context.test.ts b/packages/client/tests/unit/hook-context.test.ts new file mode 100644 index 0000000..3a6d89f --- /dev/null +++ b/packages/client/tests/unit/hook-context.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest'; + +import { + createAfterResponseContext, + createBeforeRequestContext, + createErrorContext, +} from '../../src/core/hook-context'; +import type { ExecutionContext } from '../../src/core/execution-context'; + +describe('hook-context', () => { + function createExecutionContextMock(): ExecutionContext { + const signal = new AbortController().signal; + + return { + request: { + method: 'GET', + path: '/users', + signal, + }, + url: new URL('https://api.example.com/users'), + headers: { + accept: 'application/json', + 'x-request-id': 'req_123', + }, + attempt: 1, + requestId: 'req_123', + startedAt: 1700000000000, + }; + } + + it('creates beforeRequest context with lifecycle fields', () => { + const execution = createExecutionContextMock(); + + const ctx = createBeforeRequestContext(execution); + + expect(ctx.request).toBe(execution.request); + expect(ctx.url).toBe(execution.url); + expect(ctx.headers).toBe(execution.headers); + expect(ctx.attempt).toBe(1); + expect(ctx.requestId).toBe('req_123'); + expect(ctx.startedAt).toBe(1700000000000); + expect(ctx.signal).toBe(execution.request.signal); + }); + + it('creates afterResponse context with lifecycle fields', () => { + const execution = createExecutionContextMock(); + const response = new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + + const ctx = createAfterResponseContext(execution, response, { ok: true }); + + expect(ctx.response).toBe(response); + expect(ctx.data).toEqual({ ok: true }); + expect(ctx.attempt).toBe(1); + expect(ctx.requestId).toBe('req_123'); + expect(ctx.startedAt).toBe(1700000000000); + expect(ctx.signal).toBe(execution.request.signal); + }); + + it('creates error context with lifecycle fields', () => { + const execution = createExecutionContextMock(); + const error = new Error('boom'); + + const ctx = createErrorContext(execution, error); + + expect(ctx.error).toBe(error); + expect(ctx.attempt).toBe(1); + expect(ctx.requestId).toBe('req_123'); + expect(ctx.startedAt).toBe(1700000000000); + expect(ctx.signal).toBe(execution.request.signal); + }); +}); diff --git a/packages/client/tests/unit/normalize-error.test.ts b/packages/client/tests/unit/normalize-error.test.ts new file mode 100644 index 0000000..077bca5 --- /dev/null +++ b/packages/client/tests/unit/normalize-error.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest'; + +import { normalizeError } from '../../src/core/normalize-error'; +import { HttpError } from '../../src/errors/http-error'; +import { NetworkError } from '../../src/errors/network-error'; +import { RequestAbortedError } from '../../src/errors/request-aborted-error'; +import { TimeoutError } from '../../src/errors/timeout-error'; + +describe('normalizeError', () => { + it('returns HttpError as-is', () => { + const httpError = new HttpError(new Response(null, { status: 500 })); + + expect(normalizeError(httpError, 1000)).toBe(httpError); + }); + + it('returns TimeoutError for timeout aborts', () => { + const abortError = new Error('The operation was aborted'); + abortError.name = 'AbortError'; + + const error = normalizeError(abortError, 1000, 'timeout'); + + expect(error).toBeInstanceOf(TimeoutError); + }); + + it('returns RequestAbortedError for external aborts', () => { + const abortError = new Error('The operation was aborted'); + abortError.name = 'AbortError'; + + const error = normalizeError(abortError, 1000, 'external'); + + expect(error).toBeInstanceOf(RequestAbortedError); + }); + + it('returns NetworkError for other errors', () => { + const error = normalizeError(new Error('socket hang up'), 1000); + + expect(error).toBeInstanceOf(NetworkError); + }); +}); diff --git a/packages/client/tests/unit/should-retry.test.ts b/packages/client/tests/unit/should-retry.test.ts index b147243..0ca4709 100644 --- a/packages/client/tests/unit/should-retry.test.ts +++ b/packages/client/tests/unit/should-retry.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { HttpError } from '../../src/errors/http-error'; import { NetworkError } from '../../src/errors/network-error'; +import { RequestAbortedError } from '../../src/errors/request-aborted-error'; import { shouldRetry } from '../../src/core/should-retry'; import type { RetryConfig } from '../../src/types/config'; import type { RequestMethod } from '../../src/types/request'; @@ -120,4 +121,14 @@ describe('shouldRetry', () => { ), ).toBe(true); }); + + it('does not retry on externally aborted requests', () => { + expect( + shouldRetry( + createParams({ + error: new RequestAbortedError(), + }), + ), + ).toBe(false); + }); });