Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - migrating to the new http client #432

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
15 changes: 10 additions & 5 deletions lib/resolver.content.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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]
*/

Expand All @@ -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',
Expand All @@ -62,6 +62,7 @@ export default class PodletClientContentResolver {
error,
);
});
this.#http.metrics.pipe(this.#metrics);
}

get metrics() {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
};
}
Expand Down
14 changes: 10 additions & 4 deletions lib/resolver.fallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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]
*/

Expand All @@ -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',
Expand All @@ -57,6 +57,7 @@ export default class PodletClientFallbackResolver {
error,
);
});
this.#http.metrics.pipe(this.#metrics);
}

get metrics() {
Expand Down Expand Up @@ -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);

Expand Down
17 changes: 9 additions & 8 deletions lib/resolver.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) => {
Expand All @@ -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);
Expand Down
24 changes: 13 additions & 11 deletions lib/resolver.manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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 {
Expand All @@ -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',
Expand All @@ -58,6 +58,7 @@ export default class PodletClientManifestResolver {
error,
);
});
this.#http.metrics.pipe(this.#metrics);
}

get metrics() {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
8 changes: 7 additions & 1 deletion tests/integration.basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 0 additions & 2 deletions tests/resolver.manifest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
40 changes: 40 additions & 0 deletions tests/resolver.test.js
Original file line number Diff line number Diff line change
@@ -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());
Expand All @@ -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();
},
);