From 31aa15ccc931d757a449ade2bd1881bf9a83ca51 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Wed, 31 Jul 2024 12:28:00 +0100 Subject: [PATCH] Clearer error message when trying to use Workers Sites with `wrangler versions upload` (#6379) * throw error when using Workers Sites or Legacy Assets with `wrangler versions upload` * extract and reuse experimentalAssets logic from `wrangler deploy` in `wrangler versions upload` * add tests * add changeset * add back --assets experimental warning mistakenly thought it was a duplicate * move processExperimentalAssetsArg to experimental-assets.ts --- .changeset/chatty-pans-grow.md | 5 ++ packages/wrangler/e2e/versions.test.ts | 67 +++++++++++++++++++ packages/wrangler/src/deploy/index.ts | 32 +-------- .../wrangler/src/deployment-bundle/entry.ts | 2 +- packages/wrangler/src/dev.tsx | 28 +------- packages/wrangler/src/experimental-assets.ts | 36 ++++++++++ packages/wrangler/src/versions/index.ts | 42 +++++++++++- packages/wrangler/src/versions/upload.ts | 1 + 8 files changed, 156 insertions(+), 57 deletions(-) create mode 100644 .changeset/chatty-pans-grow.md diff --git a/.changeset/chatty-pans-grow.md b/.changeset/chatty-pans-grow.md new file mode 100644 index 000000000000..d7b88b099e1c --- /dev/null +++ b/.changeset/chatty-pans-grow.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +fix: clearer error message when trying to use Workers Sites or Legacy Assets with `wrangler versions upload` diff --git a/packages/wrangler/e2e/versions.test.ts b/packages/wrangler/e2e/versions.test.ts index 49afb30db606..d065d5a54bb9 100644 --- a/packages/wrangler/e2e/versions.test.ts +++ b/packages/wrangler/e2e/versions.test.ts @@ -518,6 +518,73 @@ describe("versions deploy", { timeout: TIMEOUT }, () => { expect(countOccurrences(deploymentsList.stdout, versionId2)).toBe(1); // once for versions deploy, only }); + it("fails to upload if using Legacy Assets", async () => { + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + `, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello World!") + } + } + `, + "package.json": dedent` + { + "name": "${workerName}", + "version": "0.0.0", + "private": true + } + `, + }); + + const upload = await helper.run( + `wrangler versions upload --assets='./public' --x-versions` + ); + + expect(normalize(upload.output)).toMatchInlineSnapshot(` + "X [ERROR] Legacy Assets are not supported in Gradual Deployments. + 🪵 Logs were written to """ + `); + }); + + it("fails to upload if using Workers Sites", async () => { + await helper.seed({ + "wrangler.toml": dedent` + name = "${workerName}" + main = "src/index.ts" + compatibility_date = "2023-01-01" + + [site] + bucket = "./public" + `, + "src/index.ts": dedent` + export default { + fetch(request) { + return new Response("Hello World!") + } + } + `, + "package.json": dedent` + { + "name": "${workerName}", + "version": "0.0.0", + "private": true + } + `, + }); + + const upload = await helper.run(`wrangler versions upload --x-versions`); + + expect(normalize(upload.output)).toMatchInlineSnapshot(` + "X [ERROR] Workers Sites are not supported in Gradual Deployments. + 🪵 Logs were written to """ + `); + }); + it("should delete Worker", async () => { const { stdout } = await helper.run(`wrangler delete`); diff --git a/packages/wrangler/src/deploy/index.ts b/packages/wrangler/src/deploy/index.ts index a630c7b42ba6..326e6141ea14 100644 --- a/packages/wrangler/src/deploy/index.ts +++ b/packages/wrangler/src/deploy/index.ts @@ -1,9 +1,8 @@ -import { existsSync } from "node:fs"; import path from "node:path"; import { findWranglerToml, readConfig } from "../config"; import { getEntry } from "../deployment-bundle/entry"; import { UserError } from "../errors"; -import { getExperimentalAssetsBasePath } from "../experimental-assets"; +import { processExperimentalAssetsArg } from "../experimental-assets"; import { getRules, getScriptName, @@ -290,39 +289,14 @@ export async function deployHandler( ); } - const experimentalAssets = args.experimentalAssets - ? { directory: args.experimentalAssets } - : config.experimental_assets; - if (experimentalAssets) { - const experimentalAssetsBasePath = getExperimentalAssetsBasePath( - config, - args.experimentalAssets - ); - const resolvedExperimentalAssetsPath = path.resolve( - experimentalAssetsBasePath, - experimentalAssets.directory - ); - - if (!existsSync(resolvedExperimentalAssetsPath)) { - const sourceOfTruthMessage = args.experimentalAssets - ? '"--experimental-assets" command line argument' - : '"experimental_assets.directory" field in your configuration file'; - - throw new UserError( - `The directory specified by the ${sourceOfTruthMessage} does not exist:\n` + - `${resolvedExperimentalAssetsPath}` - ); - } - - experimentalAssets.directory = resolvedExperimentalAssetsPath; - } - if (args.assets) { logger.warn( "The --assets argument is experimental and may change or break at any time" ); } + const experimentalAssets = processExperimentalAssetsArg(args, config); + if (args.latest) { logger.warn( "Using the latest version of the Workers runtime. To silence this warning, please choose a specific version of the runtime with --compatibility-date, or add a compatibility_date to your wrangler.toml.\n" diff --git a/packages/wrangler/src/deployment-bundle/entry.ts b/packages/wrangler/src/deployment-bundle/entry.ts index 336d1ee523bf..8eef2c9e8088 100644 --- a/packages/wrangler/src/deployment-bundle/entry.ts +++ b/packages/wrangler/src/deployment-bundle/entry.ts @@ -41,7 +41,7 @@ export async function getEntry( experimentalAssets?: string; }, config: Config, - command: "dev" | "deploy" | "types" + command: "dev" | "deploy" | "versions upload" | "types" ): Promise { let file: string; let directory = process.cwd(); diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 6cc9b4cbd11e..2fa09c157275 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -1,6 +1,5 @@ import assert from "node:assert"; import events from "node:events"; -import { existsSync } from "node:fs"; import path from "node:path"; import util from "node:util"; import { isWebContainer } from "@webcontainer/env"; @@ -22,7 +21,7 @@ import registerDevHotKeys from "./dev/hotkeys"; import { maybeRegisterLocalWorker } from "./dev/local"; import { startDevServer } from "./dev/start-server"; import { UserError } from "./errors"; -import { getExperimentalAssetsBasePath } from "./experimental-assets"; +import { processExperimentalAssetsArg } from "./experimental-assets"; import { run } from "./experimental-flags"; import isInteractive from "./is-interactive"; import { logger } from "./logger"; @@ -804,31 +803,8 @@ export async function startDev(args: StartDevOptions) { }); } - const experimentalAssets = args.experimentalAssets - ? { directory: args.experimentalAssets } - : config.experimental_assets; + const experimentalAssets = processExperimentalAssetsArg(args, config); if (experimentalAssets) { - const experimentalAssetsBasePath = getExperimentalAssetsBasePath( - config, - args.experimentalAssets - ); - const resolvedExperimentalAssetsPath = path.resolve( - experimentalAssetsBasePath, - experimentalAssets.directory - ); - - if (!existsSync(resolvedExperimentalAssetsPath)) { - const sourceOfTruthMessage = args.experimentalAssets - ? '"--experimental-assets" command line argument' - : '"experimental_assets.directory" field in your configuration file'; - - throw new UserError( - `The directory specified by the ${sourceOfTruthMessage} does not exist:\n` + - `${resolvedExperimentalAssetsPath}` - ); - } - - experimentalAssets.directory = resolvedExperimentalAssetsPath; args.forceLocal = true; } diff --git a/packages/wrangler/src/experimental-assets.ts b/packages/wrangler/src/experimental-assets.ts index bc49f982d930..cb4ea7aeb4ff 100644 --- a/packages/wrangler/src/experimental-assets.ts +++ b/packages/wrangler/src/experimental-assets.ts @@ -1,4 +1,6 @@ +import { existsSync } from "node:fs"; import path from "node:path"; +import { UserError } from "./errors"; import type { Config } from "./config"; /** @@ -13,3 +15,37 @@ export function getExperimentalAssetsBasePath( ? process.cwd() : path.resolve(path.dirname(config.configPath ?? "wrangler.toml")); } + +export function processExperimentalAssetsArg( + args: { experimentalAssets: string | undefined }, + config: Config +) { + const experimentalAssets = args.experimentalAssets + ? { directory: args.experimentalAssets } + : config.experimental_assets; + if (experimentalAssets) { + const experimentalAssetsBasePath = getExperimentalAssetsBasePath( + config, + args.experimentalAssets + ); + const resolvedExperimentalAssetsPath = path.resolve( + experimentalAssetsBasePath, + experimentalAssets.directory + ); + + if (!existsSync(resolvedExperimentalAssetsPath)) { + const sourceOfTruthMessage = args.experimentalAssets + ? '"--experimental-assets" command line argument' + : '"experimental_assets.directory" field in your configuration file'; + + throw new UserError( + `The directory specified by the ${sourceOfTruthMessage} does not exist:\n` + + `${resolvedExperimentalAssetsPath}` + ); + } + + experimentalAssets.directory = resolvedExperimentalAssetsPath; + } + + return experimentalAssets; +} diff --git a/packages/wrangler/src/versions/index.ts b/packages/wrangler/src/versions/index.ts index dd463f564289..0d52d2bc5e9a 100644 --- a/packages/wrangler/src/versions/index.ts +++ b/packages/wrangler/src/versions/index.ts @@ -1,6 +1,8 @@ import path from "node:path"; import { findWranglerToml, readConfig } from "../config"; import { getEntry } from "../deployment-bundle/entry"; +import { UserError } from "../errors"; +import { processExperimentalAssetsArg } from "../experimental-assets"; import { getRules, getScriptName, @@ -86,10 +88,30 @@ export function versionsUploadOptions(yargs: CommonYargsArgv) { type: "boolean", default: false, }) + .option("legacy-assets", { + describe: "(Experimental) Static assets to be served", + type: "string", + requiresArg: true, + hidden: true, + }) + .option("assets", { + describe: "(Experimental) Static assets to be served", + type: "string", + requiresArg: true, + hidden: true, + }) + .option("experimental-assets", { + describe: "Static assets to be served", + type: "string", + alias: "x-assets", + requiresArg: true, + hidden: true, + }) .option("site", { describe: "Root folder of static assets for Workers Sites", type: "string", requiresArg: true, + hidden: true, }) .option("site-include", { describe: @@ -97,6 +119,7 @@ export function versionsUploadOptions(yargs: CommonYargsArgv) { type: "string", requiresArg: true, array: true, + hidden: true, }) .option("site-exclude", { describe: @@ -104,6 +127,7 @@ export function versionsUploadOptions(yargs: CommonYargsArgv) { type: "string", requiresArg: true, array: true, + hidden: true, }) .option("var", { describe: @@ -180,7 +204,7 @@ export async function versionsUploadHandler( args.config || (args.script && findWranglerToml(path.dirname(args.script))); const projectRoot = configPath && path.dirname(configPath); const config = readConfig(configPath, args); - const entry = await getEntry(args, config, "deploy"); + const entry = await getEntry(args, config, "versions upload"); await metrics.sendMetricsEvent( "upload worker version", { @@ -191,6 +215,21 @@ export async function versionsUploadHandler( } ); + args.legacyAssets = args.legacyAssets ?? args.assets; + + if (args.site || config.site) { + throw new UserError( + "Workers Sites are not supported in Gradual Deployments." + ); + } + if (args.legacyAssets || config.legacy_assets) { + throw new UserError( + "Legacy Assets are not supported in Gradual Deployments." + ); + } + + const experimentalAssets = processExperimentalAssetsArg(args, config); + if (args.latest) { logger.warn( "Using the latest version of the Workers runtime. To silence this warning, please choose a specific version of the runtime with --compatibility-date, or add a compatibility_date to your wrangler.toml.\n" @@ -222,6 +261,7 @@ export async function versionsUploadHandler( jsxFactory: args.jsxFactory, jsxFragment: args.jsxFragment, tsconfig: args.tsconfig, + experimentalAssets: experimentalAssets?.directory, minify: args.minify, uploadSourceMaps: args.uploadSourceMaps, nodeCompat: args.nodeCompat, diff --git a/packages/wrangler/src/versions/upload.ts b/packages/wrangler/src/versions/upload.ts index 72140b6c2afb..68f7d67095bf 100644 --- a/packages/wrangler/src/versions/upload.ts +++ b/packages/wrangler/src/versions/upload.ts @@ -50,6 +50,7 @@ type Props = { env: string | undefined; compatibilityDate: string | undefined; compatibilityFlags: string[] | undefined; + experimentalAssets: string | undefined; vars: Record | undefined; defines: Record | undefined; alias: Record | undefined;