non-TTY check for required variables:

Added a check in non-TTY environments for `account_id`, `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. If `account_id` exists in `wrangler.toml` then `CLOUDFLARE_ACCOUNT_ID` is not needed in non-TTY scope. The `CLOUDFLARE_API_TOKEN` is necessary in non-TTY scope and will always error if missing. resolves #827
cloudflare · May 4, 2022 · 16b92d5 · 16b92d5
1 parent 50cbe61
commit 16b92d5
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 24 deletions.
diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -178,7 +178,7 @@ describe("publish", () => {
         expect(std.err).toMatchInlineSnapshot(`""`);
       });
 
-      it("should throw an error in non-TTY if 'account_id' & 'CLOUDFLARE_ACCOUNT_ID' is missing", async () => {
+      it("should throw an error in non-TTY & there is more than one account associated with API token", async () => {
         setIsTTY(false);
         process.env = {
           CLOUDFLARE_API_TOKEN: "hunter2",
@@ -233,10 +233,35 @@ describe("publish", () => {
         await expect(runWrangler("publish index.js")).rejects.toThrowError();
 
         expect(std.err).toMatchInlineSnapshot(`
-          "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mMissing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD[0m
+          "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mIn a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work.
+          Please go to https://developers.cloudflare.com/api/tokens/create/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN.[0m
 
+          "
+        `);
+      });
+      it("should throw error with no account ID provided and no members retrieved", async () => {
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "picard",
+          CLOUDFLARE_ACCOUNT_ID: undefined,
+        };
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: false,
+          result: [],
+        });
 
-          [31mX [41;31m[[41;97mERROR[41;31m][0m [1mDid not login, quitting...[0m
+        await expect(runWrangler("publish index.js")).rejects.toThrowError();
+
+        expect(std.err).toMatchInlineSnapshot(`
+          "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mFailed to automatically retrieve account IDs for the logged in user.
+          In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file.[0m
 
           "
         `);

diff --git a/packages/wrangler/src/__tests__/secret.test.ts b/packages/wrangler/src/__tests__/secret.test.ts
@@ -4,7 +4,6 @@ import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
 import { setMockResponse, unsetAllMocks } from "./helpers/mock-cfetch";
 import { mockConsoleMethods } from "./helpers/mock-console";
 import { mockConfirm, mockPrompt } from "./helpers/mock-dialogs";
-import { useMockIsTTY } from "./helpers/mock-istty";
 import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
 import { useMockStdin } from "./helpers/mock-stdin";
 import { runInTempDir } from "./helpers/run-in-tmp";
@@ -13,14 +12,13 @@ import { runWrangler } from "./helpers/run-wrangler";
 describe("wrangler secret", () => {
   const std = mockConsoleMethods();
   const { mockGetMemberships } = mockOAuthFlow();
-  const { setIsTTY } = useMockIsTTY();
+
   runInTempDir();
   mockAccountId();
   mockApiToken();
 
   afterEach(() => {
     unsetAllMocks();
-    setIsTTY(true);
   });
 
   describe("put", () => {
@@ -177,11 +175,11 @@ describe("wrangler secret", () => {
             success: false,
             result: [],
           });
-          await expect(
-            runWrangler("secret put the-key --name script-name")
-          ).rejects.toThrowErrorMatchingInlineSnapshot(
-            `"No account id found, quitting..."`
-          );
+          await expect(runWrangler("secret put the-key --name script-name"))
+            .rejects.toThrowErrorMatchingInlineSnapshot(`
+                  "Failed to automatically retrieve account IDs for the logged in user.
+                  In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file."
+                `);
         });
 
         it("should use the account from wrangler.toml", async () => {
@@ -204,7 +202,6 @@ describe("wrangler secret", () => {
         });
 
         it("should error if a user has multiple accounts, and has not specified an account in wrangler.toml", async () => {
-          setIsTTY(false);
           mockGetMemberships({
             success: true,
             result: [

diff --git a/packages/wrangler/src/__tests__/whoami.test.tsx b/packages/wrangler/src/__tests__/whoami.test.tsx
@@ -4,12 +4,18 @@ import { writeAuthConfigFile } from "../user";
 import { getUserInfo, WhoAmI } from "../whoami";
 import { setMockResponse } from "./helpers/mock-cfetch";
 import { mockConsoleMethods } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import type { UserInfo } from "../whoami";
 
 describe("getUserInfo()", () => {
   runInTempDir({ homedir: "./home" });
   const std = mockConsoleMethods();
+  const { setIsTTY } = useMockIsTTY();
+
+  beforeEach(() => {
+    setIsTTY(true);
+  });
 
   it("should return undefined if there is no config file", async () => {
     const userInfo = await getUserInfo();

diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx
@@ -842,11 +842,11 @@ const createDeployment: CommandModule<
     const config = getConfigCache<PagesConfigCache>(
       PAGES_CONFIG_CACHE_FILENAME
     );
-    const isInteractive = process.stdin.isTTY;
-    const accountId = await requireAuth(config, isInteractive);
+    const accountId = await requireAuth(config);
 
     projectName ??= config.project_name;
 
+    const isInteractive = process.stdin.isTTY;
     if (!projectName && isInteractive) {
       const existingOrNew = await new Promise<"new" | "existing">((resolve) => {
         const { unmount } = render(
@@ -1605,8 +1605,8 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
             const config = getConfigCache<PagesConfigCache>(
               PAGES_CONFIG_CACHE_FILENAME
             );
-            const isInteractive = process.stdin.isTTY;
-            const accountId = await requireAuth(config, isInteractive);
+
+            const accountId = await requireAuth(config);
 
             const projects: Array<Project> = await listProjects({ accountId });
 
@@ -1650,9 +1650,9 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
             const config = getConfigCache<PagesConfigCache>(
               PAGES_CONFIG_CACHE_FILENAME
             );
-            const isInteractive = process.stdin.isTTY;
-            const accountId = await requireAuth(config, isInteractive);
+            const accountId = await requireAuth(config);
 
+            const isInteractive = process.stdin.isTTY;
             if (!projectName && isInteractive) {
               projectName = await prompt("Enter the name of your new project:");
             }
@@ -1735,11 +1735,11 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
               const config = getConfigCache<PagesConfigCache>(
                 PAGES_CONFIG_CACHE_FILENAME
               );
-              const isInteractive = process.stdin.isTTY;
-              const accountId = await requireAuth(config, isInteractive);
+              const accountId = await requireAuth(config);
 
               projectName ??= config.project_name;
 
+              const isInteractive = process.stdin.isTTY;
               if (!projectName && isInteractive) {
                 const projects = await listProjects({ accountId });
                 projectName = await new Promise((resolve) => {

diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx
@@ -418,11 +418,9 @@ export function getAPIToken(): string | undefined {
     !localAPIToken &&
     !LocalState.accessToken?.value
   ) {
-    logger.error(
-      "Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD"
+    throw new Error(
+      "In a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work. Please go to https://developers.cloudflare.com/api/tokens/create/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN."
     );
-
-    return;
   }
 
   return localAPIToken ?? LocalState.accessToken?.value;
@@ -1142,6 +1140,11 @@ export async function getAccountId(
             .join("\n")
       );
     }
+  } else {
+    if (!isInteractive)
+      throw new Error(
+        `Failed to automatically retrieve account IDs for the logged in user. In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file.`
+      );
   }
   return accountId;
 }