-
Notifications
You must be signed in to change notification settings - Fork 787
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
feat(workers-shared): Add Asset Server Worker behaviour #6539
Changes from 5 commits
2b3ebad
01ef430
4f7acde
786c1a7
98ab8e4
c0bcf8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
--- | ||
"@cloudflare/workers-shared": minor | ||
--- | ||
|
||
feat: Add basic Asset Worker behaviour | ||
|
||
This commit implements a basic Asset Worker behaviour, including: | ||
|
||
- headers handling | ||
- `200`/`404`/`500` response handling | ||
- fetching data from KV |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# Workers with Assets | ||
|
||
Welcome to Workers + Assets YAY! | ||
|
||
Please proceed with much excitement ^.^ | ||
|
||
................. | ||
.......-++**********:. | ||
..-+****++**********#: | ||
...-+***+++====+********##= | ||
....:=****+======+***********##+ | ||
....-****+======+*************###+ | ||
.:==+*+======+***************####= | ||
....-===--=====++****************#####: | ||
....:====-:::-==+******************#####+. | ||
..-====:::::::+*******************#####*.. | ||
..-===-:::::::-==+*****************######=.. | ||
..-===-::::::-=======***************######*. | ||
...-===-::::::-==========+************######*:. | ||
...-===-::::::-==----========+*********#######:.. | ||
.-===-::::::--::::::::::-======+******#######-. . | ||
..:====::::::-=:::-*####*-:::-=======+**#######-. | ||
.:====-:::::-==:::*#########-::========+**#####- | ||
..====-:::::-===-::###########*:::======*******+.. | ||
..=+====:::::=====::-############:::====+*******=... | ||
....:=++====:::-======-::###########*:::===+*******-.. | ||
...........:-==+**+=++++=============-:::*#########-::===********:.. | ||
....--=+************====+++===============:::=*####*=:::-=+*******=.. | ||
.:*************###*=====++++================::::::::::-==********:.. | ||
..*****#############=======++++==========================+*******+... | ||
...+****#############*==:::-===++++========================********-. | ||
...+****###############+=::::=====+++++====================********+... | ||
.=****################*=-:::-======+++++=================+********:.. | ||
..-****##################===:-=========++++++=============+********:.. | ||
.+***####################===========+***#++++++=========+********=.. | ||
.*###################+-.-=======+****##*==+++++++====+********=... | ||
...-+#############*=...-++=====+****####+=====++++++*********+... | ||
....-*######+:.....++++===******####==========+*######**+.. | ||
...+=.....:::-+++++******#####=========+***######*.. | ||
.......::::::=++******#####+========***********:... | ||
....::::::...+*****######=======+**********##*. | ||
..:::::....-*****######+=====+**********#####*. | ||
...:::::...:******#####*+++++*********#########+. | ||
..:::::...+*****######+++++++=.=###############:. | ||
.:::::..-*****######-..-=++=..-##########***###.. | ||
....::::..=****######=....::::..:########******##*.. | ||
...::::..+***######-.....::::...*##***********###=.. | ||
..::::...-**####*:......::::...###************###-.. | ||
..:::......=##*:......:::::...+##*************###: | ||
...::::...............:::::....+##*************####. | ||
...::::::::::::::..:::::::......**************###-.. | ||
.::::::::::::::::::::::..... ...:***********###+.... | ||
..::::.......::::::...... ..:********###*:... | ||
........ ...=******###=.. | ||
.=***###+.. | ||
..*####:... | ||
........ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
|
||
.----------------. .----------------. .----------------. | ||
| .--------------. || .--------------. || .--------------. | | ||
| | ____ ____ | || | __ | || | ____ ____ | | | ||
| | |_ _||_ _| | || | / \ | || | |_ _||_ _| | | | ||
| | \ \ / / | || | / /\ \ | || | \ \ / / | | | ||
| | \ \/ / | || | / ____ \ | || | \ \/ / | | | ||
| | _| |_ | || | _/ / \ \_ | || | _| |_ | | | ||
| | |______| | || ||____| |____|| || | |______| | | | ||
| | | || | | || | | | | ||
| '--------------' || '--------------' || '--------------' | | ||
'----------------' '----------------' '----------------' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,11 @@ const CONTENT_HASH_SIZE = 16; | |
const TAIL_SIZE = 8; | ||
const ENTRY_SIZE = PATH_HASH_SIZE + CONTENT_HASH_SIZE + TAIL_SIZE; | ||
|
||
export type AssetEntry = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this ends up being used anywhere There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh right. must be some leftover from all the iterations 😅 . will remove as part of WC-2597 |
||
path: string; | ||
contentHash: string; | ||
}; | ||
|
||
export class AssetsManifest { | ||
private data: ArrayBuffer; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
// have the browser check in with the server to make sure its local cache is valid before using it | ||
export const CACHE_CONTROL_BROWSER = "public, max-age=0, must-revalidate"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
type Env = { | ||
// ASSETS_MANIFEST is a pipeline binding to an ArrayBuffer containing the | ||
// binary-encoded site manifest | ||
ASSETS_MANIFEST: ArrayBuffer; | ||
|
||
// ASSETS_KV_NAMESPACE is a pipeline binding to the KV namespace that the | ||
// assets are in. | ||
ASSETS_KV_NAMESPACE: KVNamespace; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,62 @@ | ||
import { WorkerEntrypoint } from "cloudflare:workers"; | ||
import { AssetsManifest } from "./assets-manifest"; | ||
import { | ||
InternalServerErrorResponse, | ||
MethodNotAllowedResponse, | ||
NotFoundResponse, | ||
OkResponse, | ||
} from "./responses"; | ||
import { getAdditionalHeaders, getMergedHeaders } from "./utils/headers"; | ||
import { getAssetWithMetadataFromKV } from "./utils/kv"; | ||
|
||
interface Env { | ||
/** | ||
* ASSETS_MANIFEST is a pipeline binding to an ArrayBuffer containing the | ||
* binary-encoded site manifest | ||
*/ | ||
ASSETS_MANIFEST: ArrayBuffer; | ||
/** | ||
* ASSETS_KV_NAMESPACE is a pipeline binding to the KV namespace that the | ||
* assets are in. | ||
*/ | ||
ASSETS_KV_NAMESPACE: KVNamespace; | ||
} | ||
|
||
export default { | ||
async fetch(request: Request, env: Env) { | ||
const { ASSETS_MANIFEST, ASSETS_KV_NAMESPACE } = env; | ||
export default class extends WorkerEntrypoint<Env> { | ||
async fetch(request: Request) { | ||
if (request.method.toLowerCase() !== "get") { | ||
CarmenPopoviciu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return new MethodNotAllowedResponse(); | ||
} | ||
|
||
const url = new URL(request.url); | ||
const { pathname } = url; | ||
try { | ||
return this.handleRequest(request); | ||
} catch (err) { | ||
return new InternalServerErrorResponse(err); | ||
} | ||
} | ||
|
||
const assetsManifest = new AssetsManifest(ASSETS_MANIFEST); | ||
const assetKey = await assetsManifest.get(pathname); | ||
if (!assetKey) { | ||
return new Response("Not Found", { status: 404 }); | ||
async handleRequest(request: Request) { | ||
const assetEntry = await this.getAssetEntry(request); | ||
if (!assetEntry) { | ||
return new NotFoundResponse(); | ||
} | ||
|
||
const content = await ASSETS_KV_NAMESPACE.get(assetKey); | ||
if (!content) { | ||
const assetResponse = await getAssetWithMetadataFromKV( | ||
this.env.ASSETS_KV_NAMESPACE, | ||
assetEntry | ||
); | ||
|
||
if (!assetResponse || !assetResponse.value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. technically this should be just a sanity check, but felt like we should check nonetheless. I'll ask the EWC folks if we want to throw a different error here. IMO a |
||
throw new Error( | ||
`Requested asset ${assetKey} exists in the asset manifest but not in the KV namespace.` | ||
`Requested asset ${assetEntry} exists in the asset manifest but not in the KV namespace.` | ||
); | ||
} | ||
|
||
return new Response(content); | ||
}, | ||
}; | ||
const { value: assetContent, metadata: assetMetadata } = assetResponse; | ||
const additionalHeaders = getAdditionalHeaders( | ||
assetEntry, | ||
assetMetadata, | ||
request | ||
); | ||
const headers = getMergedHeaders(request.headers, additionalHeaders); | ||
|
||
return new OkResponse(assetContent, { headers }); | ||
} | ||
|
||
private async getAssetEntry(request: Request) { | ||
const url = new URL(request.url); | ||
let { pathname } = url; | ||
|
||
const assetsManifest = new AssetsManifest(this.env.ASSETS_MANIFEST); | ||
pathname = globalThis.decodeURIComponent(pathname); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this should be decoded because the assetManifest uses encoded pathnames? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, so funky path names def don't work, with or without this decoding. Let's have a look together, and I'll add the proper fix/tests in a follow-up PR. Tracking in WC-2597 |
||
|
||
return await assetsManifest.get(pathname); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
export class OkResponse extends Response { | ||
constructor(body: BodyInit | null, init?: ResponseInit) { | ||
super(body, { | ||
...init, | ||
status: 200, | ||
}); | ||
} | ||
} | ||
|
||
export class NotFoundResponse extends Response { | ||
constructor(...[body, init]: ConstructorParameters<typeof Response>) { | ||
super(body, { | ||
...init, | ||
status: 404, | ||
statusText: "Not Found", | ||
}); | ||
} | ||
} | ||
|
||
export class MethodNotAllowedResponse extends Response { | ||
constructor(...[body, init]: ConstructorParameters<typeof Response>) { | ||
super(body, { | ||
...init, | ||
status: 405, | ||
statusText: "Method Not Allowed", | ||
}); | ||
} | ||
} | ||
|
||
export class InternalServerErrorResponse extends Response { | ||
constructor(err: Error, init?: ResponseInit) { | ||
super(undefined, { | ||
...init, | ||
status: 500, | ||
}); | ||
} | ||
} | ||
|
||
export class NotModifiedResponse extends Response { | ||
constructor(...[_body, _init]: ConstructorParameters<typeof Response>) { | ||
super(undefined, { | ||
status: 304, | ||
statusText: "Not Modified", | ||
}); | ||
} | ||
} | ||
CarmenPopoviciu marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see https://github.com/cloudflare/workerd/blob/a20cb7eb98309b0cd38be1c294dcdf894308a2d1/src/workerd/api/kv.c%2B%2B#L218-L220