From bdc64df63646e6795ff020b688b28244b5adfe14 Mon Sep 17 00:00:00 2001
From: Tom Laplace-piedplat <tom.laplace-piedplat@pole-emploi.fr>
Date: Thu, 12 Dec 2024 12:11:36 +0100
Subject: [PATCH] feat: 500+ error handling

---
 src/request.ts                        | 16 +++-----------
 tests/response/error_handling.spec.ts | 32 ++++++++++++++++++++-------
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/request.ts b/src/request.ts
index c787337..7c6a0ef 100644
--- a/src/request.ts
+++ b/src/request.ts
@@ -24,13 +24,7 @@ import {
   SuperAgentSerializer,
   ApiRequestHooks,
 } from './types.js'
-import {
-  dumpRequest,
-  dumpRequestBody,
-  dumpRequestCookies,
-  dumpRequestHeaders,
-  stackToError,
-} from './utils.js'
+import { dumpRequest, dumpRequestBody, dumpRequestCookies, dumpRequestHeaders } from './utils.js'
 
 const DUMP_CALLS = {
   request: dumpRequest,
@@ -176,13 +170,9 @@ export class ApiRequest extends Macroable {
       }
 
       /**
-       * Raise exception when received 500 status code from the server
+       * For all HTTP errors (including 500+), return the error response
+       * This allows proper handling of server errors via ApiResponse
        */
-      if (error.response.status >= 500) {
-        await this.#setupRunner.cleanup(error, this)
-        throw stackToError(error.response.text)
-      }
-
       response = error.response
     }
 
diff --git a/tests/response/error_handling.spec.ts b/tests/response/error_handling.spec.ts
index 5fb1526..b5e7dbd 100644
--- a/tests/response/error_handling.spec.ts
+++ b/tests/response/error_handling.spec.ts
@@ -37,18 +37,34 @@ test.group('Response | error handling', (group) => {
     assert.equal(response.status(), 401)
   })
 
-  test('raise fatal errors raised by the server', async ({ assert }) => {
+  test('returns ApiResponse for 500 errors', async ({ assert }) => {
     httpServer.onRequest((_, res) => {
-      try {
-        throw new Error('Something went wrong')
-      } catch (error) {
-        res.statusCode = 500
-        res.end(error.stack)
-      }
+      res.statusCode = 500
+      res.end('Internal server error')
+    })
+
+    const request = new ApiRequest({ baseUrl: httpServer.baseUrl, method: 'GET', endpoint: '/' })
+    const response = await request
+
+    assert.equal(response.status(), 500)
+    assert.isTrue(response.hasFatalError())
+    assert.isTrue(response.hasServerError())
+  })
+
+  test('handles server errors with response body', async ({ assert }) => {
+    httpServer.onRequest((_, res) => {
+      res.statusCode = 500
+      res.setHeader('Content-Type', 'application/json')
+      res.end(JSON.stringify({ error: 'Something went wrong', code: 'INTERNAL_ERROR' }))
     })
 
     const request = new ApiRequest({ baseUrl: httpServer.baseUrl, method: 'GET', endpoint: '/' })
+    const response = await request
 
-    await assert.rejects(() => request, 'Error: Something went wrong')
+    assert.equal(response.status(), 500)
+    assert.deepEqual(response.body(), {
+      error: 'Something went wrong',
+      code: 'INTERNAL_ERROR',
+    })
   })
 })