Caching based on response status

[nurhat]

Hi,
An endpoint with caching enabled caches output even if downstream request returns anything other than httpstatus code with 200. Is it by design, or a bug?

[philproctor]

@nurhat is this happening with 4xx and 5xx requests? Success isn't 200 only.

If so, it sounds like a bug to me -- @TomPallister is there something I'm missing as to why non-success would still be cached?

[nurhat]

@philproctor yes, if you check OutputCacheMiddleware.cs line 55, it does not check http status code. Caching only disabled when http request fails with exception.

[philproctor]

Got it -- I'll keep this open as a bug.

Update by Maintainer on Feb 12, 2025

This is not a bug! This is feature proposal.

[margaale]

I think this was intentional and i wouldn't call it a bug. Why, for example, wouldn't you cache a 404 response if the api behind is telling you that the resource doesn't exist?

Update by Maintainer on Feb 12, 2025

Marcelo, you are absolutely right: this is not a bug. This is new functionality people wanna have in Ocelot: this issue is a feature request.

[philproctor]

404 is a response type that might make sense to cache, but there's also a lot of other 4xx and 5xx responses that don't make sense to cache. Pretty much anything in the 5xx series shouldn't be cached, and some 4xx responses may be made due to current state of the services or similar.

Perhaps if response is not .IsSuccessStatusCode(), we should check against a configured whitelist of cacheable responses.

[margaale]

Yep, it was just an example. You mean configured by the user?

[philproctor]

That's my thought, as part of the cache config.

[margaale]

mmmm, i don't like that. What if someone wants to disable one code for a downstream but not for another?

[philproctor]

I was thinking about the ReRoute cache config, not the global. Maybe something like:

          "FileCacheOptions": {
              "TtlSeconds": 0,
              "Region": "",
              "StatusCodes": [400, 404, 405, 410]
          },

[margaale]

Ohhhhhh that's better, but I think it's going to be kinda painful to declare all the routes explicitly. Maybe some global whitelist configuration with exceptions per route would be easier