diff --git a/packages/wrangler/src/__tests__/core/handle-errors.test.ts b/packages/wrangler/src/__tests__/core/handle-errors.test.ts index 7b52c9aa69da..ac1cbf35b1e6 100644 --- a/packages/wrangler/src/__tests__/core/handle-errors.test.ts +++ b/packages/wrangler/src/__tests__/core/handle-errors.test.ts @@ -1,7 +1,244 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { handleError } from "../../core/handle-errors"; +import { getErrorType, handleError } from "../../core/handle-errors"; import { mockConsoleMethods } from "../helpers/mock-console"; +describe("getErrorType", () => { + describe("DNS errors", () => { + it("should return 'DNSError' for ENOTFOUND to api.cloudflare.com", () => { + const error = Object.assign( + new Error("getaddrinfo ENOTFOUND api.cloudflare.com"), + { + code: "ENOTFOUND", + hostname: "api.cloudflare.com", + syscall: "getaddrinfo", + } + ); + + expect(getErrorType(error)).toBe("DNSError"); + }); + + it("should return 'DNSError' for ENOTFOUND to dash.cloudflare.com", () => { + const error = Object.assign( + new Error("getaddrinfo ENOTFOUND dash.cloudflare.com"), + { + code: "ENOTFOUND", + hostname: "dash.cloudflare.com", + } + ); + + expect(getErrorType(error)).toBe("DNSError"); + }); + + it("should return 'DNSError' for DNS errors in error cause", () => { + const cause = Object.assign( + new Error("getaddrinfo ENOTFOUND api.cloudflare.com"), + { + code: "ENOTFOUND", + hostname: "api.cloudflare.com", + } + ); + const error = new Error("Request failed", { cause }); + + expect(getErrorType(error)).toBe("DNSError"); + }); + + it("should NOT return 'DNSError' for non-Cloudflare hostnames", () => { + const error = Object.assign( + new Error("getaddrinfo ENOTFOUND example.com"), + { + code: "ENOTFOUND", + hostname: "example.com", + } + ); + + expect(getErrorType(error)).not.toBe("DNSError"); + }); + }); + + describe("Connection timeout errors", () => { + it("should return 'ConnectionTimeout' for api.cloudflare.com timeouts", () => { + const error = Object.assign( + new Error("Connect Timeout Error: https://api.cloudflare.com/endpoint"), + { code: "UND_ERR_CONNECT_TIMEOUT" } + ); + + expect(getErrorType(error)).toBe("ConnectionTimeout"); + }); + + it("should return 'ConnectionTimeout' for dash.cloudflare.com timeouts", () => { + const error = Object.assign( + new Error("Connect Timeout Error: https://dash.cloudflare.com/api"), + { code: "UND_ERR_CONNECT_TIMEOUT" } + ); + + expect(getErrorType(error)).toBe("ConnectionTimeout"); + }); + + it("should return 'ConnectionTimeout' for timeout errors in error cause", () => { + const cause = Object.assign( + new Error("timeout connecting to api.cloudflare.com"), + { code: "UND_ERR_CONNECT_TIMEOUT" } + ); + const error = new Error("Request failed", { cause }); + + expect(getErrorType(error)).toBe("ConnectionTimeout"); + }); + + it("should return 'ConnectionTimeout' when Cloudflare URL is in parent message", () => { + const cause = Object.assign(new Error("connect timeout"), { + code: "UND_ERR_CONNECT_TIMEOUT", + }); + const error = new Error( + "Failed to connect to https://api.cloudflare.com/client/v4/accounts", + { cause } + ); + + expect(getErrorType(error)).toBe("ConnectionTimeout"); + }); + + it("should NOT return 'ConnectionTimeout' for non-Cloudflare URLs", () => { + const error = Object.assign( + new Error("Connect Timeout Error: https://example.com/api"), + { code: "UND_ERR_CONNECT_TIMEOUT" } + ); + + expect(getErrorType(error)).not.toBe("ConnectionTimeout"); + }); + + it("should NOT return 'ConnectionTimeout' for user's dev server timeouts", () => { + const cause = Object.assign( + new Error("timeout connecting to localhost:8787"), + { code: "UND_ERR_CONNECT_TIMEOUT" } + ); + const error = new Error("Request failed", { cause }); + + expect(getErrorType(error)).not.toBe("ConnectionTimeout"); + }); + }); + + describe("Permission errors", () => { + it("should return 'PermissionError' for EPERM errors", () => { + const error = Object.assign( + new Error( + "EPERM: operation not permitted, open '/Users/user/.wrangler/logs/wrangler.log'" + ), + { + code: "EPERM", + errno: -1, + syscall: "open", + path: "/Users/user/.wrangler/logs/wrangler.log", + } + ); + + expect(getErrorType(error)).toBe("PermissionError"); + }); + + it("should return 'PermissionError' for EACCES errors", () => { + const error = Object.assign( + new Error( + "EACCES: permission denied, open '/Users/user/Library/Preferences/.wrangler/config/default.toml'" + ), + { + code: "EACCES", + errno: -13, + syscall: "open", + path: "/Users/user/Library/Preferences/.wrangler/config/default.toml", + } + ); + + expect(getErrorType(error)).toBe("PermissionError"); + }); + + it("should return 'PermissionError' for EPERM errors without path", () => { + const error = Object.assign( + new Error("EPERM: operation not permitted, mkdir"), + { + code: "EPERM", + } + ); + + expect(getErrorType(error)).toBe("PermissionError"); + }); + + it("should return 'PermissionError' for EPERM errors in error cause", () => { + const cause = Object.assign( + new Error( + "EPERM: operation not permitted, open '/var/logs/wrangler.log'" + ), + { + code: "EPERM", + path: "/var/logs/wrangler.log", + } + ); + const error = new Error("Failed to write to log file", { cause }); + + expect(getErrorType(error)).toBe("PermissionError"); + }); + + it("should NOT return 'PermissionError' for non-EPERM/EACCES errors", () => { + const error = Object.assign(new Error("ENOENT: file not found"), { + code: "ENOENT", + }); + + expect(getErrorType(error)).not.toBe("PermissionError"); + }); + }); + + describe("File not found errors", () => { + it("should return 'FileNotFoundError' for ENOENT errors with path", () => { + const error = Object.assign( + new Error("ENOENT: no such file or directory, open 'wrangler.toml'"), + { + code: "ENOENT", + errno: -2, + syscall: "open", + path: "wrangler.toml", + } + ); + + expect(getErrorType(error)).toBe("FileNotFoundError"); + }); + + it("should return 'FileNotFoundError' for ENOENT errors without path", () => { + const error = Object.assign( + new Error("ENOENT: no such file or directory"), + { + code: "ENOENT", + } + ); + + expect(getErrorType(error)).toBe("FileNotFoundError"); + }); + + it("should return 'FileNotFoundError' for ENOENT errors in error cause", () => { + const cause = Object.assign( + new Error("ENOENT: no such file or directory, stat '.wrangler'"), + { + code: "ENOENT", + path: ".wrangler", + } + ); + const error = new Error("Failed to read directory", { cause }); + + expect(getErrorType(error)).toBe("FileNotFoundError"); + }); + }); + + describe("Fallback behavior", () => { + it("should return constructor name for unknown Error types", () => { + const error = new TypeError("Something went wrong"); + + expect(getErrorType(error)).toBe("TypeError"); + }); + + it("should return undefined for non-Error values", () => { + expect(getErrorType("string error")).toBe(undefined); + expect(getErrorType(null)).toBe(undefined); + expect(getErrorType(undefined)).toBe(undefined); + }); + }); +}); + describe("handleError", () => { const std = mockConsoleMethods(); @@ -96,9 +333,8 @@ describe("handleError", () => { { code: "UND_ERR_CONNECT_TIMEOUT" } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("ConnectionTimeout"); expect(std.err).toContain("The request to Cloudflare's API timed out"); expect(std.err).toContain("network connectivity issues"); expect(std.err).toContain("Please check your internet connection"); @@ -110,9 +346,8 @@ describe("handleError", () => { { code: "UND_ERR_CONNECT_TIMEOUT" } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("ConnectionTimeout"); expect(std.err).toContain("The request to Cloudflare's API timed out"); }); @@ -123,9 +358,8 @@ describe("handleError", () => { ); const error = new Error("Request failed", { cause }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("ConnectionTimeout"); expect(std.err).toContain("The request to Cloudflare's API timed out"); }); @@ -138,9 +372,8 @@ describe("handleError", () => { { cause } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("ConnectionTimeout"); expect(std.err).toContain("The request to Cloudflare's API timed out"); }); @@ -150,9 +383,8 @@ describe("handleError", () => { { code: "UND_ERR_CONNECT_TIMEOUT" } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).not.toBe("ConnectionTimeout"); expect(std.err).not.toContain( "The request to Cloudflare's API timed out" ); @@ -165,9 +397,8 @@ describe("handleError", () => { ); const error = new Error("Request failed", { cause }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).not.toBe("ConnectionTimeout"); expect(std.err).not.toContain( "The request to Cloudflare's API timed out" ); @@ -188,9 +419,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("PermissionError"); expect(std.err).toContain( "A permission error occurred while accessing the file system" ); @@ -213,9 +443,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("PermissionError"); expect(std.err).toContain( "A permission error occurred while accessing the file system" ); @@ -233,9 +462,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("PermissionError"); expect(std.err).toContain( "A permission error occurred while accessing the file system" ); @@ -255,9 +483,8 @@ describe("handleError", () => { ); const error = new Error("Failed to write to log file", { cause }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("PermissionError"); expect(std.err).toContain( "A permission error occurred while accessing the file system" ); @@ -269,9 +496,8 @@ describe("handleError", () => { code: "ENOENT", }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).not.toBe("PermissionError"); expect(std.err).not.toContain( "A permission error occurred while accessing the file system" ); @@ -289,9 +515,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("DNSError"); expect(std.err).toContain("Unable to resolve Cloudflare's API hostname"); expect(std.err).toContain("api.cloudflare.com or dash.cloudflare.com"); expect(std.err).toContain("No internet connection"); @@ -307,9 +532,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("DNSError"); expect(std.err).toContain("Unable to resolve Cloudflare's API hostname"); }); @@ -323,9 +547,8 @@ describe("handleError", () => { ); const error = new Error("Request failed", { cause }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("DNSError"); expect(std.err).toContain("Unable to resolve Cloudflare's API hostname"); }); @@ -338,9 +561,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).not.toBe("DNSError"); expect(std.err).not.toContain( "Unable to resolve Cloudflare's API hostname" ); @@ -359,9 +581,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("FileNotFoundError"); expect(std.err).toContain("A file or directory could not be found"); expect(std.err).toContain("Missing file or directory: wrangler.toml"); expect(std.err).toContain("The file or directory does not exist"); @@ -375,9 +596,8 @@ describe("handleError", () => { } ); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("FileNotFoundError"); expect(std.err).toContain("A file or directory could not be found"); expect(std.err).toContain("Error: ENOENT: no such file or directory"); expect(std.err).not.toContain("Missing file or directory:"); @@ -393,9 +613,8 @@ describe("handleError", () => { ); const error = new Error("Failed to read directory", { cause }); - const errorType = await handleError(error, {}, []); + await handleError(error, {}, []); - expect(errorType).toBe("FileNotFoundError"); expect(std.err).toContain("A file or directory could not be found"); expect(std.err).toContain("Missing file or directory: .wrangler"); }); diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index 248fc9ac69b6..02ecdecacf30 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -18,7 +18,10 @@ import { readMetricsConfig, writeMetricsConfig, } from "../metrics/metrics-config"; -import { getMetricsDispatcher } from "../metrics/metrics-dispatcher"; +import { + getMetricsDispatcher, + waitForAllMetricsDispatches, +} from "../metrics/metrics-dispatcher"; import { sniffUserAgent } from "../package-manager"; import { mockConsoleMethods } from "./helpers/mock-console"; import { useMockIsTTY } from "./helpers/mock-istty"; @@ -79,7 +82,8 @@ describe("metrics", () => { }); }); - afterEach(() => { + afterEach(async () => { + await waitForAllMetricsDispatches(); vi.useRealTimers(); }); @@ -101,7 +105,7 @@ describe("metrics", () => { sendMetrics: true, }); dispatcher.sendAdhocEvent("some-event", { a: 1, b: 2 }); - await Promise.all(dispatcher.requests); + await waitForAllMetricsDispatches(); expect(requests.count).toBe(1); expect(std.debug).toMatchInlineSnapshot( `"Metrics dispatcher: Posting data {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"wranglerMajorVersion\\":1,\\"wranglerMinorVersion\\":2,\\"wranglerPatchVersion\\":3,\\"os\\":\\"foo:bar\\",\\"agent\\":null,\\"a\\":1,\\"b\\":2}}"` @@ -118,7 +122,7 @@ describe("metrics", () => { sendMetrics: true, }); dispatcher.sendAdhocEvent("version-test"); - await Promise.all(dispatcher.requests); + await waitForAllMetricsDispatches(); expect(requests.count).toBe(1); expect(std.debug).toContain('"wranglerVersion":"1.2.3"'); expect(std.debug).toContain('"wranglerMajorVersion":1'); @@ -152,7 +156,7 @@ describe("metrics", () => { sendMetrics: true, }); dispatcher.sendAdhocEvent("some-event", { a: 1, b: 2 }); - await Promise.all(dispatcher.requests); + await waitForAllMetricsDispatches(); expect(std.debug).toMatchInlineSnapshot(` "Metrics dispatcher: Posting data {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"wranglerMajorVersion\\":1,\\"wranglerMinorVersion\\":2,\\"wranglerPatchVersion\\":3,\\"os\\":\\"foo:bar\\",\\"agent\\":null,\\"a\\":1,\\"b\\":2}} @@ -194,7 +198,7 @@ describe("metrics", () => { sendMetrics: true, }); dispatcher.sendAdhocEvent("some-event", { a: 1 }); - await Promise.all(dispatcher.requests); + await waitForAllMetricsDispatches(); expect(requests.count).toBe(1); expect(std.debug).toContain('"agent":"claude-code"'); @@ -210,34 +214,13 @@ describe("metrics", () => { sendMetrics: true, }); dispatcher.sendAdhocEvent("some-event", { a: 1 }); - await Promise.all(dispatcher.requests); + await waitForAllMetricsDispatches(); expect(requests.count).toBe(1); expect(std.debug).toContain('"agent":null'); }); }); - it("should keep track of all requests made", async () => { - const requests = mockMetricRequest(); - const dispatcher = getMetricsDispatcher({ - sendMetrics: true, - }); - - dispatcher.sendAdhocEvent("some-event", { a: 1, b: 2 }); - expect(dispatcher.requests.length).toBe(1); - - expect(requests.count).toBe(0); - await Promise.allSettled(dispatcher.requests); - expect(requests.count).toBe(1); - - dispatcher.sendAdhocEvent("another-event", { c: 3, d: 4 }); - expect(dispatcher.requests.length).toBe(2); - - expect(requests.count).toBe(1); - await Promise.allSettled(dispatcher.requests); - expect(requests.count).toBe(2); - }); - describe("sendCommandEvent()", () => { const reused = { wranglerVersion: "1.2.3", @@ -336,10 +319,10 @@ describe("metrics", () => { ); expect(std.out).toMatchInlineSnapshot(` " - Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md - ⛅️ wrangler x.x.x ────────────────── + + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md Opening a link in your default browser: FAKE_DOCS_URL:{\\"params\\":\\"query=arg&hitsPerPage=1&getRankingInfo=0\\"}" `); expect(std.warn).toMatchInlineSnapshot(`""`); @@ -476,10 +459,10 @@ describe("metrics", () => { ); expect(std.out).toMatchInlineSnapshot(` " - Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md - ⛅️ wrangler x.x.x ────────────────── + + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md Opening a link in your default browser: FAKE_DOCS_URL:{\\"params\\":\\"query=arg&hitsPerPage=1&getRankingInfo=0\\"}" `); expect(std.warn).toMatchInlineSnapshot(`""`); @@ -581,10 +564,10 @@ describe("metrics", () => { await runWrangler("docs arg"); expect(std.out).toMatchInlineSnapshot(` " - Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md - ⛅️ wrangler x.x.x ────────────────── + + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md Opening a link in your default browser: FAKE_DOCS_URL:{\\"params\\":\\"query=arg&hitsPerPage=1&getRankingInfo=0\\"}" `); @@ -616,10 +599,10 @@ describe("metrics", () => { await runWrangler("docs arg"); expect(std.out).toMatchInlineSnapshot(` " - Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md - ⛅️ wrangler x.x.x ────────────────── + + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/packages/wrangler/telemetry.md Opening a link in your default browser: FAKE_DOCS_URL:{\\"params\\":\\"query=arg&hitsPerPage=1&getRankingInfo=0\\"}" `); expect(requests.count).toBe(2); diff --git a/packages/wrangler/src/core/CommandHandledError.ts b/packages/wrangler/src/core/CommandHandledError.ts new file mode 100644 index 000000000000..dfc3f78f26ce --- /dev/null +++ b/packages/wrangler/src/core/CommandHandledError.ts @@ -0,0 +1,10 @@ +/** + * A wrapper Error that indicates the original error was thrown during Command handler execution. + * + * This is used to distinguish between: + * - Errors from within command handlers: telemetry and error reporting already sent by handler. + * - Yargs validation errors: telemetry and error reporting needs to be sent in the yargs middleware. + */ +export class CommandHandledError { + constructor(public readonly originalError: unknown) {} +} diff --git a/packages/wrangler/src/core/handle-errors.ts b/packages/wrangler/src/core/handle-errors.ts index 4752a0702c5e..62986fea5c59 100644 --- a/packages/wrangler/src/core/handle-errors.ts +++ b/packages/wrangler/src/core/handle-errors.ts @@ -247,6 +247,32 @@ function isNetworkFetchFailedError(e: unknown): boolean { return false; } +/** + * Determines the error type for telemetry purposes, or `undefined` if it cannot be determined. + */ +export function getErrorType(e: unknown): string | undefined { + if (isCloudflareAPIDNSError(e)) { + return "DNSError"; + } + if (isPermissionError(e)) { + return "PermissionError"; + } + if (isFileNotFoundError(e)) { + return "FileNotFoundError"; + } + if (isCloudflareAPIConnectionTimeoutError(e)) { + return "ConnectionTimeout"; + } + if (isAuthenticationError(e) || isContainersAuthenticationError(e)) { + return "AuthenticationError"; + } + if (isBuildFailure(e) || isBuildFailureFromCause(e)) { + return "BuildFailure"; + } + // Fallback to constructor name + return e instanceof Error ? e.constructor.name : undefined; +} + /** * Handles an error thrown during command execution. * @@ -256,9 +282,8 @@ export async function handleError( e: unknown, args: ReadConfigCommandArgs, subCommandParts: string[] -) { +): Promise { let mayReport = true; - let errorType: string | undefined; let loggableException = e; logger.log(""); // Just adds a bit of space @@ -275,7 +300,6 @@ export async function handleError( // Handle DNS resolution errors to Cloudflare API with a user-friendly message if (isCloudflareAPIDNSError(e)) { mayReport = false; - errorType = "DNSError"; logger.error(dedent` Unable to resolve Cloudflare's API hostname (api.cloudflare.com or dash.cloudflare.com). @@ -287,13 +311,12 @@ export async function handleError( Please check your network connection and DNS settings. `); - return errorType; + return; } // Handle permission errors with a user-friendly message if (isPermissionError(e)) { mayReport = false; - errorType = "PermissionError"; // Extract the error message and path, checking both the error and its cause const errorMessage = e instanceof Error ? e.message : String(e); @@ -339,13 +362,12 @@ export async function handleError( Please check the file permissions and try again. `); - return errorType; + return; } // Handle file not found errors with a user-friendly message if (isFileNotFoundError(e)) { mayReport = false; - errorType = "FileNotFoundError"; // Extract the error message and path, checking both the error and its cause const errorMessage = e instanceof Error ? e.message : String(e); @@ -390,19 +412,18 @@ export async function handleError( Please check the file path and try again. `); - return errorType; + return; } // Handle connection timeout errors to Cloudflare API with a user-friendly message if (isCloudflareAPIConnectionTimeoutError(e)) { mayReport = false; - errorType = "ConnectionTimeout"; logger.error( "The request to Cloudflare's API timed out.\n" + "This is likely due to network connectivity issues or slow network speeds.\n" + "Please check your internet connection and try again." ); - return errorType; + return; } // Handle generic "fetch failed" / "Failed to fetch" network errors @@ -448,16 +469,8 @@ export async function handleError( } await wrangler.parse(); - } else if ( - isAuthenticationError(e) || - // Is this a Containers/Cloudchamber-based auth error? - // This is different because it uses a custom OpenAPI-based generated client - (e instanceof UserError && - e.cause instanceof ApiError && - e.cause.status === 403) - ) { + } else if (isAuthenticationError(e) || isContainersAuthenticationError(e)) { mayReport = false; - errorType = "AuthenticationError"; if (e.cause instanceof ApiError) { logger.error(e.cause); } else { @@ -519,12 +532,9 @@ export async function handleError( ); } else if (isBuildFailure(e)) { mayReport = false; - errorType = "BuildFailure"; - logBuildFailure(e.errors, e.warnings); } else if (isBuildFailureFromCause(e)) { mayReport = false; - errorType = "BuildFailure"; logBuildFailure(e.cause.errors, e.cause.warnings); } else if (e instanceof Cloudflare.APIError) { const error = new APIError({ @@ -576,6 +586,17 @@ export async function handleError( ) { await captureGlobalException(loggableException); } +} - return errorType; +/** + * Is this a Containers/Cloudchamber-based auth error? + * + * This is different because it uses a custom OpenAPI-based generated client + */ +function isContainersAuthenticationError(e: unknown): e is UserError { + return ( + e instanceof UserError && + e.cause instanceof ApiError && + e.cause.status === 403 + ); } diff --git a/packages/wrangler/src/core/register-yargs-command.ts b/packages/wrangler/src/core/register-yargs-command.ts index 517ef62494d3..a17ffa69b02c 100644 --- a/packages/wrangler/src/core/register-yargs-command.ts +++ b/packages/wrangler/src/core/register-yargs-command.ts @@ -1,3 +1,4 @@ +import { UserError as ContainersUserError } from "@cloudflare/containers-shared/src/error"; import { defaultWranglerConfig, FatalError, @@ -11,10 +12,13 @@ import { createCloudflareClient } from "../cfetch/internal"; import { readConfig } from "../config"; import { run } from "../experimental-flags"; import { logger } from "../logger"; +import { getMetricsDispatcher } from "../metrics"; import { writeOutput } from "../output"; import { dedent } from "../utils/dedent"; import { isLocal, printResourceLocation } from "../utils/is-local"; import { printWranglerBanner } from "../wrangler-banner"; +import { CommandHandledError } from "./CommandHandledError"; +import { getErrorType, handleError } from "./handle-errors"; import { demandSingleValue } from "./helpers"; import type { CommonYargsArgv, SubHelp } from "../yargs-types"; import type { @@ -30,7 +34,8 @@ import type { PositionalOptions } from "yargs"; */ export function createRegisterYargsCommand( yargs: CommonYargsArgv, - subHelp: SubHelp + subHelp: SubHelp, + argv: string[] ) { return function registerCommand( segment: string, @@ -97,13 +102,19 @@ export function createRegisterYargsCommand( registerSubTreeCallback(); }, // Only attach the handler for commands, not namespaces - def.type === "command" ? createHandler(def, def.command) : undefined + def.type === "command" ? createHandler(def, def.command, argv) : undefined ); }; } -function createHandler(def: CommandDefinition, commandName: string) { +function createHandler( + def: CommandDefinition, + commandName: string, + argv: string[] +) { return async function handler(args: HandlerArgs) { + const startTime = Date.now(); + try { const shouldPrintBanner = def.behaviour?.printBanner; @@ -165,7 +176,7 @@ function createHandler(def: CommandDefinition, commandName: string) { AUTOCREATE_RESOURCES: args.experimentalAutoCreate, }; - await run(experimentalFlags, () => { + await run(experimentalFlags, async () => { const config = def.behaviour?.provideConfig ?? true ? readConfig(args, { @@ -175,6 +186,13 @@ function createHandler(def: CommandDefinition, commandName: string) { }) : defaultWranglerConfig; + const dispatcher = getMetricsDispatcher({ + sendMetrics: config.send_metrics, + hasAssets: !!config.assets?.directory, + configPath: config.configPath, + argv, + }); + if (def.behaviour?.warnIfMultipleEnvsConfiguredButNoneSpecified) { if (!("env" in args) && config.configPath) { const { rawConfig } = experimental_readRawConfig( @@ -196,28 +214,70 @@ function createHandler(def: CommandDefinition, commandName: string) { } } - return def.handler(args, { - sdk: createCloudflareClient(config), - config, - errors: { UserError, FatalError }, - logger, - fetchResult, + dispatcher.sendCommandEvent("wrangler command started", { + command: commandName, + args, }); - }); - // TODO(telemetry): send command completed event - } catch (err) { - // TODO(telemetry): send command errored event + try { + const result = await def.handler(args, { + sdk: createCloudflareClient(config), + config, + errors: { UserError, FatalError }, + logger, + fetchResult, + }); + + const durationMs = Date.now() - startTime; + dispatcher.sendCommandEvent("wrangler command completed", { + command: commandName, + args, + durationMs, + durationSeconds: durationMs / 1000, + durationMinutes: durationMs / 1000 / 60, + }); + return result; + } catch (err) { + // If the error is already a CommandHandledError (e.g., from a nested wrangler.parse() call), + // don't wrap it again; just rethrow. + if (err instanceof CommandHandledError) { + throw err; + } + + const durationMs = Date.now() - startTime; + dispatcher.sendCommandEvent("wrangler command errored", { + command: commandName, + args, + durationMs, + durationSeconds: durationMs / 1000, + durationMinutes: durationMs / 1000 / 60, + errorType: getErrorType(err), + errorMessage: + err instanceof UserError || err instanceof ContainersUserError + ? err.telemetryMessage + : undefined, + }); + + await handleError(err, args, argv); + + // Wrap the error to signal that the telemetry has already been sent and the error reporting handled. + throw new CommandHandledError(err); + } + }); + } catch (err) { // Write handler failure to output file if one exists - if (err instanceof Error) { - const code = "code" in err ? (err.code as number) : undefined; - const message = "message" in err ? (err.message as string) : undefined; + // Unwrap CommandHandledError to get the original error for output + const outputErr = + err instanceof CommandHandledError ? err.originalError : err; + if (outputErr instanceof Error) { + const code = + "code" in outputErr ? (outputErr.code as number) : undefined; writeOutput({ type: "command-failed", version: 1, code, - message, + message: outputErr.message, }); } throw err; diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index 12684a58e541..bc14d9dd4e26 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -30,8 +30,9 @@ import { completionsCommand } from "./complete"; import { getDefaultEnvFiles, loadDotEnv } from "./config/dot-env"; import { containers } from "./containers"; import { demandSingleValue } from "./core"; +import { CommandHandledError } from "./core/CommandHandledError"; import { CommandRegistry } from "./core/CommandRegistry"; -import { handleError } from "./core/handle-errors"; +import { getErrorType, handleError } from "./core/handle-errors"; import { createRegisterYargsCommand } from "./core/register-yargs-command"; import { d1Namespace } from "./d1"; import { d1CreateCommand } from "./d1/create"; @@ -90,7 +91,7 @@ import { kvNamespaceRenameCommand, } from "./kv"; import { logger, LOGGER_LEVELS } from "./logger"; -import { getMetricsDispatcher } from "./metrics"; +import { getMetricsDispatcher, waitForAllMetricsDispatches } from "./metrics"; import { metricsAlias, telemetryDisableCommand, @@ -345,6 +346,7 @@ import { workflowsInstancesTerminateAllCommand } from "./workflows/commands/inst import { workflowsListCommand } from "./workflows/commands/list"; import { workflowsTriggerCommand } from "./workflows/commands/trigger"; import { printWranglerBanner } from "./wrangler-banner"; +import type { ReadConfigCommandArgs } from "./config"; import type { LoggerLevel } from "./logger"; import type { CommonYargsArgv, SubHelp } from "./yargs-types"; @@ -492,7 +494,7 @@ export function createCLIParser(argv: string[]) { }, }; - const registerCommand = createRegisterYargsCommand(wrangler, subHelp); + const registerCommand = createRegisterYargsCommand(wrangler, subHelp, argv); const registry = new CommandRegistry(registerCommand); // Helper to show help with command categories @@ -1742,8 +1744,6 @@ export async function main(argv: string[]): Promise { checkMacOSVersion({ shouldThrow: false }); - const startTime = Date.now(); - // Check if this is a root-level help request (--help or -h with no subcommand) // In this case, we use our custom help formatter to show command categories const isRootHelpRequest = @@ -1756,10 +1756,8 @@ export async function main(argv: string[]): Promise { await showHelpWithCategories(); return; } - let command: string | undefined; - let metricsArgs: Record | undefined; - let dispatcher: ReturnType | undefined; - // Register Yargs middleware to record command as Sentry breadcrumb + + // Register Yargs middleware to record command as Sentry breadcrumb and set logger level let recordedCommand = false; const wranglerWithMiddleware = wrangler.middleware((args) => { // Update logger level, before we do any logging @@ -1774,8 +1772,26 @@ export async function main(argv: string[]): Promise { return; } recordedCommand = true; - // `args._` doesn't include any positional arguments (e.g. script name, - // key to fetch) or flags + + // Record command as Sentry breadcrumb + const command = `wrangler ${args._.join(" ")}`; + addBreadcrumb(command); + + // TODO: Legacy commands (cloudchamber, containers) don't use defineCommand + // and won't emit telemetry events. Migrate them to defineCommand to enable telemetry. + }, /* applyBeforeValidation */ true); + + const startTime = Date.now(); + let command: string | undefined; + let configArgs: ReadConfigCommandArgs = {}; + let dispatcher: ReturnType | undefined; + + // Register middleware to capture command info for fallback telemetry + const wranglerWithTelemetry = wranglerWithMiddleware.middleware((args) => { + // Capture command and args for potential fallback telemetry + // (used when yargs validation errors occur before handler runs) + command = `wrangler ${args._.join(" ")}`; + configArgs = args; try { const { rawConfig, configPath } = experimental_readRawConfig(args); @@ -1783,66 +1799,42 @@ export async function main(argv: string[]): Promise { sendMetrics: rawConfig.send_metrics, hasAssets: !!rawConfig.assets?.directory, configPath, + argv, }); } catch (e) { - // If we can't parse the config, we can't send metrics - logger.debug("Failed to parse config. Disabling metrics dispatcher.", e); + // If we can't parse the config, we can still send metrics with defaults + logger.debug("Failed to parse config for metrics. Using defaults.", e); + dispatcher = getMetricsDispatcher({ argv }); } - - command = `wrangler ${args._.join(" ")}`; - metricsArgs = args; - addBreadcrumb(command); - // NB despite 'applyBeforeValidation = true', this runs *after* yargs 'validates' options, - // e.g. if a required arg is missing, yargs will error out before we send any events :/ - dispatcher?.sendCommandEvent( - "wrangler command started", - { - command, - args, - }, - argv - ); }, /* applyBeforeValidation */ true); let cliHandlerThrew = false; try { - await wranglerWithMiddleware.parse(); - - const durationMs = Date.now() - startTime; - - dispatcher?.sendCommandEvent( - "wrangler command completed", - { - command, - args: metricsArgs, - durationMs, - durationSeconds: durationMs / 1000, - durationMinutes: durationMs / 1000 / 60, - }, - argv - ); + await wranglerWithTelemetry.parse(); } catch (e) { cliHandlerThrew = true; - const errorType = await handleError(e, wrangler.arguments, argv); - const durationMs = Date.now() - startTime; - dispatcher?.sendCommandEvent( - "wrangler command errored", - { - command, - args: metricsArgs, - durationMs, - durationSeconds: durationMs / 1000, - durationMinutes: durationMs / 1000 / 60, - errorType: - errorType ?? (e instanceof Error ? e.constructor.name : undefined), - errorMessage: - e instanceof UserError || e instanceof ContainersUserError - ? e.telemetryMessage - : undefined, - }, - argv - ); - throw e; + + if (e instanceof CommandHandledError) { + // This error occurred during Command handler execution, + // and has already sent metrics and reported to the user. + // So we can just re-throw the original error. + throw e.originalError; + } else { + // The error occurred before Command handler ran + // (e.g., yargs validation errors like unknown commands or invalid arguments). + // So we need to handle telemetry and error reporting here. + if (dispatcher && command) { + dispatchGenericCommandErrorEvent( + dispatcher, + command, + configArgs, + startTime, + e + ); + } + await handleError(e, configArgs, argv); + throw e; + } } finally { try { // In the bootstrapper script `bin/wrangler.js`, we open an IPC channel, @@ -1859,8 +1851,9 @@ export async function main(argv: string[]): Promise { await closeSentry(); + // Wait for any pending telemetry requests to complete (with timeout) await Promise.race([ - Promise.allSettled(dispatcher?.requests ?? []), + waitForAllMetricsDispatches(), setTimeout(1000, undefined, { ref: false }), ]); } catch (e) { @@ -1874,3 +1867,36 @@ export async function main(argv: string[]): Promise { } } } + +/** + * Dispatches generic metrics events to indicate that a wrangler command errored + * when we don't know the CommandDefinition and cannot be sure what is safe to send. + */ +function dispatchGenericCommandErrorEvent( + dispatcher: ReturnType, + command: string, + configArgs: ReadConfigCommandArgs, + startTime: number, + error: unknown +) { + const durationMs = Date.now() - startTime; + + // Send "started" event since handler never got to send it. + dispatcher.sendCommandEvent("wrangler command started", { + command, + args: configArgs, + }); + + dispatcher.sendCommandEvent("wrangler command errored", { + command, + args: configArgs, + durationMs, + durationSeconds: durationMs / 1000, + durationMinutes: durationMs / 1000 / 60, + errorType: getErrorType(error), + errorMessage: + error instanceof UserError || error instanceof ContainersUserError + ? error.telemetryMessage + : undefined, + }); +} diff --git a/packages/wrangler/src/metrics/index.ts b/packages/wrangler/src/metrics/index.ts index 20405cf2dad1..7aee0451da19 100644 --- a/packages/wrangler/src/metrics/index.ts +++ b/packages/wrangler/src/metrics/index.ts @@ -1,4 +1,7 @@ -export { getMetricsDispatcher } from "./metrics-dispatcher"; +export { + getMetricsDispatcher, + waitForAllMetricsDispatches, +} from "./metrics-dispatcher"; export { getMetricsConfig } from "./metrics-config"; export * from "./send-event"; export { getMetricsUsageHeaders } from "./metrics-usage-headers"; diff --git a/packages/wrangler/src/metrics/metrics-config.ts b/packages/wrangler/src/metrics/metrics-config.ts index 9ec7dd52d6e5..7f56e6bf7280 100644 --- a/packages/wrangler/src/metrics/metrics-config.ts +++ b/packages/wrangler/src/metrics/metrics-config.ts @@ -19,6 +19,10 @@ import { logger } from "../logger"; export const CURRENT_METRICS_DATE = new Date(2022, 6, 4); export interface MetricsConfigOptions { + /** + * The argv passed to the `main()` function. + */ + argv?: string[]; /** * Defines whether to send metrics to Cloudflare: * If defined, then use this value for whether the dispatch is enabled. diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 782a8179c1c6..86cf2454ad56 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -30,6 +30,18 @@ import type { CommonEventProperties, Events } from "./types"; const SPARROW_URL = "https://sparrow.cloudflare.com"; +// Module-level Set to track all pending requests across all dispatchers. +// Promises are automatically removed from this Set once they settle. +const pendingRequests = new Set>(); + +/** + * Wait for all pending metrics requests to complete. + * This should be called before the process exits to ensure all metrics are sent. + */ +export function waitForAllMetricsDispatches(): Promise { + return Promise.allSettled(pendingRequests).then(() => {}); +} + /** * A list of all the command args that can be included in the event. * @@ -58,7 +70,6 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { // The SPARROW_SOURCE_KEY will be provided at build time through esbuild's `define` option // No events will be sent if the env `SPARROW_SOURCE_KEY` is not provided and the value will be set to an empty string instead. const SPARROW_SOURCE_KEY = process.env.SPARROW_SOURCE_KEY ?? ""; - const requests: Array> = []; const wranglerVersion = getWranglerVersion(); const [wranglerMajorVersion, wranglerMinorVersion, wranglerPatchVersion] = wranglerVersion.split(".").map((v) => parseInt(v, 10)); @@ -114,8 +125,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { properties: Omit< Extract["properties"], keyof CommonEventProperties - >, - argv?: string[] + > ) { try { if (properties.command?.startsWith("wrangler login")) { @@ -136,7 +146,10 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { printMetricsBanner(); } - const sanitizedArgs = sanitizeArgKeys(properties.args ?? {}, argv); + const sanitizedArgs = sanitizeArgKeys( + properties.args ?? {}, + options.argv + ); const sanitizedArgsKeys = Object.keys(sanitizedArgs).sort(); const commonEventProperties: CommonEventProperties = { amplitude_session_id, @@ -179,10 +192,6 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { logger.debug("Error sending metrics event", err); } }, - - get requests() { - return requests; - }, }; function dispatch(event: { name: string; properties: Properties }) { @@ -239,9 +248,12 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { "Metrics dispatcher: Failed to send request:", (e as Error).message ); + }) + .finally(() => { + pendingRequests.delete(request); }); - requests.push(request); + pendingRequests.add(request); } function printMetricsBanner() {