wrangler containers: Delete should have better error handling (#9268)

Author: gabivlj

.changeset/curvy-hairs-attend.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+`wrangler containers delete` handles API errors correctly

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

@@ -2,12 +2,14 @@ import { http, HttpResponse } from "msw";
 import patchConsole from "patch-console";
 import { mockAccount, setWranglerConfig } from "../cloudchamber/utils";
 import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
-import { mockConsoleMethods } from "../helpers/mock-console";
+import { mockCLIOutput, mockConsoleMethods } from "../helpers/mock-console";
 import { useMockIsTTY } from "../helpers/mock-istty";
 import { msw } from "../helpers/msw";
 import { runWrangler } from "../helpers/run-wrangler";
 
 describe("containers delete", () => {
+	const stdCli = mockCLIOutput();
+
 	const std = mockConsoleMethods();
 	const { setIsTTY } = useMockIsTTY();
 
@@ -43,6 +45,98 @@ describe("containers delete", () => {
 		`);
 	});
 
+	async function testStatusCode(code: number) {
+		setWranglerConfig({});
+		msw.use(
+			http.delete(
+				"*/applications/:id",
+				async ({ request }) => {
+					expect(await request.text()).toEqual("");
+					return new HttpResponse(`{"error": "something happened"}`, {
+						status: code,
+					});
+				},
+				{ once: true }
+			)
+		);
+		await expect(runWrangler("containers delete 123")).rejects
+			.toMatchInlineSnapshot(`
+			[Error: There has been an error deleting the container.
+			something happened]
+		`);
+		expect(stdCli.stderr).toMatchInlineSnapshot(`""`);
+		expect(stdCli.stdout).toMatchInlineSnapshot(`
+			"├ Loading account
+			│
+			├ Loading account
+			│
+			╭ Delete your container
+			│
+			"
+		`);
+	}
+
+	it("should delete container with 400", () => testStatusCode(400));
+	it("should delete container with 404", () => testStatusCode(404));
+
+	it("should delete container with 500", async () => {
+		setWranglerConfig({});
+		msw.use(
+			http.delete(
+				"*/applications/:id",
+				async ({ request }) => {
+					expect(await request.text()).toEqual("");
+					return new HttpResponse(`{"error": "something happened"}`, {
+						status: 500,
+					});
+				},
+				{ once: true }
+			)
+		);
+		await expect(runWrangler("containers delete 123")).rejects
+			.toMatchInlineSnapshot(`
+			[Error: There has been an unknown error deleting the container.
+			"{/"error/": /"something happened/"}"]
+		`);
+		expect(stdCli.stderr).toMatchInlineSnapshot(`""`);
+		expect(stdCli.stdout).toMatchInlineSnapshot(`
+			"├ Loading account
+			│
+			├ Loading account
+			│
+			╭ Delete your container
+			│
+			"
+		`);
+	});
+
+	it("should delete container", async () => {
+		setWranglerConfig({});
+		msw.use(
+			http.delete(
+				"*/applications/:id",
+				async ({ request }) => {
+					expect(await request.text()).toEqual("");
+					return new HttpResponse("{}");
+				},
+				{ once: true }
+			)
+		);
+		await runWrangler("containers delete 123");
+		expect(stdCli.stderr).toMatchInlineSnapshot(`""`);
+		expect(stdCli.stdout).toMatchInlineSnapshot(`
+			"├ Loading account
+			│
+			├ Loading account
+			│
+			╭ Delete your container
+			│
+			╰ Your container has been deleted
+
+			"
+		`);
+	});
+
 	it("should delete container (json)", async () => {
 		setIsTTY(false);
 		setWranglerConfig({});

packages/wrangler/src/containers/containers.ts

@@ -8,7 +8,7 @@ import {
 import { processArgument } from "@cloudflare/cli/args";
 import { dim, gray } from "@cloudflare/cli/colors";
 import { inputPrompt, spinner } from "@cloudflare/cli/interactive";
-import { ApplicationsService } from "../cloudchamber/client";
+import { ApiError, ApplicationsService } from "../cloudchamber/client";
 import { loadAccountSpinner } from "../cloudchamber/common";
 import { wrap } from "../cloudchamber/helpers/wrap";
 import { UserError } from "../errors";
@@ -38,33 +38,51 @@ export async function deleteCommand(
 			"You must provide an ID. Use 'wrangler containers list` to view your containers."
 		);
 	}
-	if (deleteArgs.json || !isInteractive()) {
+
+	if (deleteArgs.json) {
 		const container = await ApplicationsService.deleteApplication(
 			deleteArgs.ID
 		);
 		console.log(JSON.stringify(container, null, 4));
 		return;
 	}
+
 	startSection("Delete your container");
-	const yes = await inputPrompt({
-		question:
-			"Are you sure that you want to delete these containers? The associated DO container will lose access to the containers.",
-		type: "confirm",
-		label: "",
-	});
-	if (!yes) {
-		cancel("The operation has been cancelled");
-		return;
+	if (isInteractive()) {
+		const yes = await inputPrompt({
+			question:
+				"Are you sure that you want to delete these containers? The associated DO container will lose access to the containers.",
+			type: "confirm",
+			label: "",
+		});
+		if (!yes) {
+			cancel("The operation has been cancelled");
+			return;
+		}
 	}
 
 	const [, err] = await wrap(
 		ApplicationsService.deleteApplication(deleteArgs.ID)
 	);
 	if (err) {
-		throw new UserError(
-			`There has been an internal error deleting your containers.\n ${err.message}`
+		if (err instanceof ApiError) {
+			if (err.status === 400 || err.status === 404) {
+				const body = JSON.parse(err.body);
+				throw new UserError(
+					`There has been an error deleting the container.\n${body.error}`
+				);
+			}
+
+			throw new Error(
+				`There has been an unknown error deleting the container.\n${JSON.stringify(err.body)}`
+			);
+		}
+
+		throw new Error(
+			`There has been an internal error deleting your containers.\n${err.message}`
 		);
 	}
+
 	endSection("Your container has been deleted");
 }