fix: fix banner regressions in 3.93.0 (#7493)

emily-shen · penalosa · penalosa · commit 8fb4e0541209 · 2025-01-10T20:19:07.000Z
* telemetry banner fix

* pass configpath into readRawConfig

* changeset

* Don't print banner when --json is provided to versions list and versions view

* fix lint 🤦

* Add tests asserting that versions view and versions list hide the Wrangler banner when --json is passed

* add telemetry test

---------

Co-authored-by: Samuel Macleod &lt;smacleod@cloudflare.com&gt;
diff --git a/.changeset/old-garlics-judge.md b/.changeset/old-garlics-judge.md
@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: remove non-json output in json mode commands
+
+Fixes regressions in 3.93.0 where unwanted text (wrangler banner, telemetry notice) was printing in commands that should only output valid json.
diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts
@@ -426,6 +426,32 @@ describe("metrics", () => {
 					const { permission } = readMetricsConfig();
 					expect(permission?.bannerLastShown).toBeUndefined();
 				});
+
+				it("should *not* print the banner if command is not dev/deploy/docs", async () => {
+					writeMetricsConfig({
+						permission: {
+							enabled: true,
+							date: new Date(2022, 6, 4),
+							bannerLastShown: "1.2.1",
+						},
+					});
+					const requests = mockMetricRequest();
+
+					await runWrangler("telemetry status");
+					expect(std.out).toMatchInlineSnapshot(`
+						"Status: Enabled
+
+						To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`.
+						You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`.
+						Learn more at https://github.com/cloudflare/workers-sdk/tree/main/telemetry.md
+						"
+					`);
+					expect(std.out).not.toContain(
+						"Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md"
+					);
+
+					expect(requests.count).toBe(2);
+				});
 			});
 		});
 
diff --git a/packages/wrangler/src/__tests__/versions/versions.list.test.ts b/packages/wrangler/src/__tests__/versions/versions.list.test.ts
@@ -7,11 +7,15 @@ import { runInTempDir } from "../helpers/run-in-tmp";
 import { runWrangler } from "../helpers/run-wrangler";
 import { writeWranglerConfig } from "../helpers/write-wrangler-config";
 
+vi.unmock("../../wrangler-banner");
+
 describe("versions list", () => {
 	mockAccountId();
 	mockApiToken();
 	runInTempDir();
 	mockConsoleMethods();
+	const cnsl = mockConsoleMethods();
+
 	const std = collectCLIOutput();
 
 	beforeEach(() => {
@@ -72,6 +76,13 @@ describe("versions list", () => {
 				"
 			`);
 
+			expect(cnsl.out).toMatchInlineSnapshot(`
+				"
+				 ⛅️ wrangler 3.93.0
+				-------------------
+				"
+			`);
+
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 
@@ -148,6 +159,8 @@ describe("versions list", () => {
 				"
 			`);
 
+			expect(cnsl.out).toMatchInlineSnapshot(`""`);
+
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 	});
diff --git a/packages/wrangler/src/__tests__/versions/versions.view.test.ts b/packages/wrangler/src/__tests__/versions/versions.view.test.ts
@@ -7,19 +7,21 @@ import { runInTempDir } from "../helpers/run-in-tmp";
 import { runWrangler } from "../helpers/run-wrangler";
 import { writeWranglerConfig } from "../helpers/write-wrangler-config";
 
+vi.unmock("../../wrangler-banner");
+
 describe("versions view", () => {
 	mockAccountId();
 	mockApiToken();
 	runInTempDir();
-	mockConsoleMethods();
+	const cnsl = mockConsoleMethods();
 	const std = collectCLIOutput();
 
 	describe("without wrangler.toml", () => {
 		beforeEach(() => msw.use(mswGetVersion()));
 
 		test("fails with no args", async () => {
 			const result = runWrangler(
-				"versions view  --experimental-gradual-rollouts"
+				"versions view --experimental-gradual-rollouts"
 			);
 
 			await expect(result).rejects.toMatchInlineSnapshot(
@@ -89,6 +91,13 @@ describe("versions view", () => {
 				"
 			`);
 
+			expect(cnsl.out).toMatchInlineSnapshot(`
+				"
+				 ⛅️ wrangler 3.93.0
+				-------------------
+				"
+			`);
+
 			expect(normalizeOutput(std.err)).toMatchInlineSnapshot(`""`);
 		});
 
@@ -99,6 +108,8 @@ describe("versions view", () => {
 
 			await expect(result).resolves.toBeUndefined();
 
+			expect(cnsl.out).toMatchInlineSnapshot(`""`);
+
 			expect(std.out).toMatchInlineSnapshot(`
 				"{
 				  \\"id\\": \\"10000000-0000-0000-0000-000000000000\\",
diff --git a/packages/wrangler/src/__tests__/vitest.setup.ts b/packages/wrangler/src/__tests__/vitest.setup.ts
@@ -83,6 +83,7 @@ vi.mock("undici", async (importOriginal) => {
 vi.mock("../package-manager");
 
 vi.mock("../update-check");
+vi.mock("../wrangler-banner");
 
 beforeAll(() => {
 	msw.listen({
diff --git a/packages/wrangler/src/__tests__/worker-namespace.test.ts b/packages/wrangler/src/__tests__/worker-namespace.test.ts
@@ -1,5 +1,5 @@
 import { http, HttpResponse } from "msw";
-import { printWranglerBanner } from "../update-check";
+import { printWranglerBanner } from "../wrangler-banner";
 import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
 import { mockConsoleMethods } from "./helpers/mock-console";
 import {
diff --git a/packages/wrangler/src/core/register-yargs-command.ts b/packages/wrangler/src/core/register-yargs-command.ts
@@ -3,7 +3,7 @@ import { readConfig } from "../config";
 import { defaultWranglerConfig } from "../config/config";
 import { FatalError, UserError } from "../errors";
 import { logger } from "../logger";
-import { printWranglerBanner } from "../update-check";
+import { printWranglerBanner } from "../wrangler-banner";
 import { demandSingleValue } from "./helpers";
 import type { CommonYargsArgv, SubHelp } from "../yargs-types";
 import type {
@@ -64,7 +64,18 @@ function createHandler(def: CommandDefinition) {
 	return async function handler(args: HandlerArgs<NamedArgDefinitions>) {
 		// eslint-disable-next-line no-useless-catch
 		try {
-			if (def.behaviour?.printBanner !== false) {
+			const shouldPrintBanner = def.behaviour?.printBanner;
+
+			if (
+				/* No defautl behaviour override: show the banner */
+				shouldPrintBanner === undefined ||
+				/* Explicit opt in: show the banner */
+				(typeof shouldPrintBanner === "boolean" &&
+					shouldPrintBanner !== false) ||
+				/* Hook resolves to true */
+				(typeof shouldPrintBanner === "function" &&
+					shouldPrintBanner(args) === true)
+			) {
 				await printWranglerBanner();
 			}
 
diff --git a/packages/wrangler/src/core/types.ts b/packages/wrangler/src/core/types.ts
@@ -93,7 +93,7 @@ export type CommandDefinition<
 		 *
 		 * @default true
 		 */
-		printBanner?: boolean;
+		printBanner?: boolean | ((args: HandlerArgs<NamedArgDefs>) => boolean);
 
 		/**
 		 * By default, wrangler will print warnings about the Wrangler configuration file.
diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts
@@ -148,7 +148,7 @@ import {
 import { tailHandler, tailOptions } from "./tail";
 import registerTriggersSubcommands from "./triggers";
 import { typesHandler, typesOptions } from "./type-generation";
-import { printWranglerBanner, updateCheck } from "./update-check";
+import { updateCheck } from "./update-check";
 import { getAuthFromEnv } from "./user";
 import { loginCommand, logoutCommand, whoamiCommand } from "./user/commands";
 import { whoami } from "./user/whoami";
@@ -176,6 +176,7 @@ import { workflowsInstancesResumeCommand } from "./workflows/commands/instances/
 import { workflowsInstancesTerminateCommand } from "./workflows/commands/instances/terminate";
 import { workflowsListCommand } from "./workflows/commands/list";
 import { workflowsTriggerCommand } from "./workflows/commands/trigger";
+import { printWranglerBanner } from "./wrangler-banner";
 import { asJson } from "./yargs-types";
 import type { Config } from "./config";
 import type { LoggerLevel } from "./logger";
@@ -1110,7 +1111,7 @@ export async function main(argv: string[]): Promise<void> {
 
 		try {
 			const configPath = resolveWranglerConfigPath(args);
-			const rawConfig = readRawConfig(args.config);
+			const rawConfig = readRawConfig(configPath);
 			dispatcher = getMetricsDispatcher({
 				sendMetrics: rawConfig.send_metrics,
 				configPath,
diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts
@@ -82,7 +82,15 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) {
 				) {
 					return;
 				}
-				printMetricsBanner();
+				if (
+					properties.command === "wrangler deploy" ||
+					properties.command === "wrangler dev" ||
+					// for testing purposes
+					properties.command === "wrangler docs"
+				) {
+					printMetricsBanner();
+				}
+
 				const argsUsed = sanitiseUserInput(properties.args ?? {});
 				const argsCombination = argsUsed.sort().join(", ");
 				const commonEventProperties: CommonEventProperties = {
diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts
@@ -14,8 +14,8 @@ import { logger } from "../logger";
 import * as metrics from "../metrics";
 import { isNavigatorDefined } from "../navigator-user-agent";
 import { getBasePath } from "../paths";
-import { printWranglerBanner } from "../update-check";
 import * as shellquote from "../utils/shell-quote";
+import { printWranglerBanner } from "../wrangler-banner";
 import { buildFunctions } from "./buildFunctions";
 import { ROUTES_SPEC_VERSION, SECONDS_TO_WAIT_FOR_PROXY } from "./constants";
 import { FunctionsNoRoutesError, getFunctionsNoRoutesWarning } from "./errors";
diff --git a/packages/wrangler/src/pages/download-config.ts b/packages/wrangler/src/pages/download-config.ts
@@ -9,8 +9,8 @@ import { confirm } from "../dialogs";
 import { FatalError } from "../errors";
 import { logger } from "../logger";
 import * as metrics from "../metrics";
-import { printWranglerBanner } from "../update-check";
 import { requireAuth } from "../user";
+import { printWranglerBanner } from "../wrangler-banner";
 import { PAGES_CONFIG_CACHE_FILENAME } from "./constants";
 import type { RawEnvironment } from "../config";
 import type {
diff --git a/packages/wrangler/src/queues/cli/commands/info.ts b/packages/wrangler/src/queues/cli/commands/info.ts
@@ -1,7 +1,7 @@
 import { readConfig } from "../../../config";
 import { logger } from "../../../logger";
-import { printWranglerBanner } from "../../../update-check";
 import { requireAuth } from "../../../user";
+import { printWranglerBanner } from "../../../wrangler-banner";
 import { getQueue } from "../../client";
 import type {
 	CommonYargsArgv,
diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts
@@ -9,7 +9,7 @@ import { getVarsForDev } from "../dev/dev-vars";
 import { CommandLineArgsError, UserError } from "../errors";
 import { logger } from "../logger";
 import { parseJSONC } from "../parse";
-import { printWranglerBanner } from "../update-check";
+import { printWranglerBanner } from "../wrangler-banner";
 import { generateRuntimeTypes } from "./runtime";
 import { logRuntimeTypesMessage } from "./runtime/log-runtime-types-message";
 import type { Config } from "../config";
diff --git a/packages/wrangler/src/update-check.ts b/packages/wrangler/src/update-check.ts
@@ -1,48 +1,10 @@
-import chalk from "chalk";
-import supportsColor from "supports-color";
 import checkForUpdate from "update-check";
 import {
 	name as wranglerName,
 	version as wranglerVersion,
 } from "../package.json";
-import { logger } from "./logger";
 import type { Result } from "update-check";
 
-export async function printWranglerBanner(performUpdateCheck = true) {
-	let text = ` ⛅️ wrangler ${wranglerVersion}`;
-	let maybeNewVersion: string | undefined;
-	if (performUpdateCheck) {
-		maybeNewVersion = await updateCheck();
-		if (maybeNewVersion !== undefined) {
-			text += ` (update available ${chalk.green(maybeNewVersion)})`;
-		}
-	}
-
-	logger.log(
-		"\n" +
-			text +
-			"\n" +
-			(supportsColor.stdout
-				? chalk.hex("#FF8800")("-".repeat(text.length))
-				: "-".repeat(text.length)) +
-			"\n"
-	);
-
-	// Log a slightly more noticeable message if this is a major bump
-	if (maybeNewVersion !== undefined) {
-		const currentMajor = parseInt(wranglerVersion.split(".")[0]);
-		const newMajor = parseInt(maybeNewVersion.split(".")[0]);
-		if (newMajor > currentMajor) {
-			logger.warn(
-				`The version of Wrangler you are using is now out-of-date.
-Please update to the latest version to prevent critical errors.
-Run \`npm install --save-dev wrangler@${newMajor}\` to update to the latest version.
-After installation, run Wrangler with \`npx wrangler\`.`
-			);
-		}
-	}
-}
-
 async function doUpdateCheck(): Promise<string | undefined> {
 	let update: Result | null = null;
 	// `check-update` only requires the name and version to check. This way we
diff --git a/packages/wrangler/src/versions/deployments/list.ts b/packages/wrangler/src/versions/deployments/list.ts
@@ -4,9 +4,9 @@ import { brandColor, gray } from "@cloudflare/cli/colors";
 import { readConfig } from "../../config";
 import { UserError } from "../../errors";
 import * as metrics from "../../metrics";
-import { printWranglerBanner } from "../../update-check";
 import { requireAuth } from "../../user";
 import formatLabelledValues from "../../utils/render-labelled-values";
+import { printWranglerBanner } from "../../wrangler-banner";
 import { fetchLatestDeployments, fetchVersions } from "../api";
 import { getVersionSource } from "../list";
 import type {
diff --git a/packages/wrangler/src/versions/deployments/status.ts b/packages/wrangler/src/versions/deployments/status.ts
@@ -4,9 +4,9 @@ import { brandColor, gray } from "@cloudflare/cli/colors";
 import { readConfig } from "../../config";
 import { UserError } from "../../errors";
 import * as metrics from "../../metrics";
-import { printWranglerBanner } from "../../update-check";
 import { requireAuth } from "../../user";
 import formatLabelledValues from "../../utils/render-labelled-values";
+import { printWranglerBanner } from "../../wrangler-banner";
 import { fetchLatestDeployment, fetchVersions } from "../api";
 import { getDeploymentSource } from "./list";
 import type {
diff --git a/packages/wrangler/src/versions/deployments/view.ts b/packages/wrangler/src/versions/deployments/view.ts
@@ -1,5 +1,5 @@
 import { UserError } from "../../errors";
-import { printWranglerBanner } from "../../update-check";
+import { printWranglerBanner } from "../../wrangler-banner";
 import type {
 	CommonYargsArgv,
 	StrictYargsOptionsToInterface,
diff --git a/packages/wrangler/src/versions/list.ts b/packages/wrangler/src/versions/list.ts
@@ -15,6 +15,9 @@ export const versionsListCommand = createCommand({
 		owner: "Workers: Authoring and Testing",
 		status: "stable",
 	},
+	behaviour: {
+		printBanner: (args) => !args.json,
+	},
 	args: {
 		name: {
 			describe: "Name of the Worker",
@@ -27,7 +30,7 @@ export const versionsListCommand = createCommand({
 			default: false,
 		},
 	},
-	handler: async function versionsSecretListHandler(args, { config }) {
+	handler: async function versionsListHandler(args, { config }) {
 		metrics.sendMetricsEvent(
 			"list worker versions",
 			{ json: args.json },
diff --git a/packages/wrangler/src/versions/view.ts b/packages/wrangler/src/versions/view.ts
@@ -2,7 +2,6 @@ import { logRaw } from "@cloudflare/cli";
 import { createCommand } from "../core/create-command";
 import { UserError } from "../errors";
 import * as metrics from "../metrics";
-import { printWranglerBanner } from "../update-check";
 import { requireAuth } from "../user";
 import formatLabelledValues from "../utils/render-labelled-values";
 import { fetchVersion } from "./api";
@@ -17,6 +16,9 @@ export const versionsViewCommand = createCommand({
 		owner: "Workers: Authoring and Testing",
 		status: "stable",
 	},
+	behaviour: {
+		printBanner: (args) => !args.json,
+	},
 	args: {
 		"version-id": {
 			describe: "The Worker Version ID to view",
@@ -37,10 +39,6 @@ export const versionsViewCommand = createCommand({
 	},
 	positionalArgs: ["version-id"],
 	handler: async function versionsViewHandler(args, { config }) {
-		if (!args.json) {
-			await printWranglerBanner();
-		}
-
 		metrics.sendMetricsEvent(
 			"view worker version",
 			{},
diff --git a/packages/wrangler/src/wrangler-banner.ts b/packages/wrangler/src/wrangler-banner.ts

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ export type CommandDefinition<`
`93`	`93`	`*`
`94`	`94`	`* @default true`
`95`	`95`	`*/`
`96`		`- printBanner?: boolean;`
	`96`	`+ printBanner?: boolean \| ((args: HandlerArgs<NamedArgDefs>) => boolean);`
`97`	`97`
`98`	`98`	`/**`
`99`	`99`	`* By default, wrangler will print warnings about the Wrangler configuration file.`