When using cache: true and when the fetcher function (the one that performs the HTTP GET request) throws an error then the error is cached. Upon the next call to client.getSigningKey(kid) (with the same kid) the fetcher function then is not executed again, but the same error object is thrown again.

To reproduce this and narrow this down I have been using this lovely HTTP client test service: httpbin.org, with the following JWKS URL:

http:https://httpbin.org/status/500,501,502,503,504,505,506,400,401,402,403 -- this generates a random response using one of the status codes of those provided in the URL.

Example error for when a 501 Not Implemented response is returned:

2021-08-13T07:59:51.252Z error: NOT IMPLEMENTED: JwksError: NOT IMPLEMENTED
    at JwksClient.getKeys (/snip/node_modules/jwks-rsa/src/JwksClient.js:49:1)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at JwksClient.getSigningKeys (/snip/node_modules/jwks-rsa/src/JwksClient.js:54:1)
    at JwksClient.getSigningKey (/snip/node_modules/jwks-rsa/src/JwksClient.js:72:1)

Let's look at JwksClient.js:49:1. We use the 2.0.4 release, i.e. the request error is (re)thrown in this line of code:
https://github.com/auth0/node-jwks-rsa/blob/v2.0.4/src/JwksClient.js#L49

With cache: false and repetitive execution of const key = await client.getSigningKey(kid); I get alternating errors between attempts. That is, the HTTP request is actually performed for each attempt.

With cache: true and repetitive execution of const key = await client.getSigningKey(kid); I always get the same error thrown, corresponding to the same http error response (probably until the cache item expires). The probability for the testing service to return the same response across N attempts quickly converges to zero with each attempt. So that appeared fishy, and I suspected the error to be cached.

I went a little deeper and used a custom fetcher function (via fetcher: customfunc) to throw an artificial error. I did put a timestamp into the error message. With cache: true and repetitive execution of const key = await client.getSigningKey(kid); the same timestamp popped up in the error thrown at JwksClient.js:49:1. This is an unambiguous indicator that the fetcher function was not executed across attempts, but that it's indeed the error object that was cached and returned/rethrown again.

I had a super brief look at https://github.com/jfromaniello/lru-memoizer and the README / code does not seem to comment on errors.

What was the expected behavior?

Only 'good' results should be cached, i.e. those that appear to be a JSON Web Key Set.

I want to comment on this statement in the current README:

Even if caching is enabled the library will call the JWKS endpoint if the kid is not available in the cache, because a key rotation could have taken place.

This does not seem to happen. When even the initial JWKS endpoint fetch fails with an error then certainly no key for kid is in the cache. The next time that key is required the JWKS endpoint fetch needs to be executed, bypassing the cache. Instead, the error from the cache is re-used. In that sense, the cache worsens situations.

I don't want to be negative here but I still believe that it's worth expressing that I am surprised to find this in such a popular library -- I don't feel comfortable putting this caching solution into production.

Reproduction

Wanted to drop this here quickly -- can provide repro at some other time if absolutely required.

For the repro I used import jwt from "express-jwt"; and

jwt({
  secret: jwksRsa.expressJwtSecret({
    cache: true,
    rateLimit: true,
    jwksRequestsPerMinute: 5,
    jwksUri: "http:https://httpbin.org/status/500,501,502,503,504,505,506,400,401,402,403"
}),

The problem might be in here:

Environment

• Version of this library used: 2.0.4
• Which framework are you using, if applicable: express