Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cache #3804

Merged
merged 17 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/cache/memory-cache-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ class MemoryStoreReadableStream extends Readable {
* @type {MemoryStoreValue}
*/
#value

/**
* @type {Buffer[]}
*/
Expand Down
89 changes: 51 additions & 38 deletions lib/handler/cache-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const {
parseVaryHeader
} = require('../util/cache')

function noop () {}

/**
* Writes a response to a CacheStore and then passes it on to the next handler
*/
Expand Down Expand Up @@ -46,6 +48,17 @@ class CacheHandler extends DecoratorHandler {
this.#handler = handler
}

onConnect (abort) {
if (this.#writeStream) {
this.#writeStream.destroy()
this.#writeStream = undefined
}

if (typeof this.#handler.onConnect === 'function') {
this.#handler.onConnect(abort)
}
}

/**
* @see {DispatchHandlers.onHeaders}
*
Expand All @@ -61,49 +74,46 @@ class CacheHandler extends DecoratorHandler {
resume,
statusMessage
) {
const downstreamOnHeaders = () => this.#handler.onHeaders(
statusCode,
rawHeaders,
resume,
statusMessage
)
const downstreamOnHeaders = () => {
if (typeof this.#handler.onHeaders === 'function') {
return this.#handler.onHeaders(
statusCode,
rawHeaders,
resume,
statusMessage
)
} else {
return true
}
}

if (
!util.safeHTTPMethods.includes(this.#requestOptions.method) &&
statusCode >= 200 &&
statusCode <= 399
) {
// https://www.rfc-editor.org/rfc/rfc9111.html#name-invalidating-stored-respons
// Try/catch for if it's synchronous
// https://www.rfc-editor.org/rfc/rfc9111.html#name-invalidating-stored-response
try {
const result = this.#store.deleteByOrigin(this.#requestOptions.origin)
if (
result &&
typeof result.catch === 'function' &&
typeof this.#handler.onError === 'function'
) {
// Fail silently
result.catch(_ => {})
}
} catch (err) {
this.#store.deleteByOrigin(this.#requestOptions.origin.toString())?.catch?.(noop)
} catch {
// Fail silently
}

return downstreamOnHeaders()
}

const headers = util.parseHeaders(rawHeaders)

const cacheControlHeader = headers['cache-control']
const contentLengthHeader = headers['content-length']

if (!cacheControlHeader || !contentLengthHeader || this.#store.isFull) {
// Don't have the headers we need, can't cache
if (!cacheControlHeader || typeof cacheControlHeader !== 'string') {
// Don't have cache-control, can't cache.
return downstreamOnHeaders()
}

const contentLength = Number(contentLengthHeader)
const contentLengthHeader = headers['content-length']
const contentLength = contentLengthHeader ? Number(contentLengthHeader) : null
if (!Number.isInteger(contentLength)) {
// Don't know the final size, don't cache.
// TODO (fix): Why not cache?
return downstreamOnHeaders()
}

Expand Down Expand Up @@ -137,18 +147,24 @@ class CacheHandler extends DecoratorHandler {
})

if (this.#writeStream) {
this.#writeStream.on('drain', resume)
this.#writeStream.on('error', () => {
this.#writeStream = undefined
resume()
})
const handler = this
this.#writeStream
.on('drain', resume)
.on('error', function () {
// TODO (fix): Make error somehow observable?
})
.on('close', function () {
if (handler.#writeStream === this) {
handler.#writeStream = undefined
}

// TODO (fix): Should we resume even if was paused downstream?
resume()
})
}
}

if (typeof this.#handler.onHeaders === 'function') {
return downstreamOnHeaders()
}
return false
return downstreamOnHeaders()
}

/**
Expand Down Expand Up @@ -178,10 +194,7 @@ class CacheHandler extends DecoratorHandler {
*/
onComplete (rawTrailers) {
if (this.#writeStream) {
if (rawTrailers) {
this.#writeStream.rawTrailers = rawTrailers
}

this.#writeStream.rawTrailers = rawTrailers ?? []
this.#writeStream.end()
}

Expand Down Expand Up @@ -332,7 +345,7 @@ function stripNecessaryHeaders (rawHeaders, parsedHeaders, cacheControlDirective
for (let i = 0; i < headerNames.length; i++) {
const header = headerNames[i]

if (headersToRemove.indexOf(header) !== -1) {
if (headersToRemove.includes(header)) {
// We have a at least one header we want to remove
if (!strippedHeaders) {
// This is the first header we want to remove, let's create the object
Expand Down
57 changes: 44 additions & 13 deletions lib/handler/cache-revalidation-handler.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

const assert = require('node:assert')
const DecoratorHandler = require('../handler/decorator-handler')

/**
Expand All @@ -19,29 +20,35 @@ const DecoratorHandler = require('../handler/decorator-handler')
class CacheRevalidationHandler extends DecoratorHandler {
#successful = false
/**
* @type {(() => void)}
* @type {((boolean) => void) | null}
*/
#successCallback
#callback
/**
* @type {(import('../../types/dispatcher.d.ts').default.DispatchHandlers)}
*/
#handler

#abort
/**
* @param {() => void} successCallback Function to call if the cached value is valid
* @param {(boolean) => void} callback Function to call if the cached value is valid
* @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler
*/
constructor (successCallback, handler) {
if (typeof successCallback !== 'function') {
throw new TypeError('successCallback must be a function')
constructor (callback, handler) {
if (typeof callback !== 'function') {
throw new TypeError('callback must be a function')
}

super(handler)

this.#successCallback = successCallback
this.#callback = callback
this.#handler = handler
}

onConnect (abort) {
this.#successful = false
this.#abort = abort
}

/**
* @see {DispatchHandlers.onHeaders}
*
Expand All @@ -57,13 +64,21 @@ class CacheRevalidationHandler extends DecoratorHandler {
resume,
statusMessage
) {
assert(this.#callback != null)

// https://www.rfc-editor.org/rfc/rfc9111.html#name-handling-a-validation-respo
if (statusCode === 304) {
this.#successful = true
this.#successCallback()
this.#successful = statusCode === 304
this.#callback(this.#successful)
this.#callback = null

if (this.#successful) {
return true
}

if (typeof this.#handler.onConnect === 'function') {
this.#handler.onConnect(this.#abort)
}

if (typeof this.#handler.onHeaders === 'function') {
return this.#handler.onHeaders(
statusCode,
Expand All @@ -72,7 +87,8 @@ class CacheRevalidationHandler extends DecoratorHandler {
statusMessage
)
}
return false

return true
}

/**
Expand All @@ -90,7 +106,7 @@ class CacheRevalidationHandler extends DecoratorHandler {
return this.#handler.onData(chunk)
}

return false
return true
}

/**
Expand All @@ -99,7 +115,11 @@ class CacheRevalidationHandler extends DecoratorHandler {
* @param {string[] | null} rawTrailers
*/
onComplete (rawTrailers) {
if (!this.#successful && typeof this.#handler.onComplete === 'function') {
if (this.#successful) {
return
}

if (typeof this.#handler.onComplete === 'function') {
this.#handler.onComplete(rawTrailers)
}
}
Expand All @@ -110,8 +130,19 @@ class CacheRevalidationHandler extends DecoratorHandler {
* @param {Error} err
*/
onError (err) {
if (this.#successful) {
return
}

if (this.#callback) {
this.#callback(false)
this.#callback = null
}

if (typeof this.#handler.onError === 'function') {
this.#handler.onError(err)
} else {
throw err
}
}
}
Expand Down
Loading
Loading