From fe8730679c095cb9174d69971c8c05b96114cbb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?espen=20dall=C3=B8kken?= Date: Tue, 29 Oct 2024 14:57:14 +0100 Subject: [PATCH] adding breaker tests++ --- README.md | 61 +++++++++++++++++++- lib/http-client.js | 41 ++++++++++--- tests/http-client.test.js | 117 ++++++++++++++++++++++++-------------- tests/utilities.js | 12 ++++ 4 files changed, 176 insertions(+), 55 deletions(-) create mode 100644 tests/utilities.js diff --git a/README.md b/README.md index 45373bc..77e5a2e 100644 --- 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 index dd2dbdd..4ce2df2 100644 --- 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 index 1ad1d87..d90f37a 100644 --- 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 new file mode 100644 index 0000000..c96fc71 --- /dev/null +++ b/tests/utilities.js @@ -0,0 +1,12 @@ +/** + * Wait a little bit, promise based. + * @property {Number} [waitTime=1000] + * @returns {Promise} + */ +export async function wait(waitTime = 1000) { + return new Promise((resolve) => { + setTimeout(async () => { + resolve(); + }, waitTime); + }); +}