Python
/
uv
mirror of https://github.com/astral-sh/uv
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve HTTP response caching log messages (#15067) 

"Cached request ... is not storable" doesn't make sense from a user
perspective, it's leaking our internal `CachedClient` abstraction. I
think it makes more sense to talk about this as "Response from ... is
not storable"
This commit is contained in:
Zanie Blue 2025-08-05 14:34:12 -05:00 committed by GitHub
parent 3f83390e34
commit b2e7b2b279
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 27 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/uv-client/src/cached_client.rs
View File

@ -517,7 +517,7 @@ impl CachedClient {
BeforeRequest::NoMatch => { BeforeRequest::NoMatch => {
// This shouldn't happen; if it does, we'll override the cache. // This shouldn't happen; if it does, we'll override the cache.
warn!( warn!(
"Cached request doesn't match current request for: {}", "Cached response doesn't match current request for: {}",
req.url() req.url()
); );
let (response, cache_policy) = self.fresh_request(req, cache_control).await?; let (response, cache_policy) = self.fresh_request(req, cache_control).await?;

52
crates/uv-client/src/httpcache/mod.rs
View File

@ -563,7 +563,7 @@ impl ArchivedCachePolicy {
ArchivedMethod::Get | ArchivedMethod::Head ArchivedMethod::Get | ArchivedMethod::Head
) { ) {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because of its method {:?}", "Response from {} is not storable because of the request method {:?}",
self.request.uri, self.request.uri,
self.request.method self.request.method
); );
@ -575,8 +575,8 @@ impl ArchivedCachePolicy {
// below, but we can bail out early here. // below, but we can bail out early here.
if !self.response.has_final_status() { if !self.response.has_final_status() {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because its response has \ "Response from {} is not storable because it has \
non-final status code {:?}", a non-final status code {:?}",
self.request.uri, self.request.uri,
self.response.status, self.response.status,
); );
@ -591,8 +591,8 @@ impl ArchivedCachePolicy {
// itself. // itself.
if self.response.status == 206 || self.response.status == 304 { if self.response.status == 206 || self.response.status == 304 {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because its response has \ "Response from {} is not storable because it has \
unsupported status code {:?}", an unsupported status code {:?}",
self.request.uri, self.request.uri,
self.response.status, self.response.status,
); );
@ -605,7 +605,7 @@ impl ArchivedCachePolicy {
// S3.) // S3.)
if self.request.headers.cc.no_store { if self.request.headers.cc.no_store {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because its request has \ "Response from {} is not storable because its request has \
a 'no-store' cache-control directive", a 'no-store' cache-control directive",
self.request.uri, self.request.uri,
); );
@ -614,7 +614,7 @@ impl ArchivedCachePolicy {
// "the no-store cache directive is not present in the response" // "the no-store cache directive is not present in the response"
if self.response.headers.cc.no_store { if self.response.headers.cc.no_store {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because its response has \ "Response from {} is not storable because it has \
a 'no-store' cache-control directive", a 'no-store' cache-control directive",
self.request.uri, self.request.uri,
); );
@ -631,8 +631,8 @@ impl ArchivedCachePolicy {
// private). // private).
if self.response.headers.cc.private { if self.response.headers.cc.private {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because this is a shared \ "Response from {} is not storable because this is a shared \
cache and its response has a 'private' cache-control directive", cache and has a 'private' cache-control directive",
self.request.uri, self.request.uri,
); );
return false; return false;
@ -642,7 +642,7 @@ impl ArchivedCachePolicy {
// explicitly allows shared caching" // explicitly allows shared caching"
if self.request.headers.authorization && !self.allows_authorization_storage() { if self.request.headers.authorization && !self.allows_authorization_storage() {
tracing::trace!( tracing::trace!(
"Cached request {} is not storable because this is a shared \ "Response from {} is not storable because this is a shared \
cache and the request has an 'Authorization' header set and \ cache and the request has an 'Authorization' header set and \
the response has indicated that caching requests with an \ the response has indicated that caching requests with an \
'Authorization' header is allowed", 'Authorization' header is allowed",
@ -657,7 +657,7 @@ impl ArchivedCachePolicy {
// "a public response directive" // "a public response directive"
if self.response.headers.cc.public { if self.response.headers.cc.public {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because its response has \ "Response from {} is storable because it has \
a 'public' cache-control directive", a 'public' cache-control directive",
self.request.uri, self.request.uri,
); );
@ -666,8 +666,8 @@ impl ArchivedCachePolicy {
// "a private response directive, if the cache is not shared" // "a private response directive, if the cache is not shared"
if !self.config.shared && self.response.headers.cc.private { if !self.config.shared && self.response.headers.cc.private {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because this is a shared cache \ "Response from {} is storable because this is a shared cache \
and its response has a 'private' cache-control directive", and has a 'private' cache-control directive",
self.request.uri, self.request.uri,
); );
return true; return true;
@ -675,7 +675,7 @@ impl ArchivedCachePolicy {
// "an Expires header field" // "an Expires header field"
if self.response.headers.expires_unix_timestamp.is_some() { if self.response.headers.expires_unix_timestamp.is_some() {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because its response has an \ "Response from {} is storable because it has an \
'Expires' header set", 'Expires' header set",
self.request.uri, self.request.uri,
); );
@ -684,7 +684,7 @@ impl ArchivedCachePolicy {
// "a max-age response directive" // "a max-age response directive"
if self.response.headers.cc.max_age_seconds.is_some() { if self.response.headers.cc.max_age_seconds.is_some() {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because its response has an \ "Response from {} is storable because it has an \
'max-age' cache-control directive", 'max-age' cache-control directive",
self.request.uri, self.request.uri,
); );
@ -693,8 +693,8 @@ impl ArchivedCachePolicy {
// "if the cache is shared: an s-maxage response directive" // "if the cache is shared: an s-maxage response directive"
if self.config.shared && self.response.headers.cc.s_maxage_seconds.is_some() { if self.config.shared && self.response.headers.cc.s_maxage_seconds.is_some() {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because this is a shared cache \ "Response from {} is storable because this is a shared cache \
and its response has a 's-maxage' cache-control directive", and has a 's-maxage' cache-control directive",
self.request.uri, self.request.uri,
); );
return true; return true;
@ -705,7 +705,7 @@ impl ArchivedCachePolicy {
// "a status code that is defined as heuristically cacheable" // "a status code that is defined as heuristically cacheable"
if HEURISTICALLY_CACHEABLE_STATUS_CODES.contains(&self.response.status.into()) { if HEURISTICALLY_CACHEABLE_STATUS_CODES.contains(&self.response.status.into()) {
tracing::trace!( tracing::trace!(
"Cached request {} is storable because its response has a \ "Response from {} is storable because it has a \
heuristically cacheable status code {:?}", heuristically cacheable status code {:?}",
self.request.uri, self.request.uri,
self.response.status, self.response.status,
@ -713,7 +713,7 @@ impl ArchivedCachePolicy {
return true; return true;
} }
tracing::trace!( tracing::trace!(
"Cached response {} is not storable because it does not meet any \ "Response from {} is not storable because it does not meet any \
of the necessary criteria (e.g., it doesn't have an 'Expires' \ of the necessary criteria (e.g., it doesn't have an 'Expires' \
header set or a 'max-age' cache-control directive)", header set or a 'max-age' cache-control directive)",
self.request.uri, self.request.uri,
@ -766,7 +766,7 @@ impl ArchivedCachePolicy {
// [RFC 9111 S5.2.1.4]: https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.4 // [RFC 9111 S5.2.1.4]: https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.4
if reqcc.no_cache { if reqcc.no_cache {
tracing::trace!( tracing::trace!(
"Request {} does not have a fresh cache because \ "Request to {} does not have a fresh cache entry because \
it has a 'no-cache' cache-control directive", it has a 'no-cache' cache-control directive",
request.url(), request.url(),
); );
@ -780,7 +780,7 @@ impl ArchivedCachePolicy {
if let Some(&max_age) = reqcc.max_age_seconds.as_ref() { if let Some(&max_age) = reqcc.max_age_seconds.as_ref() {
if age > max_age { if age > max_age {
tracing::trace!( tracing::trace!(
"Request {} does not have a fresh cache because \ "Request to {} does not have a fresh cache entry because \
the cached response's age is {} seconds and the max age \ the cached response's age is {} seconds and the max age \
allowed by the request is {} seconds", allowed by the request is {} seconds",
request.url(), request.url(),
@ -800,7 +800,7 @@ impl ArchivedCachePolicy {
let time_to_live = freshness_lifetime.saturating_sub(unix_timestamp(now)); let time_to_live = freshness_lifetime.saturating_sub(unix_timestamp(now));
if time_to_live < min_fresh { if time_to_live < min_fresh {
tracing::trace!( tracing::trace!(
"Request {} does not have a fresh cache because \ "Request to {} does not have a fresh cache entry because \
the request set a 'min-fresh' cache-control directive, \ the request set a 'min-fresh' cache-control directive, \
and its time-to-live is {} seconds but it needs to be \ and its time-to-live is {} seconds but it needs to be \
at least {} seconds", at least {} seconds",
@ -818,7 +818,7 @@ impl ArchivedCachePolicy {
let allows_stale = self.allows_stale(now); let allows_stale = self.allows_stale(now);
if !allows_stale { if !allows_stale {
tracing::trace!( tracing::trace!(
"Request {} does not have a fresh cache because \ "Request to {} does not have a fresh cache entry because \
its age is {} seconds, it is greater than the freshness \ its age is {} seconds, it is greater than the freshness \
lifetime of {} seconds and stale cached responses are not \ lifetime of {} seconds and stale cached responses are not \
allowed", allowed",
@ -846,7 +846,7 @@ impl ArchivedCachePolicy {
// [RFC 9111 S5.2.2.2]: https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.2.2 // [RFC 9111 S5.2.2.2]: https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.2.2
if self.response.headers.cc.must_revalidate { if self.response.headers.cc.must_revalidate {
tracing::trace!( tracing::trace!(
"Cached request {} has a cached response that does not \ "Request to {} has a cached response that does not \
permit staleness because the response has a 'must-revalidate' \ permit staleness because the response has a 'must-revalidate' \
cache-control directive set", cache-control directive set",
self.request.uri, self.request.uri,
@ -865,7 +865,7 @@ impl ArchivedCachePolicy {
.saturating_sub(self.freshness_lifetime().as_secs()); .saturating_sub(self.freshness_lifetime().as_secs());
if stale_amount <= max_stale.into() { if stale_amount <= max_stale.into() {
tracing::trace!( tracing::trace!(
"Cached request {} has a cached response that allows staleness \ "Request to {} has a cached response that allows staleness \
in this case because the stale amount is {} seconds and the \ in this case because the stale amount is {} seconds and the \
'max-stale' cache-control directive set by the cached request \ 'max-stale' cache-control directive set by the cached request \
is {} seconds", is {} seconds",
@ -885,7 +885,7 @@ impl ArchivedCachePolicy {
// //
// [RFC 9111 S4.2.4]: https://www.rfc-editor.org/rfc/rfc9111.html#section-4.2.4 // [RFC 9111 S4.2.4]: https://www.rfc-editor.org/rfc/rfc9111.html#section-4.2.4
tracing::trace!( tracing::trace!(
"Cached request {} has a cached response that does not allow staleness", "Request to {} has a cached response that does not allow staleness",
self.request.uri, self.request.uri,
); );
false false