Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/quiet-assets-succeed.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 8 additions & 0 deletions .changeset/soft-assets-listen.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 7 additions & 1 deletion packages/workers-utils/src/cfetch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(),
Expand Down
113 changes: 112 additions & 1 deletion packages/workers-utils/tests/cfetch-utils.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import("undici")>();
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<void> {
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
Expand Down
32 changes: 23 additions & 9 deletions packages/wrangler/bin/wrangler.js
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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)))
Comment thread
petebacondarwin marked this conversation as resolved.
.on("message", (message) => {
if (process.send) {
process.send(message);
Expand Down Expand Up @@ -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);
});
}
}
68 changes: 68 additions & 0 deletions packages/wrangler/src/__tests__/bin/wrangler.test.ts
Original file line number Diff line number Diff line change
@@ -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 });
});
});
Loading