Skip to content

fix: revalidate when using CacheMode::NoCache - #174

Open
beanow-at-crabnebula wants to merge 2 commits into
06chaynes:mainfrom
beanow-at-crabnebula:fix/cache-mode-conditionals
Open

beanow-at-crabnebula wants to merge 2 commits into
06chaynes:mainfrom
beanow-at-crabnebula:fix/cache-mode-conditionals

Conversation

@beanow-at-crabnebula

@beanow-at-crabnebula beanow-at-crabnebula commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #173

Ensures we're using conditional_fetch as the documentation for NoCache suggests.
While keeping the forced Cache-Control: no-cache request header like the existing behavior.

Includes a regression test on http-cache-reqwest similar to surrounding tests there.

Note: not included in this PR is whether the NoCache comment should be updated to mention the no-cache header being set.

Additionally it doesn't mention that Cache-Control: no-cache is always set when using NoCache.
Which may be good to know when comparing with the Reload option that doesn't set it.

Assisted-by: claude-opus-5-5

@codecov

codecov Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46835% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.19%. Comparing base (b1c3fb4) to head (a0e6f69).
⚠️ Report is 52 commits behind head on main.

Files with missing lines Patch % Lines
http-cache/src/lib.rs 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
+ Coverage   85.87%   90.19%   +4.32%     
==========================================
  Files          19       19              
  Lines        7502    11470    +3968     
==========================================
+ Hits         6442    10345    +3903     
- Misses       1060     1125      +65     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@beanow-at-crabnebula

Copy link
Copy Markdown
Contributor Author

The failed CI check seems to be a test flake.

It's a first_duration < 50ms wall-clock budget for a test that involves disk I/O.
I couldn't locally reproduce that test to fail.

beanow-at-crabnebula added a commit to beanow-at-crabnebula/plugins-workspace that referenced this pull request Sep 24, 2026
`max_ttl` is only a limit there, so responses without `max-age` or `Expires`
get a hard-coded 1 hour `default_ttl`. Not configurable until
06chaynes/http-cache#175 is settled.

`CheckNow` now revalidates with a conditional request, from
06chaynes/http-cache#174.

Assisted-by: claude-opus-5-5

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: CacheMode::NoCache does not revalidate cached responses with conditional requests

1 participant