From e300bde2fd8e6356227e03a8d9ae847e509cd1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?espen=20dall=C3=B8kken?= Date: Thu, 31 Oct 2024 16:09:15 +0100 Subject: [PATCH] fix: adding @podium/http-client beta --- .github/workflows/test.yml | 2 +- lib/resolver.content.js | 15 ++++++++----- lib/resolver.fallback.js | 14 ++++++++---- lib/resolver.js | 17 +++++++------- lib/resolver.manifest.js | 24 +++++++++++--------- package.json | 2 ++ tests/integration.basic.test.js | 8 ++++++- tests/resolver.manifest.test.js | 2 -- tests/resolver.test.js | 40 +++++++++++++++++++++++++++++++++ 9 files changed, 92 insertions(+), 32 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9bc665c4..d9284bf5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macOS-latest, windows-latest] - node-version: [18.x, 20.x] + node-version: [20.x, 22.x] steps: - uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} diff --git a/lib/resolver.content.js b/lib/resolver.content.js index 1fc1bfce..ff2ba1df 100644 --- a/lib/resolver.content.js +++ b/lib/resolver.content.js @@ -6,9 +6,9 @@ import { Boom, badGateway } from '@hapi/boom'; import { join, dirname } from 'path'; import { fileURLToPath } from 'url'; import fs from 'fs'; +import HttpClient from '@podium/http-client'; import * as utils from './utils.js'; import Response from './response.js'; -import HTTP from './http.js'; import { parseLinkHeaders, filterAssets } from './utils.js'; import { AssetJs, AssetCss } from '@podium/utils'; @@ -25,7 +25,7 @@ const UA_STRING = `${pkg.name} ${pkg.version}`; /** * @typedef {object} PodletClientContentResolverOptions * @property {string} clientName - * @property {import('./http.js').default} [http] + * @property {import('@podium/http-client').default} [http] * @property {import('abslog').AbstractLoggerOptions} [logger] */ @@ -41,9 +41,9 @@ export default class PodletClientContentResolver { */ // @ts-expect-error Deliberate default empty options for better error messages constructor(options = {}) { - this.#http = options.http || new HTTP(); const name = options.clientName; this.#log = abslog(options.logger); + this.#http = options.http || new HttpClient({ logger: options.logger }); this.#metrics = new Metrics(); this.#histogram = this.#metrics.histogram({ name: 'podium_client_resolver_content_resolve', @@ -62,6 +62,7 @@ export default class PodletClientContentResolver { error, ); }); + this.#http.metrics.pipe(this.#metrics); } get metrics() { @@ -153,11 +154,16 @@ export default class PodletClientContentResolver { ); try { + const url = new URL(uri); const { statusCode, headers: hdrs, body, - } = await this.#http.request(uri, reqOptions); + } = await this.#http.request({ + origin: url.origin, + path: url.pathname, + ...reqOptions, + }); const parsedAssetObjects = parseLinkHeaders(hdrs.link); @@ -252,7 +258,6 @@ export default class PodletClientContentResolver { if (outgoing.redirectable && statusCode >= 300) { outgoing.redirect = { statusCode, - // @ts-expect-error TODO: look into what happens if the podlet returns more than one location header location: hdrs && hdrs.location, }; } diff --git a/lib/resolver.fallback.js b/lib/resolver.fallback.js index 11b5ee71..b1b8d423 100644 --- a/lib/resolver.fallback.js +++ b/lib/resolver.fallback.js @@ -3,9 +3,9 @@ import Metrics from '@metrics/client'; import { join, dirname } from 'path'; import { fileURLToPath } from 'url'; import fs from 'fs'; -import HTTP from './http.js'; import { parseLinkHeaders, filterAssets } from './utils.js'; import { AssetJs, AssetCss } from '@podium/utils'; +import HttpClient from '@podium/http-client'; const currentDirectory = dirname(fileURLToPath(import.meta.url)); @@ -20,7 +20,7 @@ const UA_STRING = `${pkg.name} ${pkg.version}`; /** * @typedef {object} PodletClientFallbackResolverOptions * @property {string} clientName - * @property {import('./http.js').default} [http] + * @property {import('@podium/http-client').default} [http] * @property {import('abslog').AbstractLoggerOptions} [logger] */ @@ -36,9 +36,9 @@ export default class PodletClientFallbackResolver { */ // @ts-expect-error Deliberate default empty options for better error messages constructor(options = {}) { - this.#http = options.http || new HTTP(); const name = options.clientName; this.#log = abslog(options.logger); + this.#http = options.http || new HttpClient({ logger: options.logger }); this.#metrics = new Metrics(); this.#histogram = this.#metrics.histogram({ name: 'podium_client_resolver_fallback_resolve', @@ -57,6 +57,7 @@ export default class PodletClientFallbackResolver { error, ); }); + this.#http.metrics.pipe(this.#metrics); } get metrics() { @@ -114,11 +115,16 @@ export default class PodletClientFallbackResolver { this.#log.debug( `start reading fallback content from remote resource - resource: ${outgoing.name} - url: ${outgoing.fallbackUri}`, ); + const url = new URL(outgoing.fallbackUri); const { statusCode, body, headers: resHeaders, - } = await this.#http.request(outgoing.fallbackUri, reqOptions); + } = await this.#http.request({ + origin: url.origin, + path: url.pathname, + ...reqOptions, + }); const parsedAssetObjects = parseLinkHeaders(resHeaders.link); diff --git a/lib/resolver.js b/lib/resolver.js index 63211949..ea657b0b 100644 --- a/lib/resolver.js +++ b/lib/resolver.js @@ -1,7 +1,7 @@ import Metrics from '@metrics/client'; import abslog from 'abslog'; import assert from 'assert'; -import HTTP from './http.js'; +import HttpClient from '@podium/http-client'; import Manifest from './resolver.manifest.js'; import Fallback from './resolver.fallback.js'; import Content from './resolver.content.js'; @@ -34,18 +34,18 @@ export default class PodletClientResolver { ); const log = abslog(options.logger); - const http = new HTTP(); + const httpClient = new HttpClient({ + clientName: options.clientName, + logger: options.logger, + }); this.#cache = new Cache(registry, options); this.#manifest = new Manifest({ clientName: options.clientName, logger: options.logger, - http, - }); - this.#fallback = new Fallback({ ...options, http }); - this.#content = new Content({ - ...options, - http, + http: httpClient, }); + this.#fallback = new Fallback({ ...options, http: httpClient }); + this.#content = new Content({ ...options, http: httpClient }); this.#metrics = new Metrics(); this.#metrics.on('error', (error) => { @@ -55,6 +55,7 @@ export default class PodletClientResolver { ); }); + httpClient.metrics.pipe(this.#metrics); this.#content.metrics.pipe(this.#metrics); this.#fallback.metrics.pipe(this.#metrics); this.#manifest.metrics.pipe(this.#metrics); diff --git a/lib/resolver.manifest.js b/lib/resolver.manifest.js index 0fa5ae98..c159037b 100644 --- a/lib/resolver.manifest.js +++ b/lib/resolver.manifest.js @@ -6,7 +6,7 @@ import * as utils from '@podium/utils'; import { join, dirname } from 'path'; import { fileURLToPath } from 'url'; import fs from 'fs'; -import HTTP from './http.js'; +import HttpClient from '@podium/http-client'; const currentDirectory = dirname(fileURLToPath(import.meta.url)); @@ -22,7 +22,7 @@ const UA_STRING = `${pkg.name} ${pkg.version}`; * @typedef {object} PodletClientManifestResolverOptions * @property {string} clientName * @property {import('abslog').AbstractLoggerOptions} [logger] - * @property {import('./http.js').default} [http] + * @property {import('@podium/http-client').default} [http] */ export default class PodletClientManifestResolver { @@ -37,9 +37,9 @@ export default class PodletClientManifestResolver { */ // @ts-expect-error Deliberate default empty options for better error messages constructor(options = {}) { - this.#http = options.http || new HTTP(); const name = options.clientName; this.#log = abslog(options.logger); + this.#http = options.http || new HttpClient({ logger: this.#log }); this.#metrics = new Metrics(); this.#histogram = this.#metrics.histogram({ name: 'podium_client_resolver_manifest_resolve', @@ -58,6 +58,7 @@ export default class PodletClientManifestResolver { error, ); }); + this.#http.metrics.pipe(this.#metrics); } get metrics() { @@ -97,10 +98,12 @@ export default class PodletClientManifestResolver { ); try { - const response = await this.#http.request( - outgoing.manifestUri, - reqOptions, - ); + const url = new URL(outgoing.manifestUri); + const response = await this.#http.request({ + origin: url.origin, + path: url.pathname, + ...reqOptions, + }); const { statusCode, headers: hdrs, body } = response; // Remote responds but with an http error code @@ -120,11 +123,10 @@ export default class PodletClientManifestResolver { await body.text(); return outgoing; } - + // Body must be consumed; https://github.com/nodejs/undici/issues/583#issuecomment-855384858 + const schema = await body.json(); const manifest = validateManifest( - /** @type {import("@podium/schemas").PodletManifestSchema} */ ( - await body.json() - ), + /** @type {import("@podium/schemas").PodletManifestSchema} */ schema, ); // Manifest validation error diff --git a/package.json b/package.json index 34c3af63..f4034ac2 100644 --- a/package.json +++ b/package.json @@ -39,11 +39,13 @@ "dependencies": { "@hapi/boom": "10.0.1", "@metrics/client": "2.5.4", + "@podium/http-client": "1.0.0-beta.8", "@podium/schemas": "5.1.0", "@podium/utils": "5.4.0", "abslog": "2.4.4", "http-cache-semantics": "^4.0.3", "lodash.clonedeep": "^4.5.0", + "opossum": "^8.3.0", "ttl-mem-cache": "4.1.0", "undici": "6.21.0" }, diff --git a/tests/integration.basic.test.js b/tests/integration.basic.test.js index eb243f06..00c147d4 100644 --- a/tests/integration.basic.test.js +++ b/tests/integration.basic.test.js @@ -393,7 +393,13 @@ tap.test('integration basic - metrics stream objects created', async (t) => { const client = new Client({ name: 'clientName' }); const metrics = []; - client.metrics.on('data', (metric) => metrics.push(metric)); + client.metrics.on('data', (metric) => { + if ( + metric.name !== 'http_client_breaker_events' && + metric.name.indexOf('podium_client') !== -1 + ) + metrics.push(metric); + }); client.metrics.on('end', async () => { t.equal(metrics.length, 3); t.equal(metrics[0].name, 'podium_client_resolver_manifest_resolve'); diff --git a/tests/resolver.manifest.test.js b/tests/resolver.manifest.test.js index 59fe6c07..b8f2665c 100644 --- a/tests/resolver.manifest.test.js +++ b/tests/resolver.manifest.test.js @@ -511,9 +511,7 @@ tap.test( {}, new HttpIncoming({ headers }), ); - await manifest.resolve(outgoing); - t.equal( outgoing.manifest.proxy[0].target, 'http://does.not.mather.com/api/bar', diff --git a/tests/resolver.test.js b/tests/resolver.test.js index b180e1c5..a3517f41 100644 --- a/tests/resolver.test.js +++ b/tests/resolver.test.js @@ -1,7 +1,10 @@ /* eslint-disable no-unused-vars */ import tap from 'tap'; import TtlMemCache from 'ttl-mem-cache'; +import { PodletServer } from '@podium/test-utils'; +import { HttpIncoming } from '@podium/utils'; import Resolver from '../lib/resolver.js'; +import HttpOutgoing from '../lib/http-outgoing.js'; tap.test('resolver() - object tag - should be PodletClientResolver', (t) => { const resolver = new Resolver(new TtlMemCache()); @@ -22,3 +25,40 @@ tap.test( t.end(); }, ); +tap.test( + 'resolver() - emit metrics from the underlying HttpClient', + async (t) => { + const outgoing = new HttpOutgoing({ + name: 'test', + timeout: 1000, + uri: 'http://some.org', + maxAge: Infinity, + }); + + const resolver = new Resolver(new TtlMemCache()); + + function verify(stream) { + return new Promise((resolve) => { + const metrics = []; + stream.on('data', (metric) => { + if (metric.name.indexOf('podium') !== -1) { + metrics.push(metric); + } + }); + stream.on('end', async () => { + t.equal(metrics.length, 1); + t.equal( + metrics[0].name, + 'podium_client_resolver_manifest_resolve', + ); + t.equal(metrics[0].type, 5); + resolve(); + }); + }); + } + await resolver.resolve(outgoing); + resolver.metrics.push(null); + await verify(resolver.metrics); + t.end(); + }, +);