fix: removing custom error

podium-lib · Nov 1, 2024 · f071157 · f071157
1 parent 1aab57c
commit f071157
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 44 deletions.
diff --git a/README.md b/README.md
@@ -156,10 +156,6 @@ For a complete list of options, consult the [undici documentation](https://undic
 
 Closes the client and it's connections.
 
-## HttpClientError
-
-Errors thrown from the library will be of the class `HttpClientError`
-
 [@metrics/metric]: https://github.com/metrics-js/metric '@metrics/metric'
 [abslog]: https://github.com/trygve-lie/abslog 'abslog'
 [undici]: https://undici.nodejs.org/

diff --git a/examples/failing.js b/examples/failing.js
@@ -1,4 +1,4 @@
-import HttpClient, { HttpClientError } from '../lib/http-client.js';
+import HttpClient from '../lib/http-client.js';
 
 /**
  * Example of the circuit breaker opening on a failing request.
@@ -22,9 +22,7 @@ try {
         method: 'GET',
     });
 } catch (err) {
-    if (err instanceof HttpClientError) {
-        if (err.code === HttpClientError.ServerDown) {
-            console.error('Server unavailable..');
-        }
+    if (err.toString() === 'Error: Breaker is open') {
+        console.error('Server unavailable..');
     }
 }
diff --git a/lib/http-client.js b/lib/http-client.js
@@ -23,6 +23,7 @@ export default class HttpClient {
     #agent;
     #breaker;
     #followRedirects;
+    // eslint-disable-next-line no-unused-private-class-members
     #hasFallback = false;
     #logger;
     #throwOn400;
@@ -84,7 +85,7 @@ export default class HttpClient {
     }
 
     async #request(options = {}) {
-        if (this.#followRedirects) {
+        if (this.#followRedirects || options.redirectable) {
             const { redirect } = interceptors;
             options.dispatcher = this.#agent.compose(
                 redirect({ maxRedirections: 1 }),
@@ -97,7 +98,13 @@ export default class HttpClient {
             ...options,
         });
 
-        if (this.#throwOn400 && statusCode >= 400 && statusCode <= 499) {
+        // TODO Look into this, as the resovlers have their own handling of this
+        // In order to be backward compatible, both throw options must be false
+        if (
+            (this.#throwOn400 || options.throwable) &&
+            statusCode >= 400 &&
+            statusCode <= 499
+        ) {
             // Body must be consumed; https://github.com/nodejs/undici/issues/583#issuecomment-855384858
             const errBody = await body.text();
             this.#logger.trace(
@@ -106,9 +113,12 @@ export default class HttpClient {
             throw createError(statusCode);
         }
 
-        if (this.#throwOn500 && statusCode >= 500 && statusCode <= 599) {
+        if (
+            (this.#throwOn500 || options.throwable) &&
+            statusCode >= 500 &&
+            statusCode <= 599
+        ) {
             // Body must be consumed; https://github.com/nodejs/undici/issues/583#issuecomment-855384858
-            await body.text();
             const errBody = await body.text();
             this.#logger.trace(
                 `HTTP ${statusCode} error catched by client. Body: ${errBody}`,
@@ -138,20 +148,7 @@ export default class HttpClient {
      * @returns {Promise<any>}
      */
     async request(options = {}) {
-        try {
-            return await this.#breaker.fire(options);
-        } catch (error) {
-            if (!this.#hasFallback) {
-                throw new HttpClientError(
-                    `Error on ${options.method} ${options.origin}${options.path}`,
-                    {
-                        code: error.code,
-                        cause: error,
-                        options,
-                    },
-                );
-            }
-        }
+        return await this.#breaker.fire(options);
     }
 
     /**
@@ -169,15 +166,15 @@ export default class HttpClient {
 /**
  * Error class for the client
  */
-export class HttpClientError extends Error {
-    // Not sure if there is a need for this tbh, but I threw it in there so we can see how it feels.
-    static ServerDown = 'EOPENBREAKER';
-
-    constructor(message, { code, cause, options }) {
-        super(message);
-        this.name = 'HttpClientError';
-        this.code = code;
-        this.cause = cause;
-        this.options = options;
-    }
-}
+// export class HttpClientError extends Error {
+//     // Not sure if there is a need for this tbh, but I threw it in there so we can see how it feels.
+//     static ServerDown = 'EOPENBREAKER';
+//
+//     constructor(message, { code, cause, options }) {
+//         super(message);
+//         this.name = 'HttpClientError';
+//         this.code = code;
+//         this.cause = cause;
+//         this.options = options;
+//     }
+// }
diff --git a/tests/http-client.test.js b/tests/http-client.test.js
@@ -40,11 +40,11 @@ async function queryUrl({
             await client.request({ path, origin: url, method: 'GET' });
         } catch (err) {
             // Push the actual cause here for the tests
-            if (!suppressErrors) errors.push(err.cause);
+            if (!suppressErrors) errors.push(err);
         }
     }
     if (errors.length > 0) {
-        throw new Error(errors.toString());
+        throw new Error(errors);
     }
 }
 await test('http-client - basics', async (t) => {
@@ -90,8 +90,7 @@ await test('http-client - basics', async (t) => {
                 });
             },
             {
-                name: 'HttpClientError',
-                message: 'Error on GET https://does-not-exist.domain/',
+                name: 'Error',
             },
         );
         await client.close();
@@ -210,7 +209,7 @@ await test('http-client - circuit breaker behaviour', async (t) => {
                     method: 'GET',
                 });
             } catch (err) {
-                if (err.cause.code === 'EOPENBREAKER') {
+                if (err.toString() === 'Error: Breaker is open') {
                     broken++;
                 }
             }
@@ -222,6 +221,7 @@ await test('http-client - circuit breaker behaviour', async (t) => {
         );
         await afterEach(client);
     });
+
     await t.test('can reset breaker', async () => {
         beforeEach();
         const invalidUrl = `http://${host}:3023`;
@@ -250,4 +250,7 @@ await test('http-client - circuit breaker behaviour', async (t) => {
         assert.strictEqual(response.statusCode, 200);
         await afterEach(client);
     });
+    // await t.test('has a .metrics property', async () => {
+    //     const client = new HttpClient({ threshold: 50, reset: breakerReset });
+    // });
 });