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..e2e65c0fd05 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); @@ -82,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 new file mode 100644 index 00000000000..bc3c644a36f --- /dev/null +++ b/packages/wrangler/src/__tests__/bin/wrangler.test.ts @@ -0,0 +1,68 @@ +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" +); + +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(); + 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 }); + }); + + it("forwards SIGTERM and exits with its shell status", async ({ expect }) => { + const result = await runWranglerWithSignal("SIGTERM"); + + expect(result).toEqual({ code: 143, signal: null }); + }); +});