BREAKING CHANGE (behavior): Modify caching to only attempt to update the response cache if a 2xx response code is received from GitHub - Githubissues

octokit / octokit.net

A GitHub API client library for .NET

https://octokitnet.readthedocs.io/en/latest/

MIT License

2.62k stars 1.07k forks source link

BREAKING CHANGE (behavior): Modify caching to only attempt to update the response cache if a 2xx response code is received from GitHub #2877

Closed daverant closed 2 months ago

daverant commented 3 months ago

Resolves https://github.com/octokit/octokit.net/issues/2876

Apologies if I've filed a bug when it should come under feature/enhancement! I couldn't find any docs on response caching, but happy to update them if there are any.

Before the change?

If the CachingHttpClient received a non-200 and non-304 response from GitHub, it still attempts to update the underlying response cache, resulting in a higher number of subsequent cache misses if IResponseCache implementations don't guard for it themselves.

After the change?

Only attempts to update the response cache if a 2xx response code is received from GitHub.

Pull request checklist

[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

I don't believe this is classed as a breaking change, but it is a change in behaviour that could impact users who rely on it calling the response cache for non-200 responses.
[x] Yes
[ ] No

nickfloyd commented 2 months ago

I am going to classify this as a breaking change due to the change in behavior in potential expectations that you point out. This is a great change, thank you for doing it! ❤️