From 8d77a2557a1c8132c203b2ad23dbf41710687596 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 12 Sep 2026 12:32:12 +0100 Subject: [PATCH 1/2] [wrangler] prevent silent success during asset uploads --- .changeset/quiet-assets-succeed.md | 7 ++ .changeset/soft-assets-listen.md | 8 ++ packages/workers-utils/src/cfetch/index.ts | 8 +- .../workers-utils/tests/cfetch-utils.test.ts | 113 +++++++++++++++++- packages/wrangler/bin/wrangler.js | 21 +++- .../src/__tests__/bin/wrangler.test.ts | 48 ++++++++ 6 files changed, 200 insertions(+), 5 deletions(-) create mode 100644 .changeset/quiet-assets-succeed.md create mode 100644 .changeset/soft-assets-listen.md create mode 100644 packages/wrangler/src/__tests__/bin/wrangler.test.ts diff --git a/.changeset/quiet-assets-succeed.md b/.changeset/quiet-assets-succeed.md new file mode 100644 index 00000000000..897223da95d --- /dev/null +++ b/.changeset/quiet-assets-succeed.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +Report a failed Wrangler command when the CLI child is terminated by a signal + +Wrangler's executable previously translated signal termination into exit code 0, which could cause callers to report a successful deployment after the operating system stopped Wrangler. Signal exits now use the conventional nonzero shell status. diff --git a/.changeset/soft-assets-listen.md b/.changeset/soft-assets-listen.md new file mode 100644 index 00000000000..ad33b101589 --- /dev/null +++ b/.changeset/soft-assets-listen.md @@ -0,0 +1,8 @@ +--- +"@cloudflare/workers-utils": patch +"wrangler": patch +--- + +Avoid materializing unused multipart request bodies + +Wrangler no longer creates a full text copy of multipart API request bodies when debug logs are disabled or sanitized. This substantially reduces peak memory use when uploading large batches of static assets while preserving request-body output when unsanitized debug logging is explicitly enabled. diff --git a/packages/workers-utils/src/cfetch/index.ts b/packages/workers-utils/src/cfetch/index.ts index 03f0d6a9fdc..8d0d4eb1ec6 100644 --- a/packages/workers-utils/src/cfetch/index.ts +++ b/packages/workers-utils/src/cfetch/index.ts @@ -3,6 +3,7 @@ import { URLSearchParams } from "node:url"; import { fetch, FormData, Headers, Response } from "undici"; import { getCloudflareApiBaseUrl, + getSanitizeLogs, getTraceHeader, } from "../environment-variables/misc-variables"; import { UserError } from "../errors"; @@ -94,7 +95,12 @@ export async function performApiFetchBase( logHeaders(headers, logger); logger.debugWithSanitization?.("INIT:", JSON.stringify({ ...init }, null, 2)); - if (init.body instanceof FormData) { + if ( + logger.debugWithSanitization !== undefined && + init.body instanceof FormData && + !getSanitizeLogs() && + (logger.loggerLevel === undefined || logger.loggerLevel === "debug") + ) { logger.debugWithSanitization?.( "BODY:", await new Response(init.body).text(), diff --git a/packages/workers-utils/tests/cfetch-utils.test.ts b/packages/workers-utils/tests/cfetch-utils.test.ts index 65bfd5f20d2..d474a4877d0 100644 --- a/packages/workers-utils/tests/cfetch-utils.test.ts +++ b/packages/workers-utils/tests/cfetch-utils.test.ts @@ -1,12 +1,123 @@ -import { describe, it } from "vitest"; +import { FormData, Response } from "undici"; +import { afterEach, beforeEach, describe, it, vi } from "vitest"; import { extractAccountTag, hasMorePages, parseRetryAfterMs, parseRetryAfterValue, + performApiFetchBase, throwFetchError, } from "../src/cfetch"; +import { COMPLIANCE_REGION_CONFIG_UNKNOWN } from "../src/environment-variables/misc-variables"; import { APIError } from "../src/parse"; +import type { Logger } from "../src/logger"; + +const fetchMock = vi.hoisted(() => vi.fn()); + +vi.mock("undici", async (importOriginal) => { + const original = await importOriginal(); + return { ...original, fetch: fetchMock }; +}); + +function createLogger(): Logger { + return { + loggerLevel: "debug", + debug: vi.fn(), + log: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debugWithSanitization: vi.fn(), + }; +} + +async function performFormDataFetch(logger: Logger): Promise { + const body = new FormData(); + body.append("field", "value"); + await performApiFetchBase( + COMPLIANCE_REGION_CONFIG_UNKNOWN, + "/test", + { method: "POST", body }, + "test-agent", + logger, + undefined, + undefined, + { apiToken: "test-token" } + ); +} + +describe("performApiFetchBase", () => { + beforeEach(() => { + fetchMock.mockResolvedValue(new Response()); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + it("does not materialize sanitized FormData request bodies", async ({ + expect, + }) => { + vi.stubEnv("WRANGLER_LOG_SANITIZE", "true"); + const logger = createLogger(); + const responseText = vi.spyOn(Response.prototype, "text"); + + await performFormDataFetch(logger); + + expect(responseText).not.toHaveBeenCalled(); + expect(logger.debugWithSanitization).not.toHaveBeenCalledWith( + "BODY:", + expect.anything(), + null, + 2 + ); + }); + + it("materializes FormData request bodies for unsanitized logs", async ({ + expect, + }) => { + vi.stubEnv("WRANGLER_LOG_SANITIZE", "false"); + const logger = createLogger(); + const responseText = vi.spyOn(Response.prototype, "text"); + + await performFormDataFetch(logger); + + expect(responseText).toHaveBeenCalledOnce(); + expect(logger.debugWithSanitization).toHaveBeenCalledWith( + "BODY:", + expect.stringContaining('name="field"'), + null, + 2 + ); + }); + + it("does not materialize FormData request bodies when debug logs are disabled", async ({ + expect, + }) => { + vi.stubEnv("WRANGLER_LOG_SANITIZE", "false"); + const logger = createLogger(); + logger.loggerLevel = "log"; + const responseText = vi.spyOn(Response.prototype, "text"); + + await performFormDataFetch(logger); + + expect(responseText).not.toHaveBeenCalled(); + }); + + it("does not materialize FormData request bodies without a sanitized logger", async ({ + expect, + }) => { + vi.stubEnv("WRANGLER_LOG_SANITIZE", "false"); + const logger = createLogger(); + logger.debugWithSanitization = undefined; + const responseText = vi.spyOn(Response.prototype, "text"); + + await performFormDataFetch(logger); + + expect(responseText).not.toHaveBeenCalled(); + }); +}); /** * hasMorePages is a function that returns a boolean based on the result_info diff --git a/packages/wrangler/bin/wrangler.js b/packages/wrangler/bin/wrangler.js index 774ecc2ce6e..e90f5094ca6 100755 --- a/packages/wrangler/bin/wrangler.js +++ b/packages/wrangler/bin/wrangler.js @@ -1,10 +1,27 @@ #!/usr/bin/env node const { spawn } = require("child_process"); +const { constants } = require("node:os"); const path = require("path"); const MIN_NODE_VERSION = "22.0.0"; let wranglerProcess; +/** + * Convert a child's exit result to the status expected by shell callers. + * + * @param {number | null | undefined} code + * @param {string | null} signal + * @returns {number} + */ +function getExitCode(code, signal) { + if (code !== undefined && code !== null) { + return code; + } + + const signalNumber = signal && constants.signals[signal]; + return signalNumber ? 128 + signalNumber : 1; +} + /** * Executes ../wrangler-dist/cli.js */ @@ -33,9 +50,7 @@ Consider using a Node.js version manager such as https://volta.sh/ or https://gi stdio: ["inherit", "inherit", "inherit", "ipc"], } ) - .on("exit", (code) => - process.exit(code === undefined || code === null ? 0 : code) - ) + .on("exit", (code, signal) => process.exit(getExitCode(code, signal))) .on("message", (message) => { if (process.send) { process.send(message); diff --git a/packages/wrangler/src/__tests__/bin/wrangler.test.ts b/packages/wrangler/src/__tests__/bin/wrangler.test.ts new file mode 100644 index 00000000000..4ac97491ff9 --- /dev/null +++ b/packages/wrangler/src/__tests__/bin/wrangler.test.ts @@ -0,0 +1,48 @@ +import { spawn } from "node:child_process"; +import { copyFileSync, mkdirSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; +import { describe, it } from "vitest"; + +const wranglerBin = path.resolve( + import.meta.dirname, + "../../../bin/wrangler.js" +); + +describe("Wrangler executable", () => { + runInTempDir(); + + it("exits nonzero when its CLI child is killed by a signal", async ({ + expect, + }) => { + mkdirSync("bin"); + copyFileSync(wranglerBin, "bin/wrangler.js"); + writeFileSync( + "mock-child-process.cjs", + `const { EventEmitter } = require("node:events"); +const childProcess = require("node:child_process"); + +childProcess.spawn = () => { + const child = new EventEmitter(); + process.nextTick(() => child.emit("exit", null, "SIGTERM")); + return child; +}; +` + ); + + const result = await new Promise<{ + code: number | null; + signal: NodeJS.Signals | null; + }>((resolve, reject) => { + const child = spawn(process.execPath, [ + "--require", + path.resolve("mock-child-process.cjs"), + path.resolve("bin/wrangler.js"), + ]); + child.on("error", reject); + child.on("exit", (code, signal) => resolve({ code, signal })); + }); + + expect(result).toEqual({ code: 143, signal: null }); + }); +}); From 27c1ef0c8097b21d80dda5fd5c0943c53b72da03 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 12 Sep 2026 12:38:32 +0100 Subject: [PATCH 2/2] [wrangler] forward termination signals to the CLI --- packages/wrangler/bin/wrangler.js | 11 ++- .../src/__tests__/bin/wrangler.test.ts | 68 ++++++++++++------- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/packages/wrangler/bin/wrangler.js b/packages/wrangler/bin/wrangler.js index e90f5094ca6..e2e65c0fd05 100755 --- a/packages/wrangler/bin/wrangler.js +++ b/packages/wrangler/bin/wrangler.js @@ -97,10 +97,9 @@ function semiver(a, b, bool) { if (module === require.main) { wranglerProcess = runWrangler(); - process.on("SIGINT", () => { - wranglerProcess && wranglerProcess.kill(); - }); - process.on("SIGTERM", () => { - wranglerProcess && wranglerProcess.kill(); - }); + for (const signal of ["SIGINT", "SIGTERM"]) { + process.on(signal, () => { + wranglerProcess && wranglerProcess.kill(signal); + }); + } } diff --git a/packages/wrangler/src/__tests__/bin/wrangler.test.ts b/packages/wrangler/src/__tests__/bin/wrangler.test.ts index 4ac97491ff9..bc3c644a36f 100644 --- a/packages/wrangler/src/__tests__/bin/wrangler.test.ts +++ b/packages/wrangler/src/__tests__/bin/wrangler.test.ts @@ -9,39 +9,59 @@ const wranglerBin = path.resolve( "../../../bin/wrangler.js" ); -describe("Wrangler executable", () => { - runInTempDir(); - - it("exits nonzero when its CLI child is killed by a signal", async ({ - expect, - }) => { - mkdirSync("bin"); - copyFileSync(wranglerBin, "bin/wrangler.js"); - writeFileSync( - "mock-child-process.cjs", - `const { EventEmitter } = require("node:events"); +async function runWranglerWithSignal( + signal: "SIGINT" | "SIGTERM" +): Promise<{ code: number | null; signal: NodeJS.Signals | null }> { + mkdirSync("bin"); + copyFileSync(wranglerBin, "bin/wrangler.js"); + writeFileSync( + "mock-child-process.cjs", + `const { EventEmitter } = require("node:events"); const childProcess = require("node:child_process"); childProcess.spawn = () => { const child = new EventEmitter(); - process.nextTick(() => child.emit("exit", null, "SIGTERM")); + child.kill = (signal) => { + process.nextTick(() => child.emit("exit", null, signal)); + }; return child; }; + +const originalProcessOn = process.on; +process.on = function (eventName, listener) { + const result = originalProcessOn.call(this, eventName, listener); + if (eventName === "${signal}") { + setImmediate(listener); + } + return result; +}; ` + ); + + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [ + "--require", + path.resolve("mock-child-process.cjs"), + path.resolve("bin/wrangler.js"), + ]); + child.on("error", reject); + child.on("exit", (code, childSignal) => + resolve({ code, signal: childSignal }) ); + }); +} + +describe("Wrangler executable", () => { + runInTempDir(); + + it("forwards SIGINT and exits with its shell status", async ({ expect }) => { + const result = await runWranglerWithSignal("SIGINT"); + + expect(result).toEqual({ code: 130, signal: null }); + }); - const result = await new Promise<{ - code: number | null; - signal: NodeJS.Signals | null; - }>((resolve, reject) => { - const child = spawn(process.execPath, [ - "--require", - path.resolve("mock-child-process.cjs"), - path.resolve("bin/wrangler.js"), - ]); - child.on("error", reject); - child.on("exit", (code, signal) => resolve({ code, signal })); - }); + it("forwards SIGTERM and exits with its shell status", async ({ expect }) => { + const result = await runWranglerWithSignal("SIGTERM"); expect(result).toEqual({ code: 143, signal: null }); });