-
Notifications
You must be signed in to change notification settings - Fork 3
Add Cloudflare artifact worker for large program sharing #488
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
Changes from all commits
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,3 @@ | ||
| # Artifact Worker Base URL | ||
| # Used by the debugger to load large programs from the artifact worker | ||
| VITE_ARTIFACTS_BASE_URL=https://pvm-artifacts.tomusdrw-cloudflare.workers.dev |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,3 +27,7 @@ dist-ssr | |
| /playwright-report/ | ||
| /blob-report/ | ||
| /playwright/.cache/ | ||
|
|
||
| # Environment files | ||
| .env | ||
| !.env.example | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,127 @@ | ||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||||||||||||||||||||||
| import { spawnSync } from "node:child_process"; | ||||||||||||||||||||||||||||||||||||
| import { readFile } from "node:fs/promises"; | ||||||||||||||||||||||||||||||||||||
| import { basename } from "node:path"; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| function printUsage() { | ||||||||||||||||||||||||||||||||||||
| console.error( | ||||||||||||||||||||||||||||||||||||
| "Usage: node scripts/upload-artifact.mjs --file <path> --worker <base-url> [--ttl <seconds>] [--debugger <base-url>] [--open]", | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| function parseArgs(argv) { | ||||||||||||||||||||||||||||||||||||
| const args = {}; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| for (let index = 0; index < argv.length; index += 1) { | ||||||||||||||||||||||||||||||||||||
| const key = argv[index]; | ||||||||||||||||||||||||||||||||||||
| const value = argv[index + 1]; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (!key.startsWith("--")) { | ||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (!value || value.startsWith("--")) { | ||||||||||||||||||||||||||||||||||||
| args[key.slice(2)] = "true"; | ||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| args[key.slice(2)] = value; | ||||||||||||||||||||||||||||||||||||
| index += 1; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return args; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| function normalizeBaseUrl(url) { | ||||||||||||||||||||||||||||||||||||
| return url.replace(/\/+$/, ""); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| function shouldOpenBrowser(value) { | ||||||||||||||||||||||||||||||||||||
| return value === "true" || value === "1" || value === "yes"; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| function openInBrowser(url) { | ||||||||||||||||||||||||||||||||||||
| const launchCommands = | ||||||||||||||||||||||||||||||||||||
| process.platform === "darwin" | ||||||||||||||||||||||||||||||||||||
| ? [["open", [url]]] | ||||||||||||||||||||||||||||||||||||
| : process.platform === "win32" | ||||||||||||||||||||||||||||||||||||
| ? [["cmd", ["/c", "start", "", url]]] | ||||||||||||||||||||||||||||||||||||
| : [["xdg-open", [url]]]; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| for (const [command, args] of launchCommands) { | ||||||||||||||||||||||||||||||||||||
| const result = spawnSync(command, args, { stdio: "ignore" }); | ||||||||||||||||||||||||||||||||||||
| if (!result.error && result.status === 0) { | ||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| throw new Error("Could not open browser automatically on this platform."); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| async function main() { | ||||||||||||||||||||||||||||||||||||
| const args = parseArgs(process.argv.slice(2)); | ||||||||||||||||||||||||||||||||||||
| const filePath = args.file; | ||||||||||||||||||||||||||||||||||||
| const workerBase = args.worker; | ||||||||||||||||||||||||||||||||||||
| const ttl = args.ttl; | ||||||||||||||||||||||||||||||||||||
| const debuggerBase = args.debugger; | ||||||||||||||||||||||||||||||||||||
| const open = shouldOpenBrowser(args.open); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (!filePath || !workerBase) { | ||||||||||||||||||||||||||||||||||||
| printUsage(); | ||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (open && !debuggerBase) { | ||||||||||||||||||||||||||||||||||||
| console.error("--open requires --debugger <base-url>."); | ||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const file = await readFile(filePath); | ||||||||||||||||||||||||||||||||||||
| const endpoint = new URL(`${normalizeBaseUrl(workerBase)}/artifacts`); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (ttl) { | ||||||||||||||||||||||||||||||||||||
| endpoint.searchParams.set("ttl", ttl); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const response = await fetch(endpoint, { | ||||||||||||||||||||||||||||||||||||
| method: "POST", | ||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||
| "content-type": "application/octet-stream", | ||||||||||||||||||||||||||||||||||||
| "x-upload-file-name": basename(filePath), | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| body: file, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+93
Contributor
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. Bound the upload request with a timeout. This POST can currently hang forever on DNS/TLS/network stalls. Please add an abort timeout so the CLI fails predictably when the worker is unreachable. Proposed fix const response = await fetch(endpoint, {
method: "POST",
headers: {
"content-type": "application/octet-stream",
"x-upload-file-name": basename(filePath),
},
body: file,
+ signal: AbortSignal.timeout(30_000),
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const textPayload = await response.text(); | ||||||||||||||||||||||||||||||||||||
| let parsed; | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| parsed = JSON.parse(textPayload); | ||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||
| parsed = { raw: textPayload }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (!response.ok) { | ||||||||||||||||||||||||||||||||||||
| console.error("Upload failed", { | ||||||||||||||||||||||||||||||||||||
| status: response.status, | ||||||||||||||||||||||||||||||||||||
| body: parsed, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| console.log(JSON.stringify(parsed, null, 2)); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (debuggerBase && parsed.artifactId) { | ||||||||||||||||||||||||||||||||||||
| const debuggerUrl = `${normalizeBaseUrl(debuggerBase)}/#/load?artifact=${encodeURIComponent(parsed.artifactId)}`; | ||||||||||||||||||||||||||||||||||||
| console.log(`Debugger link: ${debuggerUrl}`); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (open) { | ||||||||||||||||||||||||||||||||||||
| openInBrowser(debuggerUrl); | ||||||||||||||||||||||||||||||||||||
| console.log("Opened debugger in your default browser."); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| main().catch((error) => { | ||||||||||||||||||||||||||||||||||||
| console.error("Unexpected error", error); | ||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| interface ImportMetaEnv { | ||
| readonly VITE_ARTIFACTS_BASE_URL?: string; | ||
| } | ||
|
|
||
| interface ImportMeta { | ||
| readonly env: ImportMetaEnv; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { buildArtifactDownloadUrl } from "./artifact-url"; | ||
|
|
||
| describe("buildArtifactDownloadUrl", () => { | ||
| it("returns absolute URLs as-is", () => { | ||
| const url = buildArtifactDownloadUrl("https://artifacts.example.com/artifacts/abc123"); | ||
|
|
||
| expect(url).toBe("https://artifacts.example.com/artifacts/abc123"); | ||
| }); | ||
|
Comment on lines
+5
to
+9
Contributor
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. Don't accept arbitrary absolute URLs from Locking in absolute-URL passthrough means 🤖 Prompt for AI Agents |
||
|
|
||
| it("builds URL from base and artifact ID", () => { | ||
| const url = buildArtifactDownloadUrl("abc123", "https://artifacts.example.com/"); | ||
|
|
||
| expect(url).toBe("https://artifacts.example.com/artifacts/abc123"); | ||
| }); | ||
|
|
||
| it("falls back to origin when base URL is missing", () => { | ||
| const url = buildArtifactDownloadUrl("abc123", undefined, "http://localhost:5173"); | ||
|
|
||
| expect(url).toBe("http://localhost:5173/artifacts/abc123"); | ||
| }); | ||
|
|
||
| it("throws for empty artifact reference", () => { | ||
| expect(() => buildArtifactDownloadUrl(" ", "https://artifacts.example.com")).toThrow( | ||
| "Artifact reference is empty", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws when it cannot resolve base URL", () => { | ||
| expect(() => buildArtifactDownloadUrl("abc123")).toThrow("Missing artifacts base URL"); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| const HTTP_URL_PATTERN = /^https?:\/\//i; | ||
|
|
||
| export function buildArtifactDownloadUrl(artifactRef: string, baseUrl?: string, fallbackOrigin?: string): string { | ||
| const normalizedArtifactRef = artifactRef.trim(); | ||
| if (!normalizedArtifactRef) { | ||
| throw new Error("Artifact reference is empty"); | ||
| } | ||
|
|
||
| if (HTTP_URL_PATTERN.test(normalizedArtifactRef)) { | ||
| return normalizedArtifactRef; | ||
| } | ||
|
|
||
| const normalizedBaseUrl = (baseUrl?.trim() || fallbackOrigin?.trim() || "").replace(/\/+$/, ""); | ||
| if (!normalizedBaseUrl) { | ||
| throw new Error("Missing artifacts base URL"); | ||
| } | ||
|
|
||
| return `${normalizedBaseUrl}/artifacts/${encodeURIComponent(normalizedArtifactRef)}`; | ||
| } |
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.
Don't coerce missing values to
"true"for non-boolean flags.This branch currently turns
--file,--worker,--ttl, and--debuggerinto the string"true"when the next token is missing or another flag. The CLI then fails much later with misleading errors instead of stopping at argument parsing.Proposed fix
function parseArgs(argv) { const args = {}; + const booleanFlags = new Set(["open"]); for (let index = 0; index < argv.length; index += 1) { const key = argv[index]; const value = argv[index + 1]; + const name = key.slice(2); if (!key.startsWith("--")) { continue; } if (!value || value.startsWith("--")) { - args[key.slice(2)] = "true"; + if (!booleanFlags.has(name)) { + throw new Error(`Missing value for --${name}`); + } + args[name] = "true"; continue; } - args[key.slice(2)] = value; + args[name] = value; index += 1; }📝 Committable suggestion
🤖 Prompt for AI Agents