feat(client): improve retry safety for non-idempotent requests

packages/client/src/core/request.ts

@@ -129,6 +129,7 @@ export async function request<T>(
         attempt: execution.attempt,
         method: execution.request.method,
         retry,
+        idempotencyKey: execution.request.idempotencyKey,
         error,
       });
 

packages/client/src/core/should-retry.ts

@@ -9,10 +9,17 @@ type ShouldRetryParams = {
   attempt: number;
   method: RequestMethod;
   retry: NormalizedRetryConfig;
+  idempotencyKey?: string | undefined;
   error: unknown;
 };
 
-export function shouldRetry({ attempt, method, retry, error }: ShouldRetryParams): boolean {
+export function shouldRetry({
+  attempt,
+  method,
+  retry,
+  idempotencyKey,
+  error,
+}: ShouldRetryParams): boolean {
   if (attempt >= retry.attempts) {
     return false;
   }
@@ -21,6 +28,12 @@ export function shouldRetry({ attempt, method, retry, error }: ShouldRetryParams
     return false;
   }
 
+  const isNonIdempotentMethod = method === 'POST' || method === 'PATCH';
+
+  if (isNonIdempotentMethod && !idempotencyKey) {
+    return false;
+  }
+
   if (error instanceof NetworkError) {
     return retry.retryOn.includes('network-error');
   }

packages/client/tests/integration/retry.test.ts

@@ -106,7 +106,7 @@ describe('client retry', () => {
     expect(fetchMock).toHaveBeenCalledTimes(1);
   });
 
-  it('retries POST when explicitly allowed', async () => {
+  it('retries POST when explicitly allowed and idempotencyKey is provided', async () => {
     const fetchMock = vi
       .fn<typeof fetch>()
       .mockRejectedValueOnce(new Error('socket hang up'))
@@ -128,14 +128,82 @@ describe('client retry', () => {
       },
     });
 
-    const result = await client.post<{ ok: boolean }>('/users', {
-      name: 'John',
-    });
+    const result = await client.post<{ ok: boolean }>(
+      '/users',
+      {
+        name: 'John',
+      },
+      { idempotencyKey: 'idem-123' },
+    );
 
     expect(result).toEqual({ ok: true });
     expect(fetchMock).toHaveBeenCalledTimes(2);
   });
 
+  it('does not retry POST requests without idempotencyKey even when POST is allowed', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      new Response(JSON.stringify({ error: 'server error' }), {
+        status: 500,
+        headers: {
+          'content-type': 'application/json',
+        },
+      }),
+    );
+
+    const client = createClient({
+      baseUrl: 'https://api.example.com',
+      fetch: fetchMock,
+      retry: {
+        attempts: 3,
+        retryMethods: ['POST'],
+        retryOn: ['5xx'],
+        baseDelayMs: 0,
+      },
+    });
+
+    await expect(client.post('/payments', { amount: 100 })).rejects.toThrow();
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('retries POST requests with idempotencyKey when POST is allowed', async () => {
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValueOnce(
+        new Response(JSON.stringify({ error: 'server error' }), {
+          status: 500,
+          headers: {
+            'content-type': 'application/json',
+          },
+        }),
+      )
+      .mockResolvedValueOnce(
+        new Response(JSON.stringify({ ok: true }), {
+          status: 200,
+          headers: {
+            'content-type': 'application/json',
+          },
+        }),
+      );
+
+    const client = createClient({
+      baseUrl: 'https://api.example.com',
+      fetch: fetchMock,
+      retry: {
+        attempts: 2,
+        retryMethods: ['POST'],
+        retryOn: ['5xx'],
+        baseDelayMs: 0,
+      },
+    });
+
+    await expect(
+      client.post('/payments', { amount: 100 }, { idempotencyKey: 'idem-123' }),
+    ).resolves.toEqual({ ok: true });
+
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+  });
+
   it('request retry config overrides client retry config', async () => {
     const fetchMock = vi.fn<typeof fetch>(async () => {
       return new Response(JSON.stringify({ message: 'Service Unavailable' }), {

packages/client/tests/unit/should-retry.test.ts

@@ -41,6 +41,7 @@ function createParams(
     attempt: number;
     method: RequestMethod;
     retry: Required<RetryConfig>;
+    idempotencyKey?: string;
     error: unknown;
   }> = {},
 ) {
@@ -120,21 +121,41 @@ describe('shouldRetry', () => {
     ).toBe(false);
   });
 
-  it('retries POST when explicitly allowed', () => {
+  it('retries POST when explicitly allowed and idempotencyKey is provided', () => {
     expect(
       shouldRetry(
         createParams({
           method: 'POST',
+          idempotencyKey: 'idem-123',
           retry: {
             ...defaultRetry,
-            retryMethods: ['GET', 'PUT', 'DELETE', 'POST'],
+            attempts: 3,
+            retryMethods: ['POST'],
+            retryOn: ['network-error'],
           },
-          error: new NetworkError(),
+          error: new NetworkError('Network request failed', new Error('socket hang up')),
         }),
       ),
     ).toBe(true);
   });
 
+  it('does not retry POST without idempotencyKey even when explicitly allowed', () => {
+    expect(
+      shouldRetry(
+        createParams({
+          method: 'POST',
+          retry: {
+            ...defaultRetry,
+            attempts: 3,
+            retryMethods: ['POST'],
+            retryOn: ['network-error'],
+          },
+          error: new NetworkError('Network request failed', new Error('socket hang up')),
+        }),
+      ),
+    ).toBe(false);
+  });
+
   it('does not retry on externally aborted requests', () => {
     expect(
       shouldRetry(
@@ -154,4 +175,74 @@ describe('shouldRetry', () => {
       ),
     ).toBe(false);
   });
+
+  it('does not retry POST requests without idempotencyKey', () => {
+    expect(
+      shouldRetry(
+        createParams({
+          method: 'POST',
+          retry: {
+            ...defaultRetry,
+            attempts: 3,
+            retryMethods: ['POST'],
+            retryOn: ['5xx'],
+          },
+          error: createHttpError(500),
+        }),
+      ),
+    ).toBe(false);
+  });
+
+  it('retries POST requests with idempotencyKey when method and condition are allowed', () => {
+    expect(
+      shouldRetry(
+        createParams({
+          method: 'POST',
+          idempotencyKey: 'idem-123',
+          retry: {
+            ...defaultRetry,
+            attempts: 3,
+            retryMethods: ['POST'],
+            retryOn: ['5xx'],
+          },
+          error: createHttpError(500),
+        }),
+      ),
+    ).toBe(true);
+  });
+
+  it('does not retry PATCH requests without idempotencyKey', () => {
+    expect(
+      shouldRetry(
+        createParams({
+          method: 'PATCH',
+          retry: {
+            ...defaultRetry,
+            attempts: 3,
+            retryMethods: ['PATCH'],
+            retryOn: ['5xx'],
+          },
+          error: createHttpError(500),
+        }),
+      ),
+    ).toBe(false);
+  });
+
+  it('retries PATCH requests with idempotencyKey when method and condition are allowed', () => {
+    expect(
+      shouldRetry(
+        createParams({
+          method: 'PATCH',
+          idempotencyKey: 'idem-123',
+          retry: {
+            ...defaultRetry,
+            attempts: 3,
+            retryMethods: ['PATCH'],
+            retryOn: ['5xx'],
+          },
+          error: createHttpError(500),
+        }),
+      ),
+    ).toBe(true);
+  });
 });