Repository navigation
Flag stale Composer and Chef metadata - #541
Conversation
With cache_metadata on, a failed upstream metadata fetch fell back to the cached copy for any error. A 404 (package removed) and a 401/403 (the proxy's credentials rejected) are definite answers, though, and serving the old version list in their place kept them from reaching the client. They now pass through: 404 as 404, 401/403 as 502 like other upstream failures. Timeouts, connection errors, 5xx and 429 still fall back. The npm, Composer and Chef handlers serve metadata through the rewrite cache, which dropped the stale flag, so a fallback looked like a fresh 200. They now set the same Warning: 110 header as other metadata. Fixes git-pkgs#499
andrew
left a comment
andrew
left a comment
There was a problem hiding this comment.
The stale-warning fix misses handleFilteredNPMVersionMetadata. With cooldown or a version denylist enabled, version/tag requests take this separate path, which still discards the stale flag. A targeted handler test with a denylist entry for another version reproduced /left-pad/1.0.0 returning cached metadata as 200 with no Warning after upstream returned 503. Please carry the stale flag through this path and add a regression test that enters through the npm route.
With cooldown or a version denylist, /pkg/<version> reads the filtered packument through handleFilteredNPMVersionMetadata, which dropped the stale flag.
|
Done: the filtered version path now carries the stale flag, with a regression test through the npm route (denylist entry for another version, upstream 503). |
Serving cached metadata when the upstream refuses is documented for generic upstreams, and GitHub rate limits are 403s, so the fallback stays as it is. git-pkgs#503 handles npm's definite answers. What is left here is the stale warning for Composer, Chef and hosted npm metadata, which went through the rewrite cache without it.
simonchrz
changed the title
|
I've narrowed this PR. Dropping the cached fallback on 404 and 401/403 for every ecosystem is what I asked #503 to scope to npm: it breaks the documented fallback for generic upstreams, and GitHub rate limits are 403s. #503 now does that for npm only, so it's out of here. What's left is the stale warning for Composer, Chef and hosted npm metadata, which went through the rewrite cache without it. The filtered npm version path moves to #503 too. |
2 participants
Part of #499, alongside #503.
npm, Composer and Chef serve metadata through the rewrite cache, which dropped the stale flag, so a fallback to the cached copy looked like a fresh 200. #503 fixes this for npm package metadata. This PR adds the same
Warning: 110 - "Response is Stale"to Composer metadata (upstream and with pushed versions), Chef's universe and cookbook documents, and npm metadata with pushed versions.The fallback itself is unchanged: 404 and 401/403 still serve the cached copy for these ecosystems, as documented.
Testing
Composer and Chef handler tests cover an upstream 502, 404, 401 and 403 after a cached fetch, and check for the cached copy with the warning. Without the fix all eight fail.