adding breaker tests++

leftieFriele · leftieFriele · commit fe8730679c09 · 2024-10-29T14:57:27.000+01:00
diff --git a/README.md b/README.md
@@ -4,21 +4,76 @@
 
 Generic http client built on [undici](undici.nodejs.org/) with a circuit breaker, error handling and metrics out of the box.
 
-[![Dependencies](https://img.shields.io/david/podium-lib/http-client.svg)](https://david-dm.org/podium-lib/http-client)
 [![GitHub Actions status](https://github.com/podium-lib/http-client/workflows/Run%20Lint%20and%20Tests/badge.svg)](https://github.com/podium-lib/layout/actions?query=workflow%3A%22Run+Lint+and+Tests%22)
 [![Known Vulnerabilities](https://snyk.io/test/github/podium-lib/http-client/badge.svg)](https://snyk.io/test/github/podium-lib/http-client)
 
 ## Installation
 
+*Note!* Requires Node.js v20 or later.
+
 ```bash
-$ npm install @podium/http-client
+npm install @podium/http-client
 ```
 
 ## Usage
 
+```js
+import client from '@podium/http-client';
+const client = new HttpClient(options);
+
+const response = await client.request({ path: '/', origin: 'https://host.domain' })
+if (response.ok) {
+    //
+}
+```
+
+## API
+
+### Constructor
 
 ```js
+import client from '@podium/http-client';
+
+const client = new HttpClient(options);
+```
+
+#### options
+
+| option     | default | type      | required | details                                                                                                                                    |
+|------------|---------|-----------|----------|--------------------------------------------------------------------------------------------------------------------------------------------|
+| threshold  | `null`  | `number`  | `25`     | Circuit breaker: How many, in %, requests should error before the circuit should trip. Ex; when 25% of requests fail, trip the circuit.    |
+| timeout    | `null`  | `number`  | `500`    | Circuit breaker: How long, in milliseconds, a request can maximum take. Requests exceeding this limit counts against tripping the circuit. |
+| throwOn400 | `false` | `boolean` | `false`  | If the client sahould throw on HTTP 400 errors.If true, HTTP 400 errors will counts against tripping the circuit.                          |
+| throwOn500 | `false` | `boolean` | `true`   | If the client sahould throw on HTTP 500 errors.If true, HTTP 500 errors will counts against tripping the circuit.                          |
+| reset      | `false` | `number`  | `2000`   | Circuit breaker: How long, in milliseconds, to wait before a tripped circuit should be reset.                                              |
+| logger     | `null`  | `àb`      | `false`  | A logger which conform to a log4j interface                                                                                                |
 
+
+##### logger
+
+Any log4j compatible logger can be passed in and will be used for logging.
+Console is also supported for easy test / development.
+
+Example:
+
+```js
+const layout = new Layout({
+    name: 'myLayout',
+    pathname: '/foo',
+    logger: console,
+});
 ```
 
-## Constructor
+Under the hood [abslog] is used to abstract out logging. Please see [abslog] for
+further details.
+
+
+## Methods
+
+### async request(options)
+### async close()
+### fallback()
+
+
+[@metrics/metric]: https://github.com/metrics-js/metric '@metrics/metric'
+[abslog]: https://github.com/trygve-lie/abslog 'abslog'
diff --git a/lib/http-client.js b/lib/http-client.js
@@ -2,6 +2,7 @@ import { Agent, setGlobalDispatcher, request, MockAgent } from 'undici';
 import createError from 'http-errors';
 import Opossum from 'opossum';
 import abslog from 'abslog';
+import EventEmitter from 'node:events';
 
 /**
  * @typedef HttpClientOptions
@@ -17,17 +18,19 @@ import abslog from 'abslog';
  * @property {Number} reset - Circuit breaker: How long, in milliseconds, to wait before a tripped circuit should be reset.
  **/
 
-export default class HttpClient {
+export default class HttpClient extends EventEmitter {
     #throwOn400;
     #throwOn500;
     #breaker;
     #logger;
     #agent;
+    #abortController;
 
     /**
      * @property {HttpClientOptions} options - options
      */
     constructor({
+        abortController = undefined,
         keepAliveMaxTimeout = undefined,
         keepAliveTimeout = undefined,
         connections = 50,
@@ -38,13 +41,20 @@ export default class HttpClient {
         timeout = 500,
         logger = undefined,
         reset = 20000,
+        fallback = undefined,
     } = {}) {
+        super();
         this.#logger = abslog(logger);
         this.#throwOn400 = throwOn400;
         this.#throwOn500 = throwOn500;
+        // Add runtime check
+        this.#abortController = abortController;
 
         // TODO; Can we avoid bind here in a nice way?????
         this.#breaker = new Opossum(this.#request.bind(this), {
+            ...(this.#abortController && {
+                abortController: this.#abortController,
+            }),
             errorThresholdPercentage: threshold, // When X% of requests fail, trip the circuit
             resetTimeout: reset, // After X milliseconds, try again.
             timeout, // If our function takes longer than X milliseconds, trigger a failure
@@ -56,6 +66,17 @@ export default class HttpClient {
             connections,
             pipelining, // TODO unknown option, consider removing
         });
+
+        if (fallback) {
+            this.fallback(fallback);
+        }
+
+        this.#breaker.on('open', () => {
+            this.emit('open');
+        });
+        this.#breaker.on('close', () => {
+            this.emit('close');
+        });
     }
 
     async #request(options = {}) {
@@ -88,8 +109,12 @@ export default class HttpClient {
         };
     }
 
-    fallback(fn) {
-        this.#breaker.fallback(fn);
+    /**
+     * Function called if the request fails.
+     * @param {Function} func
+     */
+    fallback(func) {
+        this.#breaker.fallback(func);
     }
 
     async request(options = {}) {
@@ -103,9 +128,9 @@ export default class HttpClient {
         }
     }
 
-    static mock(origin) {
-        const agent = new MockAgent();
-        setGlobalDispatcher(agent);
-        return agent.get(origin);
-    }
+    // static mock(origin) {
+    //     const agent = new MockAgent();
+    //     setGlobalDispatcher(agent);
+    //     return agent.get(origin);
+    // }
 }
diff --git a/tests/http-client.test.js b/tests/http-client.test.js
@@ -3,91 +3,120 @@ import assert from 'node:assert/strict';
 import http from 'node:http';
 
 import HttpClient from '../lib/http-client.js';
+import { wait } from './utilities.js';
 
 let httpServer,
     host = 'localhost',
     port = 3003;
 
-async function beforeEach() {
+const url = `http://${host}:${port}`;
+
+function beforeEach() {
     httpServer = http.createServer(async (request, response) => {
         response.writeHead(200);
         response.end();
     });
-    httpServer.listen(port, host, () => Promise.resolve());
+    httpServer.listen(port, host);
 }
 
+function closeServer() {
+    return new Promise((resolve) => {
+        httpServer.close(resolve);
+        console.log('Closed');
+    });
+}
 async function afterEach(client) {
     await client.close();
-    await httpServer.close();
+    await closeServer();
 }
 
-test('http-client - basics', async (t) => {
-    await t.test(
-        'http-client: returns 200 response when given valid input',
-        async () => {
-            await beforeEach();
-            const url = `http://${host}:${port}`;
-            const client = new HttpClient();
-            const response = await client.request({
-                path: '/',
-                origin: url,
-                method: 'GET',
-            });
-            assert.strictEqual(response.statusCode, 200);
-            await afterEach(client);
-        },
-    );
-
-    await t.test('does not cause havoc with built in fetch', async () => {
-        await beforeEach();
-        const url = `http://${host}:${port}`;
+async function queryUrl({
+    client,
+    path = '/',
+    url = `http://${host}:${port}`,
+    loop = 2,
+    supresErrors = true,
+}) {
+    for (let i = 0; i < loop; i++) {
+        try {
+            await client.request({ path, origin: url, method: 'GET' });
+        } catch (err) {
+            if (!supresErrors) throw err;
+        }
+    }
+}
+await test('http-client - basics', async (t) => {
+    await t.test('returns 200 response when given valid input', async () => {
+        beforeEach();
         const client = new HttpClient();
-        await fetch(url);
         const response = await client.request({
             path: '/',
             origin: url,
             method: 'GET',
         });
         assert.strictEqual(response.statusCode, 200);
-        await client.close();
-        await fetch(url);
         await afterEach(client);
     });
 
-    await test.skip('http-client: should not invalid port input', async () => {
-        await beforeEach();
-        const url = `http://${host}:3013`;
+    // await t.test('does not cause havoc with built in fetch', async () => {
+    //     beforeEach();
+    //     const client = new HttpClient();
+    //     await fetch(url);
+    //     const response = await client.request({
+    //         path: '/',
+    //         origin: url,
+    //         method: 'GET',
+    //     });
+    //     assert.strictEqual(response.statusCode, 200);
+    //     await client.close();
+    //     await fetch(url);
+    //     await afterEach(client);
+    // });
+    await t.test('can pass in an abort controller', async () => {
+        beforeEach();
+        const abortController = new AbortController();
+
         const client = new HttpClient();
-        await client.request({
-            path: '/',
-            origin: url,
-            method: 'GET',
-        });
         const response = await client.request({
             path: '/',
             origin: url,
             method: 'GET',
         });
-        assert.strictEqual(response.statusCode, 200);
         await afterEach(client);
     });
 });
 
-test.skip('http-client circuit breaker behaviour', async (t) => {
-    await t.test('closes on failure threshold', async () => {
-        await beforeEach();
-        const url = `http://${host}:3014`;
-        const client = new HttpClient({ threshold: 2 });
-        await client.request({
-            path: '/',
-            origin: url,
-            method: 'GET',
+await test('http-client - circuit breaker behaviour', async (t) => {
+    await t.test('opens on failure threshold', async () => {
+        beforeEach();
+        const invalidUrl = `http://${host}:3013`;
+        const client = new HttpClient({ threshold: 50 });
+        let hasOpened = false;
+        client.on('open', () => {
+            hasOpened = true;
+        });
+        await queryUrl({ client, url: invalidUrl });
+
+        assert.strictEqual(hasOpened, true);
+        await afterEach(client);
+    });
+    await t.test('can reset breaker', async () => {
+        beforeEach();
+        const invalidUrl = `http://${host}:3013`;
+        const client = new HttpClient({ threshold: 50, reset: 1 });
+        await queryUrl({ client, url: invalidUrl });
+
+        let hasClosed = false;
+        client.on('close', () => {
+            hasClosed = true;
         });
+        await wait();
         const response = await client.request({
             path: '/',
             origin: url,
             method: 'GET',
         });
+        assert.strictEqual(hasClosed, true);
         assert.strictEqual(response.statusCode, 200);
         await afterEach(client);
     });
diff --git a/tests/utilities.js b/tests/utilities.js
@@ -0,0 +1,12 @@
+/**
+ * Wait a little bit, promise based.
+ * @property {Number} [waitTime=1000]
+ * @returns {Promise<void>}
+ */
+export async function wait(waitTime = 1000) {
+    return new Promise((resolve) => {
+        setTimeout(async () => {
+            resolve();
+        }, waitTime);
+    });
+}
