diff --git a/src/commands/app/uninstall.test.ts b/src/commands/app/uninstall.test.ts new file mode 100644 index 000000000..146241f2d --- /dev/null +++ b/src/commands/app/uninstall.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "@jest/globals"; +import { AxiosError, AxiosHeaders } from "axios"; +import { mapUninstallError } from "./uninstall.js"; + +function axiosErrorWithStatus(status: number): AxiosError { + const headers = new AxiosHeaders(); + const config = { headers }; + return new AxiosError( + `Request failed with status code ${status}`, + "ERR_BAD_RESPONSE", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config as any, + {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { status, data: {}, statusText: "", headers, config } as any, + ); +} + +describe("mapUninstallError", () => { + it("maps an HTTP 412 to a linked-primary-database hint", () => { + const original = axiosErrorWithStatus(412); + const mapped = mapUninstallError(original); + + expect(mapped).toBeInstanceOf(Error); + expect((mapped as Error).message).toContain("cannot uninstall"); + expect((mapped as Error).message).toContain("primary database"); + // The original error is preserved as the cause and not swallowed. + expect((mapped as Error).cause).toBe(original); + }); + + it("passes through other Axios errors unchanged", () => { + const original = axiosErrorWithStatus(409); + const mapped = mapUninstallError(original); + + expect(mapped).toBe(original); + }); + + it("passes through non-Axios errors unchanged", () => { + const original = new Error("something else"); + const mapped = mapUninstallError(original); + + expect(mapped).toBe(original); + }); +}); diff --git a/src/commands/app/uninstall.ts b/src/commands/app/uninstall.ts index d25e0bdce..779a0e325 100644 --- a/src/commands/app/uninstall.ts +++ b/src/commands/app/uninstall.ts @@ -1,7 +1,28 @@ import { assertStatus } from "@mittwald/api-client-commons"; +import axios from "axios"; import { DeleteBaseCommand } from "../../lib/basecommands/DeleteBaseCommand.js"; import { appInstallationArgs } from "../../lib/resources/app/flags.js"; +/** + * Maps an error raised while uninstalling an app installation to a more + * actionable one. The API answers with an HTTP 412 (Precondition Failed) when + * the app still has a linked _primary_ database; in that case we surface a + * clear hint instead of a raw Axios error. All other errors are passed through + * unchanged so they are not accidentally swallowed. + */ +export function mapUninstallError(err: unknown): unknown { + if (axios.isAxiosError(err) && err.response?.status === 412) { + return new Error( + "cannot uninstall: app has a linked primary database — " + + "unlink or repurpose it first (a primary database must be " + + "repurposed to custom or cache before it can be unlinked)", + { cause: err }, + ); + } + + return err; +} + export default class Uninstall extends DeleteBaseCommand { static description = "Uninstall an app"; static resourceName = "app installation"; @@ -12,10 +33,15 @@ export default class Uninstall extends DeleteBaseCommand { protected async deleteResource(): Promise { const appInstallationId = await this.withAppInstallationId(Uninstall); - const response = await this.apiClient.app.uninstallAppinstallation({ - appInstallationId, - }); - assertStatus(response, 204); + try { + const response = await this.apiClient.app.uninstallAppinstallation({ + appInstallationId, + }); + + assertStatus(response, 204); + } catch (err) { + throw mapUninstallError(err); + } } } diff --git a/src/lib/basecommands/DeleteBaseCommand.test.tsx b/src/lib/basecommands/DeleteBaseCommand.test.tsx new file mode 100644 index 000000000..347bab0a4 --- /dev/null +++ b/src/lib/basecommands/DeleteBaseCommand.test.tsx @@ -0,0 +1,97 @@ +import { beforeEach, describe, expect, it, jest } from "@jest/globals"; + +/* + * Regression test for the bug where a failing deletion (e.g. an app uninstall + * rejected with HTTP 412) still printed "Process completed successfully" and + * exited with code 0. The command must instead surface the error via + * process.error() and exit with a non-zero code. + */ + +const fakeStep = { + complete: jest.fn(), + error: jest.fn(), +}; + +const fakeProcess = { + start: jest.fn(), + addStep: jest.fn(() => fakeStep), + addConfirmation: jest.fn(async () => true), + addInfo: jest.fn(), + complete: jest.fn(async (_summary: unknown) => {}), + error: jest.fn(async (_err: unknown) => {}), +}; + +const makeProcessRenderer = jest.fn(() => fakeProcess); + +jest.unstable_mockModule("../../rendering/process/process_flags.js", () => ({ + __esModule: true, + makeProcessRenderer, + processFlags: {}, +})); + +const { DeleteBaseCommand } = await import("./DeleteBaseCommand.js"); + +class TestDelete extends DeleteBaseCommand { + static resourceName = "test resource"; + public shouldThrow: unknown = undefined; + + protected async deleteResource(): Promise { + if (this.shouldThrow !== undefined) { + throw this.shouldThrow; + } + } +} + +function makeInstance(): TestDelete { + const instance = Object.create(TestDelete.prototype) as TestDelete; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (instance as any).flags = { force: true }; + return instance; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +async function runExec(instance: TestDelete): Promise { + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await (instance as any).exec(); + return undefined; + } catch (err) { + return err; + } +} + +describe("DeleteBaseCommand", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("reports success and does not exit non-zero when deletion succeeds", async () => { + const instance = makeInstance(); + + const thrown = await runExec(instance); + + expect(fakeStep.complete).toHaveBeenCalled(); + expect(fakeProcess.complete).toHaveBeenCalledTimes(1); + expect(fakeProcess.error).not.toHaveBeenCalled(); + // no ux.exit() -> no thrown ExitError + expect(thrown).toBeUndefined(); + }); + + it("surfaces the error and exits non-zero when deletion fails", async () => { + const instance = makeInstance(); + const failure = new Error("boom"); + instance.shouldThrow = failure; + + const thrown = await runExec(instance); + + // Must NOT falsely report success ... + expect(fakeProcess.complete).not.toHaveBeenCalled(); + // ... must surface the actual error ... + expect(fakeStep.error).toHaveBeenCalledWith(failure); + expect(fakeProcess.error).toHaveBeenCalledWith(failure); + // ... and must exit with a non-zero code (ux.exit(1) throws an ExitError). + expect(thrown).toBeDefined(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((thrown as any).oclif?.exit).toBe(1); + }); +}); diff --git a/src/lib/basecommands/DeleteBaseCommand.tsx b/src/lib/basecommands/DeleteBaseCommand.tsx index 129d2c3d6..8215b5ad8 100644 --- a/src/lib/basecommands/DeleteBaseCommand.tsx +++ b/src/lib/basecommands/DeleteBaseCommand.tsx @@ -7,7 +7,6 @@ import { processFlags, } from "../../rendering/process/process_flags.js"; import { Success } from "../../rendering/react/components/Success.js"; -import { Text } from "ink"; import React from "react"; export abstract class DeleteBaseCommand< @@ -56,7 +55,7 @@ export abstract class DeleteBaseCommand< ); } catch (err) { deletingStep.error(err); - process.complete(Failed to delete {resourceName}); + await process.error(err); ux.exit(1); } }