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

Author: JacobMGEvans

.changeset/eighty-yaks-jump.md

@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+feat: 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

packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts

@@ -1,5 +1,6 @@
 import fetchMock from "jest-fetch-mock";
 import { Request } from "undici";
+import { getCloudflareApiBaseUrl } from "../../cfetch";
 import openInBrowser from "../../open-in-browser";
 import { mockHttpServer } from "./mock-http-server";
 
@@ -85,6 +86,28 @@ export const mockOAuthFlow = () => {
     return outcome;
   };
 
+  const mockGetMemberships = (args: {
+    success: boolean;
+    result: { id: string; account: { id: string; name: string } }[];
+  }) => {
+    const outcome = {
+      actual: new Request("https://example.org"),
+      expected: new Request(`${getCloudflareApiBaseUrl()}/memberships`, {
+        method: "GET",
+      }),
+    };
+
+    fetchMock.mockIf(outcome.expected.url, async (req) => {
+      outcome.actual = req;
+      return {
+        status: 200,
+        body: JSON.stringify(args),
+      };
+    });
+
+    return outcome;
+  };
+
   const mockGrantAccessToken = ({
     respondWith,
   }: {
@@ -150,10 +173,11 @@ export const mockOAuthFlow = () => {
   };
 
   return {
-    mockOAuthServerCallback,
+    mockGetMemberships,
+    mockGrantAccessToken,
     mockGrantAuthorization,
+    mockOAuthServerCallback,
     mockRevokeAuthorization,
-    mockGrantAccessToken,
     mockExchangeRefreshTokenForAccessToken,
   };
 };

packages/wrangler/src/__tests__/publish.test.ts

@@ -22,6 +22,7 @@ import type { WorkerMetadata } from "../create-worker-upload-form";
 import type { KVNamespaceInfo } from "../kv";
 import type { CfWorkerInit } from "../worker";
 import type { FormData, File } from "undici";
+import { useMockIsTTY } from "./helpers/mock-istty";
 
 describe("publish", () => {
   mockAccountId();
@@ -32,6 +33,7 @@ describe("publish", () => {
     mockOAuthServerCallback,
     mockGrantAccessToken,
     mockGrantAuthorization,
+    mockGetMemberships,
   } = mockOAuthFlow();
 
   beforeEach(() => {
@@ -56,6 +58,7 @@ describe("publish", () => {
     });
 
     it("drops a user into the login flow if they're unauthenticated", async () => {
+      // Should not throw missing Errors in TTY environment
       writeWranglerToml();
       writeWorkerSource();
       mockSubDomainRequest();
@@ -116,6 +119,128 @@ describe("publish", () => {
       `);
       expect(std.err).toMatchInlineSnapshot(`""`);
     });
+
+    describe("non-TTY", () => {
+      const ENV_COPY = process.env;
+      const { setIsTTY } = useMockIsTTY();
+
+      afterEach(() => {
+        process.env = ENV_COPY;
+      });
+
+      it("should not throw an error in non-TTY if 'CLOUDFLARE_API_TOKEN' & 'account_id' are in scope", async () => {
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "123456789",
+        };
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: "some-account-id",
+        });
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+
+        await runWrangler("publish index.js");
+
+        expect(std.out).toMatchInlineSnapshot(`
+          "Uploaded test-name (TIMINGS)
+          Published test-name (TIMINGS)
+            test-name.test-sub-domain.workers.dev"
+        `);
+        expect(std.err).toMatchInlineSnapshot(`""`);
+      });
+
+      it("should not throw an error if 'CLOUDFLARE_ACCOUNT_ID' & 'CLOUDFLARE_API_TOKEN' are in scope", async () => {
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "hunter2",
+          CLOUDFLARE_ACCOUNT_ID: "some-account-id",
+        };
+        setIsTTY(false);
+        writeWranglerToml();
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [],
+        });
+
+        await runWrangler("publish index.js");
+
+        expect(std.out).toMatchInlineSnapshot(`
+          "Uploaded test-name (TIMINGS)
+          Published test-name (TIMINGS)
+            test-name.test-sub-domain.workers.dev"
+        `);
+        expect(std.err).toMatchInlineSnapshot(`""`);
+      });
+
+      it("should throw an error in non-TTY if 'account_id' & 'CLOUDFLARE_ACCOUNT_ID' is missing", async () => {
+        setIsTTY(false);
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "hunter2",
+          CLOUDFLARE_ACCOUNT_ID: undefined,
+        };
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [
+            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
+            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
+          ],
+        });
+
+        await expect(runWrangler("publish index.js")).rejects
+          .toMatchInlineSnapshot(`
+                [Error: More than one account available but unable to select one in non-interactive mode.
+                Please set the appropriate \`account_id\` in your \`wrangler.toml\` file.
+                Available accounts are ("<name>" - "<id>"):
+                  "enterprise" - "1701")
+                  "enterprise-nx" - "nx01")]
+              `);
+      });
+
+      it("should throw error in non-TTY if 'CLOUDFLARE_API_TOKEN' is missing", async () => {
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        process.env = {
+          CLOUDFLARE_API_TOKEN: undefined,
+          CLOUDFLARE_ACCOUNT_ID: "badwolf",
+        };
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [
+            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
+            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
+          ],
+        });
+
+        await expect(runWrangler("publish index.js")).rejects.toThrowError();
+
+        expect(std.err).toMatchInlineSnapshot(`
+          "X [ERROR] Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD
+
+
+          X [ERROR] Did not login, quitting...
+
+          "
+        `);
+      });
+    });
   });
 
   describe("environments", () => {

packages/wrangler/src/__tests__/sentry.test.ts

@@ -7,26 +7,27 @@ import * as Sentry from "@sentry/node";
 import prompts from "prompts";
 
 import { mockConsoleMethods } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 const { reportError } = jest.requireActual("../reporting");
 
 describe("Error Reporting", () => {
+  const { setIsTTY } = useMockIsTTY();
+
   runInTempDir({ homedir: "./home" });
   mockConsoleMethods();
   const reportingTOMLPath = ".wrangler/config/reporting.toml";
 
-  const originalTTY = process.stdout.isTTY;
   beforeEach(() => {
     jest.mock("@sentry/node");
     jest.spyOn(Sentry, "captureException");
-    process.stdout.isTTY = true;
+    setIsTTY(true);
   });
 
   afterEach(() => {
     jest.unmock("@sentry/node");
     jest.clearAllMocks();
-    process.stdout.isTTY = originalTTY;
   });
 
   it("should confirm user will allow error reporting usage", async () => {
@@ -127,20 +128,15 @@ describe("Error Reporting", () => {
   });
 
   it("should not prompt in non-TTY environment", async () => {
-    process.stdout.isTTY = false;
-
+    setIsTTY(false);
     await reportError(new Error("test error"), "testFalse");
 
-    const { error_tracking_opt, error_tracking_opt_date } = TOML.parse(
-      await fsp.readFile(path.join(os.homedir(), reportingTOMLPath), "utf-8")
-    );
-
-    expect(error_tracking_opt).toBe(false);
-    expect(error_tracking_opt_date).toBeTruthy();
+    expect(
+      fs.existsSync(path.join(os.homedir(), reportingTOMLPath, "utf-8"))
+    ).toBe(false);
 
     expect(Sentry.captureException).not.toHaveBeenCalledWith(
       new Error("test error")
     );
-    process.stdout.isTTY = originalTTY;
   });
 });

packages/wrangler/src/reporting.ts

@@ -93,7 +93,8 @@ function exceptionTransaction(error: Error, origin = "") {
 }
 
 export async function reportError(err: Error, origin = "") {
-  if (!process.stdout.isTTY) return await appendReportingDecision("false");
+  // If the user has not opted in to error reporting, we don't want to do anything in CI or non-interactive environments
+  if (!process.stdout.isTTY) return;
 
   const errorTrackingOpt = await reportingPermission();
 

packages/wrangler/src/user.tsx

@@ -410,7 +410,22 @@ export function getAPIToken(): string | undefined {
         "If you wish to authenticate via an API token then please set the `CLOUDFLARE_API_TOKEN` environment variable."
     );
   }
-  return LocalState.accessToken?.value;
+
+  const localAPIToken = getCloudflareAPITokenFromEnv();
+
+  if (
+    !process.stdout.isTTY &&
+    !localAPIToken &&
+    !LocalState.accessToken?.value
+  ) {
+    logger.error(
+      "Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD"
+    );
+
+    return;
+  }
+
+  return localAPIToken ?? LocalState.accessToken?.value;
 }
 
 interface AccessContext {
@@ -1155,10 +1170,8 @@ export function ChooseAccount(props: {
 /**
  * Ensure that a user is logged in, and a valid account_id is available.
  */
-export async function requireAuth(
-  config: Config,
-  isInteractive = true
-): Promise<string> {
+export async function requireAuth(config: Config): Promise<string> {
+  const isInteractive = process.stdout.isTTY;
   const loggedIn = await loginOrRefreshIfRequired(isInteractive);
   if (!loggedIn) {
     // didn't login, let's just quit