Any plans to support the Vary header? We're using https://github.com/gitterHQ/request-http-cache precisely for that reason, but could adopt this more featured client if there were good support for the vary header and stale-if-error.

Hi,

I installed the v.3.1.1 and we have an issue in our release package, the current ./core is missing in our release

Will cause error with import

import { RequestOptions } from 'flashheart/dist/core/restClient';

Add support for rate limiting in V3.

Rate limiting middleware:
https://github.com/bbc/http-transport-rate-limiter

We are trying to pass Client browser's signature (request headers) to our back-end application. As of now what we are observing is the flashheart is overriding the User-Agent header.
How can we disable flashheart to overwrite certain headers?

The option object passed on createClient should not contain request settings directly.
timeout and User-Agent can be passed on options.default so it's clear that request settings are being overwritten.

advisory

└─┬ [email protected]
  ├─┬ [email protected]
  │ └── [email protected] 
  ├─┬ [email protected]
  │ └─┬ [email protected]
  │   ├── [email protected]  deduped
  │   └─┬ [email protected]
  │     └── [email protected]  deduped
  └─┬ [email protected]
    ├─┬ [email protected]
    │ └── [email protected]  deduped
    └── [email protected]  deduped

reported on levee & request.

Wreck current version is 14.0.2, which depends on [email protected].

So this repo is waiting on levee & request, but updating wreck would resolve.

We (Sounds web) are getting vulnerability alerts in rms-client package (https://github.com/bbc/rms-client). When running yarn audit it points to flashheart's dependencies. Is it possible to update these dependecies? Thanks

In lib/client.js, the stats for number of attempts are send as a timing stat.

I have some problem when i run "npm start"

Error : Error: Cannot find module './lib/collapse'

When a ~400 error condition is detected the response 'body' is not returned in the usual manner.

I honestly don't know if this is the correct approach for flashheart or not. However, it's not uncommon for an api to return information about an error in the request body. Because Error.prototype.toStack() method just dumps the stack, the only way to know that the information you're looking for is stored in a body property on err is to look through the source.

Not possible?

Should probably start using const, let etc. even though FH is going to still be written in ES5.

My use case is that I want to retry until a 200 status code is received (this is because the 200 code depends on another data set turning from "pending" to "approved"). Is this possible with this library? It seems like this would be a straightforward feature to add, so I am curious.

It would be nice to change the logging format of Flashheart.

For data/log visualisers that leverage JSON logs like Kibana, having the upstream access log as a single message becomes hard to parse, i.e.

GET http://echo.jsontest.com/hello/world 200 10 ms

Libraries like Morgan allow for custom format functions, e.g.

morgan((tokens, req, res) => tokens)

Which would return the JSON object of all of the tokens that Morgan monitors.

Since Flashheart allows you to inject your own logger, it makes sense (to me at least) to be able to define your own formatter to return the right thing for the logger to consume.

This could also give users access to more information that's on the request and response options, I would like to be able to define something like

const format = (req, res) => ({ short: `${req.method} - ${res.statusCode}`, time: res.elapsedTime })

createClient({ format, logger })

And this could easily be done here with something like;

Client.prototype._log = function (res) {
  var debugMessage = util.format(res.request.method, res.request.href, res.statusCode, res.elapsedTime, 'ms');

  debug(debugMessage);

  if (this.logger) {
    var logMessage = this.format 
      ? this.format(res,request, res) 
      : debugMessage

    this.logger.info(logMessage);
  }
};

Assuming that you still want the debug message to be string only.

What do you guys think?

Description
It's great that we can override the name of our client on a per-request basis.
This enables us to have a single-configured client which we can use for multiple feeds. However we do wish to monitor the cache-hits and misses of each individual feed, and it seems currently flashheart doesn't allow that.

Problem
When it comes to the cachingClient, the feedname isn't taken into account.
I've added a PR (attached) which fixes that, here is an example..

http.cache.misses
GET https://navigation.api.bbci.co.uk/api 200 147 ms
http.Orbit.requests
http.Orbit.responses.200
http.Orbit.response_time 147
http.Orbit.attempts 1
http.cache.misses
GET https://radio-nav-service.api.bbci.co.uk/radio-nav-service/json 200 414 ms
http.RadioNavService.requests
http.RadioNavService.responses.200
http.RadioNavService.response_time 414
http.RadioNavService.attempts 1
http.cache.hits
http.cache.hits

You can see that cache.hits and cache.misses don't include the feed name, so i can't tell here whether its RadioNavService or Orbit which is hitting the cache

In v2 we used to be able to pass in the encoding option to get requests to have request return the data back as a Buffer instead of a string. That option is not supported in v3. Instead the docs say to set the json request option to false:

If you need to call an API that returns plain text, XML, animated GIFs etc. then set the json flag to false in your request options.

However, this option isn't even implemented, and if it were implemented it still wouldn't work.

Adding:

if (opts && opts.encoding !== undefined) {
    req.use(httpTransport.setContextProperty({
        encoding: opts.encoding
    }, 'opts'));
}

to the get method in src/httpTransport/client.ts with the option encoding set to null causes body to be returned as a Buffer instead of a string - allowing support for binary data. However, it would be great to add support in a more generic fashion for additional request options.

Thanks.

Allow a flag (simple is used by request-promise https://github.com/request/request-promise#api-in-detail which defaults to true) which prevents formatting errors with a new message, body, etc. when the status code family != 2xx.

Motivation

Currently we (BBC ID) use our own https://github.com/request/request wrapper, https://github.com/bbc/id-http-client.

We began constructing this before we were aware of Flashheart's existence, and we seem to slowly be bringing it up to feature parity with Flashheart due to product/resilience needs. We'd rather not duplicate these non-product specific features in BBC estate

Ideally we'd be able to instead wrap Flashheart to gain circuit breakers, retries, rate limiting and built-in-cache, and roll our own stats and logs as we see fit (for example for statsd we use datadoghq.com, and the statsd protocol wrapper dogstatsd).

Problem

Currently we're unable to wrap Flashheart due to the change in error format from standard request. We currently have the chain of request decoration, request -> bbc-http-client-node -> request-promise -> id-http-client, request-promise or id-http-client would be unable to interpret the reformatted errors passed via 'errorback'.

If these could be returned as normal, without filtering by statusCode etc, we could in theory consume Flashheart (request -> bbc-http-client-node -> flashheart -> request-promise -> id-http-client) while keeping the same API, with new errors thrown by Flashheart falling into request-promise' RequestError prototype.

Currently, clients have to configure external cache and pass it to flashheart. We could set Redis as the default external cache and allow clients to override this. Any thoughts / suggestions? Thanks.

Currently the client is created synchronously. In particular, the start function of the Catbox instance is passed noop rather than a callback. This has the potential to cause a race condition while the client attempts to connect to Redis when using catbox-redis.

My feeling is that the whole create client function should be async for consistency, but this obviously would be a breaking change and is generally unnecessary when using memory caches.

For this reason I propose to create a createClientAsync function and make that available to use separately. I'll raise a PR for you to review.

Allow http client to be configured via the Transport interface.

Add an option to set a default cache time, for if caching is enabled and no max-age is provided.

Migrate to Typescript. Version 3 must retain the existing functionality, with the additional features:

1. Support configurable certs
2. Support configurable HTTP clients
3. Request collapsing

We'd like to pass in an array of HTTP status codes (when the client is created) that might trigger a retry (non error codes). Would you guys be OK with this?

Happy to do the work myself.

We want to be able to monitor when we make a cold call to an API, but not monitor cached calls. It's hard for us to know when a response is coming from a cache or whether it comes from the real thing (as this logic is buried within the API).

Are you able to expose some flag to say whether the response was cached or not? Or is this already possible and i don't know how to do it pings @nspragg

It would be nice if we could use the stats field, but we use cloudwatch and i don't know if the StatsD stuff works with that

Question
Any plans to move away from using request?

• Use Github pages
• Wiki
• Roadmap
• README (concise)

Include query string details in logs

Hi @nspragg,

Have type error in our current logger when try to use the console for logger https://github.com/bbc/flashheart/blob/master/src/core/clientParams.ts#L20

interface LogMethod {
  (message: string, callback: LogCallback): Logger;
  (message: string, meta: any, callback: LogCallback): Logger;
  (message: string, ...meta: any[]): Logger;
  (infoObject: object): Logger;
}

Do I need to do have custom class to implement the Logger interface?

I am trying to use this package with request-promise for requests that look more like this:

client.post(url, body).then(res => console.log(res));

I'm not able to get this working, but is there any recommended usage with Promises? Thank you.

Hey guys.

I'm using the getAsync function and i'm wondering if its possible to get the full response passed into the .then?

Here's my code:

this.restClient.getAsync(url, options)
   .then((body) => {
      ....
    })
   .catch((err) => {
      ....
   });

If I was using the callback based approach, I could do:

this.restClient.get(url, (err, body, res) => {
    ....
});

Which would give me access to the response object (including headers and status code). Is it possible to get a handle of this object using getAsync?

Thanks.

We do not currently support HEAD requests

Possible options:

• Allow for a cert object/reference to be passed in the constructor.
• Configure cert in a "transport" client (delegate) passed to the constructor.