From b9f6310d93bcdec600890b79930a5a4f343cc2e9 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 23 Sep 2025 16:41:51 +0100 Subject: [PATCH 1/2] allow WRANGLER_SEND_METRICS to override whether to report Wrangler crashes to Sentry --- .changeset/angry-apes-share.md | 5 + .../wrangler/src/__tests__/sentry.test.ts | 327 ++++++++++++++++++ packages/wrangler/src/sentry/index.ts | 13 +- 3 files changed, 341 insertions(+), 4 deletions(-) create mode 100644 .changeset/angry-apes-share.md diff --git a/.changeset/angry-apes-share.md b/.changeset/angry-apes-share.md new file mode 100644 index 00000000000..eea9c2b356b --- /dev/null +++ b/.changeset/angry-apes-share.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +allow WRANGLER_SEND_METRICS to override whether to report Wrangler crashes to Sentry diff --git a/packages/wrangler/src/__tests__/sentry.test.ts b/packages/wrangler/src/__tests__/sentry.test.ts index d1e83c9a496..f84e06d05b0 100644 --- a/packages/wrangler/src/__tests__/sentry.test.ts +++ b/packages/wrangler/src/__tests__/sentry.test.ts @@ -123,6 +123,31 @@ describe("sentry", () => { expect(sentryRequests?.length).toEqual(0); }); + it("should not hit sentry (or even ask) after reportable error if WRANGLER_SEND_METRICS is explicitly false", async () => { + // Trigger an API error + msw.use( + http.get( + `https://api.cloudflare.com/client/v4/user`, + async () => { + return HttpResponse.error(); + }, + { once: true } + ), + http.get("*/user/tokens/verify", () => { + return HttpResponse.json(createFetchResult([])); + }) + ); + await expect( + runWrangler("whoami", { WRANGLER_SEND_METRICS: "false" }) + ).rejects.toMatchInlineSnapshot(`[TypeError: Failed to fetch]`); + expect(std.out).toMatchInlineSnapshot(` + "Getting User settings... + + If you think this is a bug then please create an issue at https://github.com/cloudflare/workers-sdk/issues/new/choose" + `); + expect(sentryRequests?.length).toEqual(0); + }); + it("should hit sentry after reportable error when permission provided", async () => { // Trigger an API error msw.use( @@ -428,6 +453,308 @@ describe("sentry", () => { } `); }); + + it("should hit sentry after reportable error (without confirmation) if WRANGLER_SEND_METRICS is explicitly true", async () => { + // Trigger an API error + msw.use( + http.get( + `https://api.cloudflare.com/client/v4/user`, + async () => { + return HttpResponse.error(); + }, + { once: true } + ), + http.get("*/user/tokens/verify", () => { + return HttpResponse.json(createFetchResult([])); + }) + ); + await expect( + runWrangler("whoami", { WRANGLER_SEND_METRICS: "true" }) + ).rejects.toMatchInlineSnapshot(`[TypeError: Failed to fetch]`); + expect(std.out).toMatchInlineSnapshot(` + "Getting User settings... + + If you think this is a bug then please create an issue at https://github.com/cloudflare/workers-sdk/issues/new/choose" + `); + + // Sentry sends multiple HTTP requests to capture breadcrumbs + expect(sentryRequests?.length).toBeGreaterThan(0); + assert(sentryRequests !== undefined); + + // Check requests don't include PII + const envelopes = sentryRequests.map(({ envelope }) => { + const parts = envelope.split("\n").map((line) => JSON.parse(line)); + expect(parts).toHaveLength(3); + return { header: parts[0], type: parts[1], data: parts[2] }; + }); + const event = envelopes.find(({ type }) => type.type === "event"); + assert(event !== undefined); + + // Redact fields with random contents we know don't contain PII + event.header.event_id = ""; + event.header.sent_at = ""; + event.header.trace.trace_id = ""; + event.header.trace.release = ""; + for (const exception of event.data.exception.values) { + for (const frame of exception.stacktrace.frames) { + if ( + frame.filename.startsWith("C:\\Project\\") || + frame.filename.startsWith("/project/") + ) { + frame.filename = "/project/..."; + } + frame.function = ""; + frame.lineno = 0; + frame.colno = 0; + frame.in_app = false; + frame.pre_context = []; + frame.context_line = ""; + frame.post_context = []; + } + } + event.data.event_id = ""; + event.data.contexts.trace.trace_id = ""; + event.data.contexts.trace.span_id = ""; + event.data.contexts.runtime.version = ""; + event.data.contexts.app.app_start_time = ""; + event.data.contexts.app.app_memory = 0; + event.data.contexts.os = {}; + event.data.contexts.device = {}; + event.data.timestamp = 0; + event.data.release = ""; + for (const breadcrumb of event.data.breadcrumbs) { + breadcrumb.timestamp = 0; + } + + const fakeInstallPath = "/wrangler/"; + for (const exception of event.data.exception?.values ?? []) { + for (const frame of exception.stacktrace?.frames ?? []) { + if (frame.module.startsWith("@mswjs")) { + frame.module = + "@mswjs.interceptors.src.interceptors.fetch:index.ts"; + } + if (frame.filename === undefined) { + continue; + } + + const wranglerPackageIndex = frame.filename.indexOf( + path.join("packages", "wrangler", "src") + ); + if (wranglerPackageIndex === -1) { + continue; + } + frame.filename = + fakeInstallPath + + frame.filename + .substring(wranglerPackageIndex) + .replaceAll("\\", "/"); + continue; + } + } + + // If more data is included in the Sentry request, we'll need to verify it + // couldn't contain PII and update this snapshot + expect(event).toStrictEqual({ + data: { + breadcrumbs: [ + { + level: "log", + message: "wrangler whoami", + timestamp: 0, + }, + ], + contexts: { + app: { + app_memory: 0, + app_start_time: "", + }, + cloud_resource: {}, + device: {}, + os: {}, + runtime: { + name: "node", + version: "", + }, + trace: { + span_id: "", + trace_id: "", + }, + }, + environment: "production", + event_id: "", + exception: { + values: [ + { + mechanism: { + handled: true, + type: "generic", + }, + stacktrace: { + frames: [ + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: expect.any(String), + function: "", + in_app: false, + lineno: 0, + module: expect.any(String), + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: "/project/...", + function: "", + in_app: false, + lineno: 0, + module: + "@mswjs.interceptors.src.interceptors.fetch:index.ts", + post_context: [], + pre_context: [], + }, + { + colno: 0, + context_line: "", + filename: "/project/...", + function: "", + in_app: false, + lineno: 0, + module: + "@mswjs.interceptors.src.interceptors.fetch:index.ts", + post_context: [], + pre_context: [], + }, + ], + }, + type: "TypeError", + value: "Failed to fetch", + }, + ], + }, + modules: {}, + platform: "node", + release: "", + sdk: { + integrations: [ + "InboundFilters", + "FunctionToString", + "LinkedErrors", + "Console", + "OnUncaughtException", + "OnUnhandledRejection", + "ContextLines", + "Context", + "Modules", + ], + name: "sentry.javascript.node", + packages: [ + { + name: "npm:@sentry/node", + version: "7.87.0", + }, + ], + version: "7.87.0", + }, + timestamp: 0, + }, + header: { + event_id: "", + sdk: { + name: "sentry.javascript.node", + version: "7.87.0", + }, + sent_at: "", + trace: { + environment: "production", + public_key: "9edbb8417b284aa2bbead9b4c318918b", + release: "", + trace_id: "", + }, + }, + type: { + type: "event", + }, + }); + }); }); }); diff --git a/packages/wrangler/src/sentry/index.ts b/packages/wrangler/src/sentry/index.ts index b29f4d901c3..afbd5665ac3 100644 --- a/packages/wrangler/src/sentry/index.ts +++ b/packages/wrangler/src/sentry/index.ts @@ -3,6 +3,7 @@ import { rejectedSyncPromise } from "@sentry/utils"; import { fetch } from "undici"; import { version as wranglerVersion } from "../../package.json"; import { confirm } from "../dialogs"; +import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; import { logger } from "../logger"; import type { BaseTransportOptions, TransportRequest } from "@sentry/types"; import type { RequestInit } from "undici"; @@ -150,10 +151,14 @@ export function addBreadcrumb( // consent if not already granted. export async function captureGlobalException(e: unknown) { if (typeof SENTRY_DSN !== "undefined") { - sentryReportingAllowed = await confirm( - "Would you like to report this error to Cloudflare? Wrangler's output and the error details will be shared with the Wrangler team to help us diagnose and fix the issue.", - { fallbackValue: false } - ); + const sendMetricsEnvVar = getWranglerSendMetricsFromEnv(); + sentryReportingAllowed = + sendMetricsEnvVar !== undefined + ? sendMetricsEnvVar + : await confirm( + "Would you like to report this error to Cloudflare? Wrangler's output and the error details will be shared with the Wrangler team to help us diagnose and fix the issue.", + { fallbackValue: false } + ); if (!sentryReportingAllowed) { logger.debug(`Sentry: Reporting disabled - would have sent ${e}.`); From 273e4a977ed3b0b34642986712bd23d9255752c5 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 2 Oct 2025 14:45:53 +0100 Subject: [PATCH 2/2] fixup! allow WRANGLER_SEND_METRICS to override whether to report Wrangler crashes to Sentry --- .changeset/angry-apes-share.md | 2 +- .../wrangler/src/__tests__/sentry.test.ts | 8 ++-- .../src/environment-variables/factory.ts | 46 +++++++++++++++++++ .../environment-variables/misc-variables.ts | 13 +++++- packages/wrangler/src/sentry/index.ts | 8 ++-- 5 files changed, 67 insertions(+), 10 deletions(-) diff --git a/.changeset/angry-apes-share.md b/.changeset/angry-apes-share.md index eea9c2b356b..a3e765bdf6b 100644 --- a/.changeset/angry-apes-share.md +++ b/.changeset/angry-apes-share.md @@ -2,4 +2,4 @@ "wrangler": patch --- -allow WRANGLER_SEND_METRICS to override whether to report Wrangler crashes to Sentry +Allow WRANGLER_SEND_ERROR_REPORTS env var to override whether to report Wrangler crashes to Sentry diff --git a/packages/wrangler/src/__tests__/sentry.test.ts b/packages/wrangler/src/__tests__/sentry.test.ts index f84e06d05b0..15ffb405ff2 100644 --- a/packages/wrangler/src/__tests__/sentry.test.ts +++ b/packages/wrangler/src/__tests__/sentry.test.ts @@ -123,7 +123,7 @@ describe("sentry", () => { expect(sentryRequests?.length).toEqual(0); }); - it("should not hit sentry (or even ask) after reportable error if WRANGLER_SEND_METRICS is explicitly false", async () => { + it("should not hit sentry (or even ask) after reportable error if WRANGLER_SEND_ERROR_REPORTS is explicitly false", async () => { // Trigger an API error msw.use( http.get( @@ -138,7 +138,7 @@ describe("sentry", () => { }) ); await expect( - runWrangler("whoami", { WRANGLER_SEND_METRICS: "false" }) + runWrangler("whoami", { WRANGLER_SEND_ERROR_REPORTS: "false" }) ).rejects.toMatchInlineSnapshot(`[TypeError: Failed to fetch]`); expect(std.out).toMatchInlineSnapshot(` "Getting User settings... @@ -454,7 +454,7 @@ describe("sentry", () => { `); }); - it("should hit sentry after reportable error (without confirmation) if WRANGLER_SEND_METRICS is explicitly true", async () => { + it("should hit sentry after reportable error (without confirmation) if WRANGLER_SEND_ERROR_REPORTS is explicitly true", async () => { // Trigger an API error msw.use( http.get( @@ -469,7 +469,7 @@ describe("sentry", () => { }) ); await expect( - runWrangler("whoami", { WRANGLER_SEND_METRICS: "true" }) + runWrangler("whoami", { WRANGLER_SEND_ERROR_REPORTS: "true" }) ).rejects.toMatchInlineSnapshot(`[TypeError: Failed to fetch]`); expect(std.out).toMatchInlineSnapshot(` "Getting User settings... diff --git a/packages/wrangler/src/environment-variables/factory.ts b/packages/wrangler/src/environment-variables/factory.ts index 3b60a48b721..5e417822e44 100644 --- a/packages/wrangler/src/environment-variables/factory.ts +++ b/packages/wrangler/src/environment-variables/factory.ts @@ -1,3 +1,5 @@ +import { UserError } from "../errors"; + type VariableNames = | "CLOUDFLARE_ACCOUNT_ID" | "CLOUDFLARE_API_BASE_URL" @@ -22,6 +24,7 @@ type VariableNames = | "WRANGLER_TOKEN_URL" | "WRANGLER_OUTPUT_FILE_DIRECTORY" | "WRANGLER_OUTPUT_FILE_PATH" + | "WRANGLER_SEND_ERROR_REPORTS" | "WRANGLER_CI_MATCH_TAG" | "WRANGLER_CI_OVERRIDE_NAME" | "WRANGLER_BUILD_CONDITIONS" @@ -35,6 +38,49 @@ type DeprecatedNames = | "CF_EMAIL" | "CF_API_BASE_URL"; +/** + * Create a function used to access a boolean environment variable. It may return undefined if the variable is not set. + * + * This is not memoized to allow us to change the value at runtime, such as in testing. + * + * The environment variable must be either "true" or "false" (after lowercasing), otherwise it will throw an error. + */ +export function getBooleanEnvironmentVariableFactory(options: { + variableName: VariableNames; +}): () => boolean | undefined; +export function getBooleanEnvironmentVariableFactory(options: { + variableName: VariableNames; + defaultValue: boolean | (() => boolean); +}): () => boolean; +export function getBooleanEnvironmentVariableFactory(options: { + variableName: VariableNames; + defaultValue?: boolean | (() => boolean); +}): () => boolean | undefined { + return () => { + if ( + !(options.variableName in process.env) || + process.env[options.variableName] === undefined + ) { + return typeof options.defaultValue === "function" + ? options.defaultValue() + : options.defaultValue; + } + + switch (process.env[options.variableName]?.toLowerCase()) { + case "true": + return true; + case "false": + return false; + default: + throw new UserError( + `Expected ${options.variableName} to be "true" or "false", but got ${JSON.stringify( + process.env[options.variableName] + )}` + ); + } + }; +} + /** * Create a function used to access an environment variable. * diff --git a/packages/wrangler/src/environment-variables/misc-variables.ts b/packages/wrangler/src/environment-variables/misc-variables.ts index c9e4aaa5a7d..acb1049730d 100644 --- a/packages/wrangler/src/environment-variables/misc-variables.ts +++ b/packages/wrangler/src/environment-variables/misc-variables.ts @@ -1,6 +1,9 @@ import path from "node:path"; import { getGlobalWranglerConfigPath } from "../global-wrangler-config-path"; -import { getEnvironmentVariableFactory } from "./factory"; +import { + getBooleanEnvironmentVariableFactory, + getEnvironmentVariableFactory, +} from "./factory"; /** * `WRANGLER_C3_COMMAND` can override the command used by `wrangler init` when delegating to C3. @@ -38,6 +41,14 @@ export const getWranglerSendMetricsFromEnv = getEnvironmentVariableFactory({ variableName: "WRANGLER_SEND_METRICS", }); +/** + * `WRANGLER_SEND_ERROR_REPORTS` can override whether we attempt to send error reports to Sentry. + */ +export const getWranglerSendErrorReportsFromEnv = + getBooleanEnvironmentVariableFactory({ + variableName: "WRANGLER_SEND_ERROR_REPORTS", + }); + /** * Set `WRANGLER_API_ENVIRONMENT` environment variable to "staging" to tell Wrangler to hit the staging APIs rather than production. */ diff --git a/packages/wrangler/src/sentry/index.ts b/packages/wrangler/src/sentry/index.ts index afbd5665ac3..90caf5baad5 100644 --- a/packages/wrangler/src/sentry/index.ts +++ b/packages/wrangler/src/sentry/index.ts @@ -3,7 +3,7 @@ import { rejectedSyncPromise } from "@sentry/utils"; import { fetch } from "undici"; import { version as wranglerVersion } from "../../package.json"; import { confirm } from "../dialogs"; -import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; +import { getWranglerSendErrorReportsFromEnv } from "../environment-variables/misc-variables"; import { logger } from "../logger"; import type { BaseTransportOptions, TransportRequest } from "@sentry/types"; import type { RequestInit } from "undici"; @@ -151,10 +151,10 @@ export function addBreadcrumb( // consent if not already granted. export async function captureGlobalException(e: unknown) { if (typeof SENTRY_DSN !== "undefined") { - const sendMetricsEnvVar = getWranglerSendMetricsFromEnv(); + const sendErrorReportsEnvVar = getWranglerSendErrorReportsFromEnv(); sentryReportingAllowed = - sendMetricsEnvVar !== undefined - ? sendMetricsEnvVar + sendErrorReportsEnvVar !== undefined + ? sendErrorReportsEnvVar : await confirm( "Would you like to report this error to Cloudflare? Wrangler's output and the error details will be shared with the Wrangler team to help us diagnose and fix the issue.", { fallbackValue: false }