Skip to content

Commit

Permalink
fix: log cache hits distinct from fetch (#273)
Browse files Browse the repository at this point in the history
Avoid confusion between actual remote fetches and cache hits (which
don't involve a fetch).

Ref npm/pacote#403 (comment)
  • Loading branch information
mbtools authored Oct 14, 2024
1 parent c046cca commit 8044781
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
16 changes: 12 additions & 4 deletions lib/check-response.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,18 @@ function logRequest (method, res, startTime) {
const cacheStr = cacheStatus ? ` (cache ${cacheStatus})` : ''
const urlStr = cleanUrl(res.url)

log.http(
'fetch',
`${method.toUpperCase()} ${res.status} ${urlStr} ${elapsedTime}ms${attemptStr}${cacheStr}`
)
// If make-fetch-happen reports a cache hit, then there was no fetch
if (cacheStatus === 'hit') {
log.http(
'cache',
`${urlStr} ${elapsedTime}ms${attemptStr}${cacheStr}`
)
} else {
log.http(
'fetch',
`${method.toUpperCase()} ${res.status} ${urlStr} ${elapsedTime}ms${attemptStr}${cacheStr}`
)
}
}

function checkErrors (method, res, startTime, opts) {
Expand Down
4 changes: 2 additions & 2 deletions test/check-response.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ t.test('logs the value of x-local-cache-status when set', t => {
startTime,
})
res.body.emit('end')
t.equal(header, 'fetch')
t.equal(header, 'cache')
t.match(
msg,
/^GET 200 http:\/\/username:\*\*\*@example.com\/foo\/bar\/baz [0-9]+m?s \(cache hit\)$/
/^http:\/\/username:\*\*\*@example.com\/foo\/bar\/baz [0-9]+m?s \(cache hit\)$/
)
})

0 comments on commit 8044781

Please sign in to comment.