feat: add sanitised error messages to wrangler telemetry collection (#…

…7856)

* initial

* feedback and some more examples

* loads of tests + fixups

* changeset + update telemetry docs

* update tests

.changeset/old-teachers-bow.md

@@ -0,0 +1,7 @@
+---
+"wrangler": minor
+---
+
+feat: add sanitised error messages to Wrangler telemetry
+
+Error messages that have been audited for potential inclusion of personal information, and explicitly opted-in, are now included in Wrangler's telemetry collection. Collected error messages will not include any filepaths, user input or any other potentially private content.

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

@@ -0,0 +1,135 @@
+import {
+	CommandLineArgsError,
+	DeprecationError,
+	FatalError,
+	MissingConfigError,
+	UserError,
+} from "../errors";
+import { APIError, ParseError } from "../parse";
+
+describe("errors", () => {
+	describe("UserError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new UserError("message", { telemetryMessage: "telemetry" });
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new UserError("message", { telemetryMessage: true });
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+		});
+	});
+
+	describe("DeprecationError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new DeprecationError("message", {
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("Deprecation:\nmessage");
+			expect(error.telemetryMessage).toBe("telemetry");
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new DeprecationError("message", { telemetryMessage: true });
+			expect(error.message).toBe("Deprecation:\nmessage");
+			expect(error.telemetryMessage).toBe("Deprecation:\nmessage");
+		});
+	});
+
+	describe("FatalError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new FatalError("message", undefined, {
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+			expect(error.code).toBeUndefined();
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new FatalError("message", 1, { telemetryMessage: true });
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+			expect(error.code).toBe(1);
+		});
+	});
+
+	describe("CommandLineArgsError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new CommandLineArgsError("message", {
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new CommandLineArgsError("message", {
+				telemetryMessage: true,
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+		});
+	});
+
+	describe("JsonFriendlyFatalError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new FatalError("message", undefined, {
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+			expect(error.code).toBeUndefined();
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new FatalError("message", 1, { telemetryMessage: true });
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+			expect(error.code).toBe(1);
+		});
+	});
+
+	describe("MissingConfigError", () => {
+		it("just sets the telemetry message as the main message", () => {
+			const error = new MissingConfigError("message");
+			expect(error.message).toBe("Missing config value for message");
+			expect(error.telemetryMessage).toBe("Missing config value for message");
+		});
+	});
+
+	describe("ParseError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new ParseError({
+				text: "message",
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new ParseError({
+				text: "message",
+				telemetryMessage: true,
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+		});
+	});
+
+	describe("APIError", () => {
+		it("takes a custom telemetry message", () => {
+			const error = new APIError({
+				text: "message",
+				telemetryMessage: "telemetry",
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("telemetry");
+		});
+		it("can set telemetryMessage to equal the main message", () => {
+			const error = new APIError({
+				text: "message",
+				telemetryMessage: true,
+			});
+			expect(error.message).toBe("message");
+			expect(error.telemetryMessage).toBe("message");
+		});
+	});
+});

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

@@ -321,6 +321,7 @@ describe("metrics", () => {
 						durationSeconds: 6,
 						durationMinutes: 0.1,
 						errorType: "TypeError",
+						errorMessage: undefined,
 					},
 				};
 
@@ -445,6 +446,34 @@ describe("metrics", () => {
 				expect(std.debug).toContain('"isInteractive":false,');
 			});
 
+			it("should include an error message if the specific error has been allow-listed with {telemetryMessage:true}", async () => {
+				setIsTTY(false);
+				const requests = mockMetricRequest();
+
+				await expect(
+					runWrangler("docs arg -j=false")
+				).rejects.toThrowErrorMatchingInlineSnapshot(
+					`[Error: Wrangler now supports wrangler.json configuration files by default and ignores the value of the \`--experimental-json-config\` flag.]`
+				);
+				expect(requests.count).toBe(2);
+				expect(std.debug).toContain(
+					'"errorMessage":"Wrangler now supports wrangler.json configuration files by default and ignores the value of the `--experimental-json-config` flag."'
+				);
+			});
+
+			it("should include an error message if the specific error has been allow-listed with a custom telemetry message", async () => {
+				setIsTTY(false);
+				const requests = mockMetricRequest();
+
+				await expect(
+					runWrangler("bloop")
+				).rejects.toThrowErrorMatchingInlineSnapshot(
+					`[Error: Unknown argument: bloop]`
+				);
+				expect(requests.count).toBe(2);
+				expect(std.debug).toContain('"errorMessage":"yargs validation error"');
+			});
+
 			describe("banner", () => {
 				beforeEach(() => {
 					vi.mocked(getWranglerVersion).mockReturnValue("1.2.3");

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

@@ -142,6 +142,7 @@ describe("parseTOML", () => {
 					lineText: "name = 'fail\"",
 				},
 				notes: [],
+				telemetryMessage: "TOML parse error",
 			});
 		}
 	});
@@ -163,6 +164,7 @@ describe("parseTOML", () => {
 					fileText: "\n[name",
 				},
 				notes: [],
+				telemetryMessage: "TOML parse error",
 			});
 		}
 	});
@@ -232,6 +234,7 @@ describe("parseJSON", () => {
 					fileText: `\n{\n"version" "1\n}\n`,
 				},
 				notes: [],
+				telemetryMessage: "JSON(C) parse error",
 			});
 			expect(text).toEqual("UnexpectedEndOfString");
 		}
@@ -257,6 +260,7 @@ describe("parseJSON", () => {
 					lineText: `\t\t\t"c":[012345]`,
 				},
 				notes: [],
+				telemetryMessage: "JSON(C) parse error",
 			});
 		}
 	});
@@ -339,6 +343,7 @@ describe("parseJSONC", () => {
 					fileText: `\n{\n"version" "1\n}\n`,
 				},
 				notes: [],
+				telemetryMessage: "JSON(C) parse error",
 			});
 		}
 	});
@@ -363,6 +368,7 @@ describe("parseJSONC", () => {
 					lineText: `\t\t\t"c":[012345]`,
 				},
 				notes: [],
+				telemetryMessage: "JSON(C) parse error",
 			});
 		}
 	});

packages/wrangler/src/assets.ts

@@ -77,7 +77,8 @@ export const syncAssets = async (
 		if (!initializeAssetsResponse.jwt) {
 			throw new FatalError(
 				"Could not find assets information to attach to deployment. Please try again.",
-				1
+				1,
+				{ telemetryMessage: true }
 			);
 		}
 		logger.info(`No files to upload. Proceeding with deployment...`);
@@ -102,7 +103,11 @@ export const syncAssets = async (
 			if (manifestEntry === undefined) {
 				throw new FatalError(
 					`A file was requested that does not appear to exist.`,
-					1
+					1,
+					{
+						telemetryMessage:
+							"A file was requested that does not appear to exist. (asset manifest upload)",
+					}
 				);
 			}
 			// just logging file uploads at the moment...
@@ -182,7 +187,9 @@ export const syncAssets = async (
 					throw new FatalError(
 						`Upload took too long.\n` +
 							`Asset upload took too long on bucket ${bucketIndex + 1}/${initializeAssetsResponse.buckets.length}. Please try again.\n` +
-							`Assets already uploaded have been saved, so the next attempt will automatically resume from this point.`
+							`Assets already uploaded have been saved, so the next attempt will automatically resume from this point.`,
+						undefined,
+						{ telemetryMessage: "Asset upload took too long" }
 					);
 				} else {
 					throw e;
@@ -207,7 +214,8 @@ export const syncAssets = async (
 	if (!completionJwt) {
 		throw new FatalError(
 			"Failed to complete asset upload. Please try again.",
-			1
+			1,
+			{ telemetryMessage: true }
 		);
 	}
 
@@ -249,7 +257,8 @@ const buildAssetManifest = async (dir: string) => {
 					throw new UserError(
 						`Maximum number of assets exceeded.\n` +
 							`Cloudflare Workers supports up to ${MAX_ASSET_COUNT.toLocaleString()} assets in a version. We found ${counter.toLocaleString()} files in the specified assets directory "${dir}".\n` +
-							`Ensure your assets directory contains a maximum of ${MAX_ASSET_COUNT.toLocaleString()} files, and that you have specified your assets directory correctly.`
+							`Ensure your assets directory contains a maximum of ${MAX_ASSET_COUNT.toLocaleString()} files, and that you have specified your assets directory correctly.`,
+						{ telemetryMessage: "Maximum number of assets exceeded" }
 					);
 				}
 
@@ -267,7 +276,8 @@ const buildAssetManifest = async (dir: string) => {
 									binary: true,
 								}
 							)}.\n` +
-							`Ensure all assets in your assets directory "${dir}" conform with the Workers maximum size requirement.`
+							`Ensure all assets in your assets directory "${dir}" conform with the Workers maximum size requirement.`,
+						{ telemetryMessage: "Asset too large" }
 					);
 				}
 				manifest[normalizeFilePath(relativeFilepath)] = {
@@ -335,12 +345,15 @@ export function getAssetsOptions(
 
 	if (directory === undefined) {
 		throw new UserError(
-			"The `assets` property in your configuration is missing the required `directory` property."
+			"The `assets` property in your configuration is missing the required `directory` property.",
+			{ telemetryMessage: true }
 		);
 	}
 
 	if (directory === "") {
-		throw new UserError("`The assets directory cannot be an empty string.");
+		throw new UserError("`The assets directory cannot be an empty string.", {
+			telemetryMessage: true,
+		});
 	}
 
 	const assetsBasePath = getAssetsBasePath(config, args.assets);
@@ -353,7 +366,11 @@ export function getAssetsOptions(
 
 		throw new UserError(
 			`The directory specified by the ${sourceOfTruthMessage} does not exist:\n` +
-				`${resolvedAssetsPath}`
+				`${resolvedAssetsPath}`,
+
+			{
+				telemetryMessage: `The assets directory specified does not exist`,
+			}
 		);
 	}
 
@@ -413,7 +430,11 @@ export function validateAssetsArgsAndConfig(
 	) {
 		throw new UserError(
 			"Cannot use assets and legacy assets in the same Worker.\n" +
-				"Please remove either the `legacy_assets` or `assets` field from your configuration file."
+				"Please remove either the `legacy_assets` or `assets` field from your configuration file.",
+			{
+				telemetryMessage:
+					"Cannot use assets and legacy assets in the same Worker",
+			}
 		);
 	}
 
@@ -440,7 +461,8 @@ export function validateAssetsArgsAndConfig(
 	) {
 		throw new UserError(
 			"Cannot use assets with a binding in an assets-only Worker.\n" +
-				"Please remove the asset binding from your configuration file, or provide a Worker script in your configuration file (`main`)."
+				"Please remove the asset binding from your configuration file, or provide a Worker script in your configuration file (`main`).",
+			{ telemetryMessage: true }
 		);
 	}
 
@@ -481,7 +503,8 @@ export function validateAssetsArgsAndConfig(
 	) {
 		throw new UserError(
 			"Cannot set experimental_serve_directly=false without a Worker script.\n" +
-				"Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (`main`)."
+				"Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (`main`).",
+			{ telemetryMessage: true }
 		);
 	}
 }
@@ -526,12 +549,15 @@ function errorOnLegacyPagesWorkerJSAsset(
 					? "directory"
 					: null;
 		if (workerJsType !== null) {
-			throw new UserError(dedent`
+			throw new UserError(
+				dedent`
 			Uploading a Pages _worker.js ${workerJsType} as an asset.
 			This could expose your private server-side code to the public Internet. Is this intended?
 			If you do not want to upload this ${workerJsType}, either remove it or add an "${CF_ASSETS_IGNORE_FILENAME}" file, to the root of your asset directory, containing "_worker.js" to avoid uploading.
 			If you do want to upload this ${workerJsType}, you can add an empty "${CF_ASSETS_IGNORE_FILENAME}" file, to the root of your asset directory, to hide this error.
-		`);
+		`,
+				{ telemetryMessage: true }
+			);
 		}
 	}
 }

packages/wrangler/src/d1/execute.ts

@@ -116,7 +116,9 @@ export const Handler = async (args: HandlerOptions): Promise<void> => {
 	if (file && command) {
 		throw createFatalError(
 			`Error: can't provide both --command and --file.`,
-			json
+			json,
+			undefined,
+			{ telemetryMessage: true }
 		);
 	}