validate id in wrangler containers delete and also include container API calls in WRANGLER_LOG=debug (#10232)

* validate id for containers delete

* changeset

* add debug logging to containers api calls

* Update packages/wrangler/src/containers/containers.ts

Co-authored-by: Nikita Sharma <[email protected]>

* update test

* lint

* feedback

---------

Co-authored-by: Nikita Sharma <[email protected]>

Author: emily-shen

.changeset/blue-seas-visit.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: validate `wrangler containers delete ID` to ensure a valid ID has been provided. Previously if you provided the container name (or any non-ID shaped string) you would get an auth error instead of a 404.

.changeset/pink-heads-help.md

@@ -0,0 +1,6 @@
+---
+"@cloudflare/containers-shared": patch
+"wrangler": patch
+---
+
+include containers API calls in output of WRANGLER_LOG=debug

packages/containers-shared/src/client/core/OpenAPI.ts

@@ -1,5 +1,6 @@
 /* istanbul ignore file */
 /* tslint:disable */
+import type { Logger } from "../../types";
 import type { ApiRequestOptions } from "./ApiRequestOptions";
 
 type Resolver<T> = (options: ApiRequestOptions) => Promise<T>;
@@ -15,6 +16,7 @@ export type OpenAPIConfig = {
 	PASSWORD?: string | Resolver<string>;
 	HEADERS?: Headers | Resolver<Headers>;
 	ENCODE_PATH?: (path: string) => string;
+	LOGGER?: Logger | undefined;
 };
 
 export const OpenAPI: OpenAPIConfig = {
@@ -27,4 +29,5 @@ export const OpenAPI: OpenAPIConfig = {
 	PASSWORD: undefined,
 	HEADERS: undefined,
 	ENCODE_PATH: undefined,
+	LOGGER: undefined,
 };

packages/containers-shared/src/client/core/request.ts

@@ -352,6 +352,7 @@ export const request = <T>(
 			const formData = getFormData(options);
 			const body = getRequestBody(options);
 			const headers = await getHeaders(config, options);
+			debugLogRequest(config, url, headers, formData ?? body ?? {});
 
 			if (!onCancel.isCancelled) {
 				const response = await sendRequest(
@@ -387,7 +388,7 @@ export const request = <T>(
 						body: responseHeader ?? responseBody,
 					};
 				}
-
+				debugLogResponse(config, result);
 				catchErrorCodes(options, result);
 				resolve(result.body);
 			}
@@ -396,3 +397,38 @@ export const request = <T>(
 		}
 	});
 };
+
+const debugLogRequest = async (
+	config: OpenAPIConfig,
+	url: string,
+	headers: Headers,
+	body: FormData | unknown
+) => {
+	config.LOGGER?.debug(`-- START CF API REQUEST: ${url}`);
+	const logHeaders = new Headers(headers);
+	logHeaders.delete("Authorization");
+	config.LOGGER?.debugWithSanitization(
+		"HEADERS:",
+		JSON.stringify(logHeaders, null, 2)
+	);
+	config.LOGGER?.debugWithSanitization(
+		"BODY:",
+		JSON.stringify(
+			body instanceof FormData ? await new Response(body).text() : body,
+			null,
+			2
+		)
+	);
+	config.LOGGER?.debug("-- END CF API REQUEST");
+};
+
+const debugLogResponse = (config: OpenAPIConfig, response: ApiResult) => {
+	config.LOGGER?.debug(
+		"-- START CF API RESPONSE:",
+		response.statusText,
+		response.status
+	);
+
+	config.LOGGER?.debugWithSanitization("RESPONSE:", response.body);
+	config.LOGGER?.debug("-- END CF API RESPONSE");
+};

packages/containers-shared/src/types.ts

@@ -1,11 +1,12 @@
 import type { InstanceType, SchedulingPolicy } from "./client";
 
 export interface Logger {
-	debug: (message: string) => void;
-	log: (message: string) => void;
-	info: (message: string) => void;
-	warn: (message: string) => void;
-	error: (error: Error) => void;
+	debug: (...args: unknown[]) => void;
+	debugWithSanitization: (label: string, ...args: unknown[]) => void;
+	log: (...args: unknown[]) => void;
+	info: (...args: unknown[]) => void;
+	warn: (...args: unknown[]) => void;
+	error: (...args: unknown[]) => void;
 }
 
 export type BuildArgs = {

packages/wrangler/src/__tests__/containers/delete.test.ts

@@ -43,6 +43,15 @@ describe("containers delete", () => {
 		`);
 	});
 
+	it("should reject invalid container ID format", async () => {
+		setWranglerConfig({});
+		await expect(
+			runWrangler("containers delete invalid-id")
+		).rejects.toMatchInlineSnapshot(
+			`[Error: Expected a container ID but got invalid-id. Use \`wrangler containers list\` to view your containers and corresponding IDs.]`
+		);
+	});
+
 	async function testStatusCode(code: number) {
 		setWranglerConfig({});
 		msw.use(

packages/wrangler/src/cloudchamber/common.ts

@@ -194,6 +194,7 @@ export async function fillOpenAPIConfiguration(
 		...(OpenAPI.HEADERS ?? {}),
 		...Object.fromEntries(headers.entries()),
 	};
+	OpenAPI.LOGGER = logger;
 }
 
 type NonObject = undefined | null | boolean | string | number;

packages/wrangler/src/containers/containers.ts

@@ -27,16 +27,20 @@ export function deleteYargs(args: CommonYargsArgv) {
 	return args.positional("ID", {
 		describe: "id of the containers to delete",
 		type: "string",
+		demandOption: true,
 	});
 }
 
 export async function deleteCommand(
 	deleteArgs: StrictYargsOptionsToInterface<typeof deleteYargs>,
 	_config: Config
 ) {
-	if (!deleteArgs.ID) {
-		throw new Error(
-			"You must provide an ID. Use 'wrangler containers list` to view your containers."
+	// API gateway has path restrictions so if someone provides a string that isn't ID shaped, we get a weird error instead of a 404
+	const uuidRegex =
+		/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+	if (!uuidRegex.test(deleteArgs.ID)) {
+		throw new UserError(
+			`Expected a container ID but got ${deleteArgs.ID}. Use \`wrangler containers list\` to view your containers and corresponding IDs.`
 		);
 	}