[BUG]: Failed API calls update response cache

[daverant]

What happened?

The response cache in CachingHttpClient is called even when GitHub returns a non-2xx and non-304 status code. It's likely that implementations of IResponseCache will only want to cache data for successful responses. Without this check, it's easy for implementations to miss this check and flush valid responses from the cache when receiving a non-2xx response, resulting in an increase of subsequent cache misses.

This is an opinionated change - it's reasonable to expect implementations to cater for this themselves, but feels like it would help users more than hinder (I can't think of a use case for caching non-successful responses).

Versions

octokit.net v9.1.2, net7.0

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[kfcampbell]

I'm generally okay with this approach. @nickfloyd are you?

I'm also curious how redirect status codes (like a 301 or 307) affect caching.