From e5251df52154e9ebc98edb02ee0598c7210dcf0f Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 16:36:50 -0500 Subject: [PATCH 1/7] Refactor error handling to stop execution on errors. Fixes #160. --- .changeset/stale-spiders-turn.md | 5 + src/index.ts | 168 +++++++++++++++---------------- 2 files changed, 88 insertions(+), 85 deletions(-) create mode 100644 .changeset/stale-spiders-turn.md diff --git a/.changeset/stale-spiders-turn.md b/.changeset/stale-spiders-turn.md new file mode 100644 index 0000000..d682a4b --- /dev/null +++ b/.changeset/stale-spiders-turn.md @@ -0,0 +1,5 @@ +--- +"wrangler-action": patch +--- + +Refactored error handling to stop execution when action fails. Previously, the action would continue executing to completion if one of the steps encountered an error. Fixes #160. diff --git a/src/index.ts b/src/index.ts index f21f3a8..544da0a 100755 --- a/src/index.ts +++ b/src/index.ts @@ -80,13 +80,18 @@ function semverCompare(version1: string, version2: string) { } async function main() { - installWrangler(); - authenticationSetup(); - await execCommands(getMultilineInput("preCommands"), "pre"); - await uploadSecrets(); - await wranglerCommands(); - await execCommands(getMultilineInput("postCommands"), "post"); - info("🏁 Wrangler Action completed", true); + try { + installWrangler(); + authenticationSetup(); + await execCommands(getMultilineInput("preCommands"), "pre"); + await uploadSecrets(); + await wranglerCommands(); + await execCommands(getMultilineInput("postCommands"), "post"); + info("🏁 Wrangler Action completed", true); + } catch (err) { + error(`${err}`); + setFailed("🚨 Action failed"); + } } async function runProcess( @@ -103,7 +108,7 @@ async function runProcess( } catch (err: any) { err.stdout && info(err.stdout.toString()); err.stderr && error(err.stderr.toString(), true); - throw err; + throw new Error(`\`${command}\` returned non-zero exit code.`); } } @@ -113,19 +118,19 @@ function checkWorkingDirectory(workingDirectory = ".") { if (existsSync(normalizedPath)) { return normalizedPath; } else { - setFailed(`🚨 Directory ${workingDirectory} does not exist.`); + throw new Error(`Directory ${workingDirectory} does not exist.`); } } catch (error) { - setFailed( - `🚨 While checking/creating directory ${workingDirectory} received ${error}`, + throw new Error( + `While checking/creating directory ${workingDirectory} received ${error}`, ); } } function installWrangler() { if (config["WRANGLER_VERSION"].startsWith("1")) { - setFailed( - `🚨 Wrangler v1 is no longer supported by this action. Please use major version 2 or greater`, + throw new Error( + `Wrangler v1 is no longer supported by this action. Please use major version 2 or greater`, ); } startGroup("📥 Installing Wrangler"); @@ -145,27 +150,26 @@ async function execCommands(commands: string[], cmdType: string) { if (!commands.length) { return; } + startGroup(`🚀 Running ${cmdType}Commands`); + try { + const arrPromises = commands.map(async (command) => { + const cmd = command.startsWith("wrangler") + ? `${getNpxCmd()} ${command}` + : command; - const arrPromises = commands.map(async (command) => { - const cmd = command.startsWith("wrangler") - ? `${getNpxCmd()} ${command}` - : command; + info(`🚀 Executing command: ${cmd}`); - info(`🚀 Executing command: ${cmd}`); - - return await runProcess(cmd, { - cwd: config["workingDirectory"], - env: process.env, + return await runProcess(cmd, { + cwd: config["workingDirectory"], + env: process.env, + }); }); - }); - await Promise.all(arrPromises).catch((result) => { - result.stdout && info(result.stdout.toString()); - result.stderr && error(result.stderr.toString(), true); - setFailed(`🚨 ${cmdType}Commands failed`); - }); - endGroup(); + await Promise.all(arrPromises); + } finally { + endGroup(); + } } /** @@ -173,12 +177,12 @@ async function execCommands(commands: string[], cmdType: string) { */ function getSecret(secret: string) { if (!secret) { - setFailed("No secret provided"); + throw new Error("Secret name cannot be blank."); } const value = process.env[secret]; if (!value) { - setFailed(`Secret ${secret} not found`); + throw new Error(`Value for secret ${secret} not found.`); } return value; @@ -189,26 +193,22 @@ async function legacyUploadSecrets( environment?: string, workingDirectory?: string, ) { - try { - const arrPromises = secrets - .map((secret) => { - const command = `echo ${getSecret( - secret, - )} | ${getNpxCmd()} wrangler secret put ${secret}`; - return environment ? command.concat(` --env ${environment}`) : command; - }) - .map( - async (command) => - await execAsync(command, { - cwd: workingDirectory, - env: process.env, - }), - ); + const arrPromises = secrets + .map((secret) => { + const command = `echo ${getSecret( + secret, + )} | ${getNpxCmd()} wrangler secret put ${secret}`; + return environment ? command.concat(` --env ${environment}`) : command; + }) + .map( + async (command) => + await execAsync(command, { + cwd: workingDirectory, + env: process.env, + }), + ); - await Promise.all(arrPromises); - } catch { - setFailed(`🚨 Error uploading secrets`); - } + await Promise.all(arrPromises); } async function uploadSecrets() { @@ -245,8 +245,8 @@ async function uploadSecrets() { stdio: "ignore", }); info(`✅ Uploaded secrets`); - } catch { - setFailed(`🚨 Error uploading secrets`); + } catch (err) { + throw new Error(`Error uploading secrets: ${err}`); } finally { endGroup(); } @@ -258,7 +258,7 @@ function getVarArgs() { if (process.env[envVar] && process.env[envVar]?.length !== 0) { return `${envVar}:${process.env[envVar]!}`; } else { - setFailed(`🚨 ${envVar} not found in variables.`); + throw new Error(`Value for var ${envVar} not found in environment.`); } }); @@ -267,47 +267,45 @@ function getVarArgs() { async function wranglerCommands() { startGroup("🚀 Running Wrangler Commands"); - const commands = config["COMMANDS"]; - const environment = config["ENVIRONMENT"]; + try { + const commands = config["COMMANDS"]; + const environment = config["ENVIRONMENT"]; - if (!commands.length) { - const wranglerVersion = config["WRANGLER_VERSION"]; - const deployCommand = semverCompare("2.20.0", wranglerVersion) - ? "deploy" - : "publish"; - commands.push(deployCommand); - } - - const arrPromises = commands.map(async (command) => { - if (environment.length > 0 && !command.includes(`--env`)) { - command = command.concat(` --env ${environment}`); + if (!commands.length) { + const wranglerVersion = config["WRANGLER_VERSION"]; + const deployCommand = semverCompare("2.20.0", wranglerVersion) + ? "deploy" + : "publish"; + commands.push(deployCommand); } - const cmd = `${getNpxCmd()} wrangler ${command} ${ - (command.startsWith("deploy") || command.startsWith("publish")) && - !command.includes(`--var`) - ? getVarArgs() - : "" - }`.trim(); + const arrPromises = commands.map(async (command) => { + if (environment.length > 0 && !command.includes(`--env`)) { + command = command.concat(` --env ${environment}`); + } - info(`🚀 Executing command: ${cmd}`); + const cmd = `${getNpxCmd()} wrangler ${command} ${ + (command.startsWith("deploy") || command.startsWith("publish")) && + !command.includes(`--var`) + ? getVarArgs() + : "" + }`.trim(); - return await runProcess(cmd, { - cwd: config["workingDirectory"], - env: process.env, + info(`🚀 Executing command: ${cmd}`); + + return await runProcess(cmd, { + cwd: config["workingDirectory"], + env: process.env, + }); }); - }); - await Promise.all(arrPromises).catch((result) => { - result.stdout && info(result.stdout.toString()); - result.stderr && error(result.stderr.toString()); - setFailed(`🚨 Command failed`); - }); - - endGroup(); + await Promise.all(arrPromises); + } finally { + endGroup(); + } } -main().catch(() => setFailed("🚨 Action failed")); +main(); export { wranglerCommands, From 105f191b1918c28dd59db2aca6ebc37b584a0f59 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 17:43:29 -0500 Subject: [PATCH 2/7] Update snapshot for checkWorkingDirectory test --- src/index.test.ts | 18 +++++------------- src/index.ts | 16 +++++----------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index f698741..8210778 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -47,18 +47,10 @@ describe("checkWorkingDirectory", () => { }); test("should fail if the directory does not exist", () => { - try { - checkWorkingDirectory("/does/not/exist"); - } catch (error) { - expect(error.message).toMatchInlineSnapshot(); - } - }); - - test("should fail if an error occurs while checking/creating the directory", () => { - try { - checkWorkingDirectory("/does/not/exist"); - } catch (error) { - expect(error.message).toMatchInlineSnapshot(); - } + expect(() => + checkWorkingDirectory("/does/not/exist"), + ).toThrowErrorMatchingInlineSnapshot( + '"Directory /does/not/exist does not exist."', + ); }); }); diff --git a/src/index.ts b/src/index.ts index 544da0a..0046f4a 100755 --- a/src/index.ts +++ b/src/index.ts @@ -113,17 +113,11 @@ async function runProcess( } function checkWorkingDirectory(workingDirectory = ".") { - try { - const normalizedPath = path.normalize(workingDirectory); - if (existsSync(normalizedPath)) { - return normalizedPath; - } else { - throw new Error(`Directory ${workingDirectory} does not exist.`); - } - } catch (error) { - throw new Error( - `While checking/creating directory ${workingDirectory} received ${error}`, - ); + const normalizedPath = path.normalize(workingDirectory); + if (existsSync(normalizedPath)) { + return normalizedPath; + } else { + throw new Error(`Directory ${workingDirectory} does not exist.`); } } From f74c325efc99a3516bf305dde83caeaf5603d987 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 17:57:19 -0500 Subject: [PATCH 3/7] Limit error logging on secret upload --- src/index.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 0046f4a..ad7c1bb 100755 --- a/src/index.ts +++ b/src/index.ts @@ -213,9 +213,10 @@ async function uploadSecrets() { if (!secrets.length) { return; } - try { - startGroup("🔑 Uploading Secrets"); + startGroup("🔑 Uploading secrets..."); + + try { if (semverCompare(config["WRANGLER_VERSION"], "3.4.0")) return legacyUploadSecrets(secrets, environment, workingDirectory); @@ -238,9 +239,11 @@ async function uploadSecrets() { env: process.env, stdio: "ignore", }); + info(`✅ Uploaded secrets`); } catch (err) { - throw new Error(`Error uploading secrets: ${err}`); + error(`❌ Upload failed`); + throw new Error(`Failed to upload secrets.`); } finally { endGroup(); } From 5517edbb28efeaa083381256dea5e6498800d95b Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 18:00:03 -0500 Subject: [PATCH 4/7] Avoid double "Error: Error:" logging --- src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index ad7c1bb..3e18064 100755 --- a/src/index.ts +++ b/src/index.ts @@ -88,8 +88,8 @@ async function main() { await wranglerCommands(); await execCommands(getMultilineInput("postCommands"), "post"); info("🏁 Wrangler Action completed", true); - } catch (err) { - error(`${err}`); + } catch (err: unknown) { + err instanceof Error && error(err.message); setFailed("🚨 Action failed"); } } From 2962e94ac86209af88c83967a41647f2e5096e56 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 18:16:41 -0500 Subject: [PATCH 5/7] Move unit-tested modules into utils file This fixes the issue where running the unit tests invoked the entire action since index.ts calls `main()` at the top-level scope. --- action-env-setup.ts | 1 - src/index.ts | 39 +--------------------------- src/{index.test.ts => utils.test.ts} | 13 +--------- src/utils.ts | 35 +++++++++++++++++++++++++ tsconfig.json | 4 +-- vitest.config.ts | 7 ----- 6 files changed, 39 insertions(+), 60 deletions(-) delete mode 100644 action-env-setup.ts rename src/{index.test.ts => utils.test.ts} (79%) create mode 100644 src/utils.ts delete mode 100644 vitest.config.ts diff --git a/action-env-setup.ts b/action-env-setup.ts deleted file mode 100644 index 75c7e92..0000000 --- a/action-env-setup.ts +++ /dev/null @@ -1 +0,0 @@ -process.env.INPUT_QUIET ??= "false"; diff --git a/src/index.ts b/src/index.ts index 3e18064..927c727 100755 --- a/src/index.ts +++ b/src/index.ts @@ -9,8 +9,7 @@ import { getBooleanInput, } from "@actions/core"; import { execSync, exec } from "node:child_process"; -import { existsSync } from "node:fs"; -import * as path from "node:path"; +import { checkWorkingDirectory, getNpxCmd, semverCompare } from "./utils"; import * as util from "node:util"; const execAsync = util.promisify(exec); @@ -31,10 +30,6 @@ const config = { QUIET_MODE: getBooleanInput("quiet"), } as const; -function getNpxCmd() { - return process.env.RUNNER_OS === "Windows" ? "npx.cmd" : "npx"; -} - function info(message: string, bypass?: boolean): void { if (!config.QUIET_MODE || bypass) { originalInfo(message); @@ -59,26 +54,6 @@ function endGroup(): void { } } -/** - * A helper function to compare two semver versions. If the second arg is greater than the first arg, it returns true. - */ -function semverCompare(version1: string, version2: string) { - if (version2 === "latest") return true; - - const version1Parts = version1.split("."); - const version2Parts = version2.split("."); - - for (const version1Part of version1Parts) { - const version2Part = version2Parts.shift(); - - if (version1Part !== version2Part && version2Part) { - return version1Part < version2Part ? true : false; - } - } - - return false; -} - async function main() { try { installWrangler(); @@ -112,15 +87,6 @@ async function runProcess( } } -function checkWorkingDirectory(workingDirectory = ".") { - const normalizedPath = path.normalize(workingDirectory); - if (existsSync(normalizedPath)) { - return normalizedPath; - } else { - throw new Error(`Directory ${workingDirectory} does not exist.`); - } -} - function installWrangler() { if (config["WRANGLER_VERSION"].startsWith("1")) { throw new Error( @@ -310,7 +276,4 @@ export { uploadSecrets, authenticationSetup, installWrangler, - checkWorkingDirectory, - getNpxCmd, - semverCompare, }; diff --git a/src/index.test.ts b/src/utils.test.ts similarity index 79% rename from src/index.test.ts rename to src/utils.test.ts index 8210778..cfe65cf 100644 --- a/src/index.test.ts +++ b/src/utils.test.ts @@ -1,18 +1,7 @@ import { expect, test, describe } from "vitest"; -import { checkWorkingDirectory, getNpxCmd, semverCompare } from "./index"; +import { checkWorkingDirectory, getNpxCmd, semverCompare } from "./utils"; import path from "node:path"; -const config = { - WRANGLER_VERSION: "mockVersion", - secrets: ["mockSercret", "mockSecretAgain"], - workingDirectory: "./mockWorkingDirectory", - CLOUDFLARE_API_TOKEN: "mockAPIToken", - CLOUDFLARE_ACCOUNT_ID: "mockAccountID", - ENVIRONMENT: undefined, - VARS: ["mockVar", "mockVarAgain"], - COMMANDS: ["mockCommand", "mockCommandAgain"], -}; - test("getNpxCmd ", async () => { process.env.RUNNER_OS = "Windows"; expect(getNpxCmd()).toBe("npx.cmd"); diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 0000000..ab6d0d2 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,35 @@ +import { existsSync } from "node:fs"; +import * as path from "node:path"; + +export function getNpxCmd() { + return process.env.RUNNER_OS === "Windows" ? "npx.cmd" : "npx"; +} + +/** + * A helper function to compare two semver versions. If the second arg is greater than the first arg, it returns true. + */ +export function semverCompare(version1: string, version2: string) { + if (version2 === "latest") return true; + + const version1Parts = version1.split("."); + const version2Parts = version2.split("."); + + for (const version1Part of version1Parts) { + const version2Part = version2Parts.shift(); + + if (version1Part !== version2Part && version2Part) { + return version1Part < version2Part ? true : false; + } + } + + return false; +} + +export function checkWorkingDirectory(workingDirectory = ".") { + const normalizedPath = path.normalize(workingDirectory); + if (existsSync(normalizedPath)) { + return normalizedPath; + } else { + throw new Error(`Directory ${workingDirectory} does not exist.`); + } +} diff --git a/tsconfig.json b/tsconfig.json index 99fbbbf..03e6f29 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,12 +9,12 @@ "isolatedModules": true, "target": "ESNext", "module": "ESNext", - "moduleResolution": "NodeNext", + "moduleResolution": "Bundler", "rootDir": "./src", "outDir": "./dist", "lib": ["ESNext"], "types": ["node", "@cloudflare/workers-types"] }, "exclude": ["node_modules", "**/*.test.ts"], - "include": ["src/index.ts"] + "include": ["src"] } diff --git a/vitest.config.ts b/vitest.config.ts deleted file mode 100644 index d5d5cd3..0000000 --- a/vitest.config.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { defineConfig } from "vitest/dist/config"; - -export default defineConfig({ - test: { - setupFiles: ["./action-env-setup.ts"], - }, -}); From 71199d2757cef997124a6bae48e7aa26c0518c16 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 18:27:03 -0500 Subject: [PATCH 6/7] prettier scripts incorrectly set --ignore-path --ignore-path is meant to be the path of an ignore-file, not the actual directories to ignore. --- .github/workflows/issues.yml | 2 +- package.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/issues.yml b/.github/workflows/issues.yml index e129dab..8b469dc 100644 --- a/.github/workflows/issues.yml +++ b/.github/workflows/issues.yml @@ -13,4 +13,4 @@ jobs: - uses: actions/add-to-project@v0.5.0 with: project-url: https://github.com/orgs/cloudflare/projects/1 - github-token: ${{ secrets.GH_ACCESS_TOKEN }} \ No newline at end of file + github-token: ${{ secrets.GH_ACCESS_TOKEN }} diff --git a/package.json b/package.json index 9e1179d..ceb9feb 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,8 @@ "scripts": { "build": "npx ncc build ./src/index.ts && mv ./dist/index.js ./dist/index.mjs", "test": "vitest", - "format": "prettier --write . --ignore-path ./dist/**", - "check": "prettier --check . --ignore-path ./dist/**" + "format": "prettier --write .", + "check": "prettier --check ." }, "dependencies": { "@actions/core": "^1.10.0" From d7637bf514c4e96dfacc8af9692080f8ac46f119 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Tue, 29 Aug 2023 19:39:50 -0500 Subject: [PATCH 7/7] workaround to fix failing workflow. Refs #162 The action fails to upload secrets during one of the testing steps since the Worker doesn't exist yet. --- .github/workflows/deploy.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4fde508..2050e36 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -51,6 +51,7 @@ jobs: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} environment: dev + preCommands: npx wrangler deploy --env dev # https://github.com/cloudflare/wrangler-action/issues/162 secrets: | SECRET1 SECRET2