Implement a first beta to replace the existing podium http client

[leftieFriele]

Goal

Replace client in Podium internals with this module.

Design goal: It should pretty much function like a regular fetch call

const httpClient = new HttpClient();
const response = await client.request({ /*options*/ });

// Catching

const httpClient = new HttpClient();
try {
 const response = await client.request({ /*options*/ });
} catch (err) {
 if (err === /breaker is open/) {
  // Handle closed circuit breaker
 }
}

Rough outline

• Expand the circuit breaking behaviour and add more tests
• Enabled using an AbortController
• Write propper documentation
• Add types
• clean up test code
• Do some benchmarking

[digitalsadhu]

I did a theoretical refactor of the content resolver to use this new http client.
It fits in pretty well. The most friction may be around the fallback and throwable 400/500s perhaps but generally looks to me like it should be easy enough to replace the existing client.

export default class PodletClientContentResolver {
    #http;
    ...
    constructor(options = {}) {
        this.#http = new HttpClient({
          timeout: options.timeout,
          throwOn400: options.throwable, // I think we only want to set these to true when throwable is true... maybe
          throwOn500: options.throwable,
        });
        ...
    }

    async resolve(outgoing) {
        // this is going to have to be set per request since its not available when the content resource is
        // first constructed. We also want this to be up to date say if the manifest is re-fetched.
        // will setting this on every request work? Presumably it holds on to and uses whatever fallback is
        // set until a new one is provided so if the circuit is open it will use whatever fallback is set until the
        // circuit goes half-open and a new request goes out?
        this.#http.fallback(outgoing.fallback);
        
        ...
        const headers = {
            ...outgoing.reqOptions.headers,
            'User-Agent': UA_STRING,
        };
        const uri = putils.uriBuilder(
            outgoing.reqOptions.pathname,
            outgoing.contentUri,
        );
        const reqOptions = {
            rejectUnauthorized: outgoing.rejectUnauthorized,
            method: 'GET',
            query: outgoing.reqOptions.query,
            headers,
        };
        ...
        try {
            const {
                statusCode,
                headers: hdrs,
                body,
            } = await this.#http.request(uri, reqOptions);

            ...
            // lots of handling of failure cases and pushing of fallback
            ...

           // emit beforeStream headers and assets
           outgoing.emit(
                'beforeStream',
                new Response({
                    headers: outgoing.headers,
                    js: utils.filterAssets('content', outgoing.manifest.js),
                    css: utils.filterAssets('content', outgoing.manifest.css),
                    redirect: outgoing.redirect,
                }),
            );

            // pipe the body into outgoing which a stream itself
            pipeline([body, outgoing], (err) => {
                if (err) {
                    this.#log.warn('error while piping content stream', err);
                }
            });

            // return outgoing and we are done.
            return outgoing;
        } catch (err) {
            // do some stuff (metrics etc) then rethrow
            // we dont want the fallback for this case
        }
        ...
    }
}

[digitalsadhu]

Really cool that you are looking at this. I was playing around with circuit breaking on Friday while on the plane so your timing is impeccable.

[leftieFriele]

👍 nice, doesn't look half bad does it?

.fallback is optional, so in this case you just wouldn't set it and deal with the error thrown when the breaker opens.

One thing we need to consider is wether we wrap it in our own errors, I think that would probably make sense.

looking at your sample, it seems like events coming from the breaker isn't really interesting 🤔

[digitalsadhu]

👍 nice, doesn't look half bad does it?

yup!

.fallback is optional, so in this case you just wouldn't set it and deal with the error thrown when the breaker opens.

true

One thing we need to consider is wether we wrap it in our own errors, I think that would probably make sense.

looking at your sample, it seems like events coming from the breaker isn't really interesting 🤔

I don't think so...

[github-actions]

🎉 This issue has been resolved in version 1.0.0-beta.2 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 1.0.0-beta.3 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 1.0.0-beta.4 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 1.0.0-beta.5 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 1.0.0-beta.6 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀