diff --git a/.changeset/early-ducks-travel.md b/.changeset/early-ducks-travel.md new file mode 100644 index 000000000000..ba4eeeec5318 --- /dev/null +++ b/.changeset/early-ducks-travel.md @@ -0,0 +1,9 @@ +--- +"wrangler": patch +--- + +fix: Error messaging from failed login would dump a `JSON.parse` error in some situations. Added a fallback if `.json` fails to parse +it will attempt `.text()` then throw result. If both attempts to parse fail it will throw an `UnknownError` with a message showing where +it originated. + +resolves #539 diff --git a/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts b/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts index e5d3a44ce9ef..7b644e01c515 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts @@ -105,11 +105,53 @@ export const mockOAuthFlow = () => { return outcome; }; + const mockExchangeRefreshTokenForAccessToken = ({ + respondWith, + }: { + respondWith: "refreshSuccess" | "refreshError" | "badResponse"; + }) => { + fetchMock.mockOnceIf( + (req) => + (req as Request).url === "https://dash.cloudflare.com/oauth2/token" && + (req as Request).headers.get("Content-Type") === + "application/x-www-form-urlencoded", + async () => { + switch (respondWith) { + case "refreshSuccess": + return JSON.stringify({ + access_token: "access_token_success_mock", + expires_in: 1701, + refresh_token: "refresh_token_sucess_mock", + scope: "scope_success_mock", + token_type: "bearer", + }); + case "refreshError": + return JSON.stringify({ + error: "invalid_request", + error_description: "error_description_mock", + error_hint: "error_hint_mock", + error_verbose: "error_verbose_mock", + status_code: 400, + }); + case "badResponse": + return { + status: 400, + body: ` This shouldn't be sent, but should be handled `, + }; + + default: + return "Not a respondWith option for `mockExchangeRefreshTokenForAccessToken`"; + } + } + ); + }; + return { mockOAuthServerCallback, mockGrantAuthorization, mockRevokeAuthorization, mockGrantAccessToken, + mockExchangeRefreshTokenForAccessToken, }; }; diff --git a/packages/wrangler/src/__tests__/user.test.ts b/packages/wrangler/src/__tests__/user.test.ts index b4b5190fcc4a..bb268c6c63a8 100644 --- a/packages/wrangler/src/__tests__/user.test.ts +++ b/packages/wrangler/src/__tests__/user.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import fetchMock from "jest-fetch-mock"; import { readAuthConfigFile, + requireAuth, USER_AUTH_CONFIG_FILE, writeAuthConfigFile, } from "../user"; @@ -11,9 +12,10 @@ import { mockConsoleMethods } from "./helpers/mock-console"; import { mockOAuthFlow } from "./helpers/mock-oauth-flow"; import { runInTempDir } from "./helpers/run-in-tmp"; import { runWrangler } from "./helpers/run-wrangler"; +import type { Config } from "../config"; import type { UserAuthConfig } from "../user"; -describe("wrangler", () => { +describe("User", () => { runInTempDir({ homedir: "./home" }); const std = mockConsoleMethods(); const { @@ -21,6 +23,7 @@ describe("wrangler", () => { mockGrantAccessToken, mockGrantAuthorization, mockRevokeAuthorization, + mockExchangeRefreshTokenForAccessToken, } = mockOAuthFlow(); describe("login", () => { @@ -82,4 +85,23 @@ describe("wrangler", () => { expect(fs.existsSync(config)).toBeFalsy(); }); }); + + // TODO: Improve OAuth mocking to handle `/token` endpoints from different calls + it("should confirm error message and output for failed login", async () => { + mockOAuthServerCallback(); + writeAuthConfigFile({ + oauth_token: "hunter2", + refresh_token: "Order 66", + }); + mockGrantAuthorization({ respondWith: "success" }); + + mockExchangeRefreshTokenForAccessToken({ + respondWith: "badResponse", + }); + + await expect(requireAuth({} as Config, false)).rejects.toThrowError(); + expect(std.err).toContain( + `Error: This shouldn't be sent, but should be handled ` + ); + }); }); diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index 4617fd9269fc..325bb1d8023d 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -661,8 +661,27 @@ async function exchangeRefreshTokenForAccessToken(): Promise { "Content-Type": "application/x-www-form-urlencoded", }, }); + if (response.status >= 400) { - throw await response.json(); + let tokenExchangeResErr = undefined; + + try { + tokenExchangeResErr = await response.text(); + tokenExchangeResErr = JSON.parse(tokenExchangeResErr); + } catch (e) { + // If it can't parse to JSON ignore the error + } + + if (tokenExchangeResErr !== undefined) { + // We will throw the parsed error if it parsed correctly, otherwise we throw an unknown error. + throw typeof tokenExchangeResErr === "string" + ? new Error(tokenExchangeResErr) + : tokenExchangeResErr; + } else { + throw new ErrorUnknown( + "Failed to parse Error from exchangeRefreshTokenForAccessToken" + ); + } } else { try { const json = (await response.json()) as TokenResponse;