fix: do not crash in wrangler dev if user has multiple accounts

When a user has multiple accounts we show a prompt to allow the user to select which they should use.
This was broken in `wrangler dev` as we were trying to start a new ink.js app (to show the prompt)
from inside a running ink.js app (the UI for `wrangler dev`).

This fix refactors the `ChooseAccount` component so that it can be used directly within another component.

Fixes #1258

Author: petebacondarwin

.changeset/proud-cougars-sell.md

@@ -0,0 +1,13 @@
+---
+"wrangler": patch
+---
+
+fix: do not crash in `wrangler dev` if user has multiple accounts
+
+When a user has multiple accounts we show a prompt to allow the user to select which they should use.
+This was broken in `wrangler dev` as we were trying to start a new ink.js app (to show the prompt)
+from inside a running ink.js app (the UI for `wrangler dev`).
+
+This fix refactors the `ChooseAccount` component so that it can be used directly within another component.
+
+Fixes #1258

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

@@ -1,9 +1,17 @@
 import fetchMock from "jest-fetch-mock";
 import { Request } from "undici";
-import { getCloudflareApiBaseUrl } from "../../cfetch";
 import openInBrowser from "../../open-in-browser";
+import { setMockResponse } from "./mock-cfetch";
 import { mockHttpServer } from "./mock-http-server";
 
+export function mockGetMemberships(
+  accounts: { id: string; account: { id: string; name: string } }[]
+) {
+  setMockResponse("/memberships", "GET", () => {
+    return accounts;
+  });
+}
+
 // the response to send when wrangler wants an oauth grant
 let oauthGrantResponse: GrantResponseOptions | "timeout" = {};
 
@@ -86,28 +94,6 @@ 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,
   }: {
@@ -173,7 +159,6 @@ export const mockOAuthFlow = () => {
   };
 
   return {
-    mockGetMemberships,
     mockGrantAccessToken,
     mockGrantAuthorization,
     mockOAuthServerCallback,

packages/wrangler/src/__tests__/helpers/mock-stdin.ts

@@ -100,4 +100,9 @@ class MockStdIn {
     }
     return this.chunks.shift() ?? null;
   }
+
+  /**
+   * Used by Ink.js to set the encoding of the stdout.
+   */
+  setEncoding() {}
 }

packages/wrangler/src/__tests__/jest.setup.ts

@@ -77,7 +77,7 @@ jest.mock("../dev/dev", () => {
 jest.mock("../open-in-browser");
 
 // Mock the functions involved in getAuthURL so we don't take snapshots of the constantly changing URL.
-jest.mock("../generate-auth-url", () => {
+jest.mock("../user/generate-auth-url", () => {
   return {
     generateRandomState: jest.fn().mockImplementation(() => "MOCK_STATE_PARAM"),
     generateAuthUrl: jest
@@ -100,7 +100,7 @@ jest.mock("../generate-auth-url", () => {
   };
 });
 
-jest.mock("../generate-random-state", () => {
+jest.mock("../user/generate-random-state", () => {
   return {
     generateRandomState: jest.fn().mockImplementation(() => "MOCK_STATE_PARAM"),
   };

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

@@ -15,7 +15,7 @@ import { mockConsoleMethods, normalizeSlashes } from "./helpers/mock-console";
 import { mockConfirm } from "./helpers/mock-dialogs";
 import { useMockIsTTY } from "./helpers/mock-istty";
 import { mockKeyListRequest } from "./helpers/mock-kv";
-import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
+import { mockGetMemberships, mockOAuthFlow } from "./helpers/mock-oauth-flow";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 import { writeWorkerSource } from "./helpers/write-worker-source";
@@ -36,7 +36,6 @@ describe("publish", () => {
     mockOAuthServerCallback,
     mockGrantAccessToken,
     mockGrantAuthorization,
-    mockGetMemberships,
   } = mockOAuthFlow();
 
   beforeEach(() => {
@@ -167,10 +166,7 @@ describe("publish", () => {
         mockSubDomainRequest();
         mockUploadWorkerRequest();
         mockOAuthServerCallback();
-        mockGetMemberships({
-          success: true,
-          result: [],
-        });
+        mockGetMemberships([]);
 
         await runWrangler("publish index.js");
 
@@ -195,13 +191,10 @@ describe("publish", () => {
         mockSubDomainRequest();
         mockUploadWorkerRequest();
         mockOAuthServerCallback();
-        mockGetMemberships({
-          success: true,
-          result: [
-            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
-            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
-          ],
-        });
+        mockGetMemberships([
+          { 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(`
@@ -226,13 +219,10 @@ describe("publish", () => {
         mockSubDomainRequest();
         mockUploadWorkerRequest();
         mockOAuthServerCallback();
-        mockGetMemberships({
-          success: true,
-          result: [
-            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
-            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
-          ],
-        });
+        mockGetMemberships([
+          { 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();
 
@@ -255,15 +245,15 @@ describe("publish", () => {
         mockSubDomainRequest();
         mockUploadWorkerRequest();
         mockOAuthServerCallback();
-        mockGetMemberships({
-          success: false,
-          result: [],
-        });
+        mockGetMemberships([]);
 
         await expect(runWrangler("publish index.js")).rejects.toThrowError();
 
         expect(std.err).toMatchInlineSnapshot(`
-          "X [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.
+          "X [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.
 
           "
         `);

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

@@ -9,14 +9,13 @@ import {
   clearConfirmMocks,
   clearPromptMocks,
 } from "./helpers/mock-dialogs";
-import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
+import { mockGetMemberships } from "./helpers/mock-oauth-flow";
 import { useMockStdin } from "./helpers/mock-stdin";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 
 describe("wrangler secret", () => {
   const std = mockConsoleMethods();
-  const { mockGetMemberships } = mockOAuthFlow();
 
   runInTempDir();
   mockAccountId();
@@ -213,15 +212,12 @@ describe("wrangler secret", () => {
         mockAccountId({ accountId: null });
 
         it("should error if a user has no account", async () => {
-          mockGetMemberships({
-            success: false,
-            result: [],
-          });
-          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."`
-          );
+          mockGetMemberships([]);
+          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 () => {
@@ -244,23 +240,20 @@ describe("wrangler secret", () => {
         });
 
         it("should error if a user has multiple accounts, and has not specified an account in wrangler.toml", async () => {
-          mockGetMemberships({
-            success: true,
-            result: [
-              {
-                id: "1",
-                account: { id: "account-id-1", name: "account-name-1" },
-              },
-              {
-                id: "2",
-                account: { id: "account-id-2", name: "account-name-2" },
-              },
-              {
-                id: "3",
-                account: { id: "account-id-3", name: "account-name-3" },
-              },
-            ],
-          });
+          mockGetMemberships([
+            {
+              id: "1",
+              account: { id: "account-id-1", name: "account-name-1" },
+            },
+            {
+              id: "2",
+              account: { id: "account-id-2", name: "account-name-2" },
+            },
+            {
+              id: "3",
+              account: { id: "account-id-3", name: "account-name-3" },
+            },
+          ]);
 
           await expect(runWrangler("secret put the-key --name script-name"))
             .rejects.toThrowErrorMatchingInlineSnapshot(`

packages/wrangler/src/config-cache.ts

@@ -37,7 +37,6 @@ export const saveToConfigCache = <T>(
     configCacheLocation,
     JSON.stringify({ ...existingValues, ...newValues }, null, 2)
   );
-  showCacheMessage();
 };
 
 export const purgeConfigCaches = () => {

packages/wrangler/src/dev/remote.tsx

@@ -1,12 +1,13 @@
 import { readFile } from "node:fs/promises";
 import path from "node:path";
-import { useState, useEffect, useRef } from "react";
+import React, { useState, useEffect, useRef } from "react";
+import { useErrorHandler } from "react-error-boundary";
 import { createWorkerPreview } from "../create-worker-preview";
 import useInspector from "../inspect";
 import { logger } from "../logger";
 import { usePreviewServer } from "../proxy";
 import { syncAssets } from "../sites";
-import { requireApiToken, requireAuth } from "../user";
+import { ChooseAccount, requireApiToken } from "../user";
 import type { CfPreviewToken } from "../create-worker-preview";
 import type { AssetPaths } from "../sites";
 import type { CfModule, CfWorkerInit, CfScriptFormat } from "../worker";
@@ -32,12 +33,13 @@ export function Remote(props: {
   zone: string | undefined;
   host: string | undefined;
 }) {
+  const [accountId, setAccountId] = useState(props.accountId);
   const previewToken = useWorker({
     name: props.name,
     bundle: props.bundle,
     format: props.format,
     modules: props.bundle ? props.bundle.modules : [],
-    accountId: props.accountId,
+    accountId,
     bindings: props.bindings,
     assetPaths: props.assetPaths,
     isWorkersSite: props.isWorkersSite,
@@ -66,7 +68,16 @@ export function Remote(props: {
     port: props.inspectorPort,
     logToTerminal: true,
   });
-  return null;
+
+  const errorHandler = useErrorHandler();
+
+  return !accountId ? (
+    <ChooseAccount
+      isInteractive={true}
+      onSelect={(selectedAccountId) => setAccountId(selectedAccountId)}
+      onError={(err) => errorHandler(err)}
+    ></ChooseAccount>
+  ) : null;
 }
 
 export function useWorker(props: {
@@ -106,13 +117,13 @@ export function useWorker(props: {
   // something's "happened" in our system; We make a ref and
   // mark it once we log our initial message. Refs are vars!
   const startedRef = useRef(false);
-  // This ref holds the actual accountId being used, the `accountId` prop could be undefined
-  // as it is only what is retrieved from the wrangler.toml config.
-  const accountIdRef = useRef(accountId);
 
   useEffect(() => {
     const abortController = new AbortController();
     async function start() {
+      if (accountId === undefined) {
+        return;
+      }
       setToken(undefined); // reset token in case we're re-running
 
       if (!bundle || !format) return;
@@ -123,13 +134,8 @@ export function useWorker(props: {
         logger.log("⎔ Detected changes, restarted server.");
       }
 
-      // Ensure we have an account id, even if it means logging in here.
-      accountIdRef.current = await requireAuth({
-        account_id: accountIdRef.current,
-      });
-
       const assets = await syncAssets(
-        accountIdRef.current,
+        accountId,
         // When we're using the newer service environments, we wouldn't
         // have added the env name on to the script name. However, we must
         // include it in the kv namespace name regardless (since there's no
@@ -182,7 +188,7 @@ export function useWorker(props: {
         await createWorkerPreview(
           init,
           {
-            accountId: accountIdRef.current,
+            accountId,
             apiToken: requireApiToken(),
           },
           {
@@ -207,7 +213,7 @@ export function useWorker(props: {
             "Error: You need to register a workers.dev subdomain before running the dev command in remote mode";
           const solutionMessage =
             "You can either enable local mode by pressing l, or register a workers.dev subdomain here:";
-          const onboardingLink = `https://dash.cloudflare.com/${accountIdRef.current}/workers/onboarding`;
+          const onboardingLink = `https://dash.cloudflare.com/${accountId}/workers/onboarding`;
           logger.error(
             `${errorMessage}\n${solutionMessage}\n${onboardingLink}`
           );

packages/wrangler/src/pages.tsx

@@ -1855,7 +1855,7 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
               account_id: accountId,
             });
 
-            render(<Table data={data}></Table>);
+            render(<Table data={data}></Table>, { patchConsole: false });
           }
         )
         .command(
@@ -2030,7 +2030,7 @@ export const pages: BuilderCallback<unknown, unknown> = (yargs) => {
                 account_id: accountId,
               });
 
-              render(<Table data={data}></Table>);
+              render(<Table data={data}></Table>, { patchConsole: false });
             }
           )
           .command({