Skip to content
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
42 changes: 41 additions & 1 deletion packages/cache/src/bootstrap/cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Buffer } from 'node:buffer'

import { describe, test, expect } from 'vitest'
import { describe, test, expect, vi } from 'vitest'

import { NetlifyCache } from './cache.js'
import { ERROR_CODES } from './errors.js'
import { getMockFetch } from '../test/fetch.js'
import { decodeHeaders } from '../test/headers.js'

Expand Down Expand Up @@ -196,5 +197,44 @@ describe('Cache API', () => {

expect([...resourceHeaders]).toStrictEqual([...headers])
})

test('logs a message when the response is not added to the cache', async () => {
const consoleWarn = vi.spyOn(globalThis.console, 'warn')
const mockFetch = getMockFetch({
responses: {
'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F': [
(_, init) => {
const headers = init?.headers as Record<string, string>

expect(headers.Authorization).toBe(`Bearer ${token}`)
expect(headers['netlify-forwarded-host']).toBe(host)

return new Response(null, { headers: { 'netlify-programmable-error': 'no_ttl' }, status: 400 })
},
],
},
})
const cache = new NetlifyCache({
base64Encode,
getContext: () => ({ host, token, url }),
name: 'my-cache',
userAgent,
})

const headers = new Headers()
headers.set('content-type', 'text/html')
headers.set('x-custom-header', 'foobar')

const response = new Response('<h1>Hello world</h1>', { headers })

await cache.put(new Request('https://netlify.com'), response)

mockFetch.restore()

expect(consoleWarn).toHaveBeenCalledWith(`Failed to write to the cache: ${ERROR_CODES.no_ttl}`)
consoleWarn.mockRestore()

expect(mockFetch.requests.length).toBe(1)
})
})
})
10 changes: 9 additions & 1 deletion packages/cache/src/bootstrap/cache.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Base64Encoder, EnvironmentOptions, RequestContext, RequestContextFactory } from './environment.js'

import { ERROR_CODES, GENERIC_ERROR } from './errors.js'
import * as HEADERS from '../headers.js'

const allowedProtocols = new Set(['http:', 'https:'])
Expand Down Expand Up @@ -146,7 +147,7 @@ export class NetlifyCache implements Cache {
const context = this.#getContext()
const resourceURL = extractAndValidateURL(request)

await fetch(`${context.url}/${toCacheKey(resourceURL)}`, {
const cacheResponse = await fetch(`${context.url}/${toCacheKey(resourceURL)}`, {
body: response.body,
headers: {
...this[getInternalHeaders](context),
Expand All @@ -157,6 +158,13 @@ export class NetlifyCache implements Cache {
duplex: 'half',
method: 'POST',
})

if (!cacheResponse.ok) {
const errorDetail = cacheResponse.headers.get(HEADERS.ErrorDetail) ?? ''
const errorMessage = ERROR_CODES[errorDetail as keyof typeof ERROR_CODES] || GENERIC_ERROR
Comment on lines +163 to +164
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
const errorDetail = cacheResponse.headers.get(HEADERS.ErrorDetail) ?? ''
const errorMessage = ERROR_CODES[errorDetail as keyof typeof ERROR_CODES] || GENERIC_ERROR
const errorDetail = cacheResponse.headers.get(HEADERS.ErrorDetail)
const errorMessage = ERROR_CODES[errorDetail as keyof typeof ERROR_CODES] ?? GENERIC_ERROR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll do as a follow-up just so I don't lose your ✅.


console.warn(`Failed to write to the cache: ${errorMessage}`)
}
}
}

Expand Down
18 changes: 18 additions & 0 deletions packages/cache/src/bootstrap/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export const ERROR_CODES = {
invalid_vary:
'Responses must not use unsupported directives of the `Netlify-Vary` header (https://ntl.fyi/cache_api_invalid_vary).',
no_cache:
'Responses must not set cache control headers with the `private`, `no-cache` or `no-store` directives (https://ntl.fyi/cache_api_no_cache).',
low_ttl:
'Responses must have a cache control header with a `max-age` or `s-maxage` directive (https://ntl.fyi/cache_api_low_ttl).',
no_directive:
'Responses must have a cache control header with caching directives (https://ntl.fyi/cache_api_no_directive).',
no_ttl:
'Responses must have a cache control header with a `max-age` or `s-maxage` directive (https://ntl.fyi/cache_api_no_ttl).',
no_status: 'Responses must specify a status code (https://ntl.fyi/cache_api_no_status).',
invalid_directive:
'Responses must have a cache control header with caching directives (https://ntl.fyi/cache_api_invalid_directive).',
status: 'Responses must have a status code between 200 and 299 (https://ntl.fyi/cache_api_status).',
}

export const GENERIC_ERROR = 'The server has returned an unexpected error (https://ntl.fyi/cache_api_error).'
1 change: 1 addition & 0 deletions packages/cache/src/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const NetlifyCacheId = 'netlify-cache-id'
export const NetlifyCacheTag = 'netlify-cache-tag'
export const NetlifyCdnCacheControl = 'netlify-cdn-cache-control'
export const NetlifyVary = 'netlify-vary'
export const ErrorDetail = 'netlify-programmable-error'
export const ResourceHeaders = 'netlify-programmable-headers'
export const ResourceStatus = 'netlify-programmable-status'
export const ResourceStore = 'netlify-programmable-store'
Expand Down
Loading