Skip to content
Merged
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
60 changes: 60 additions & 0 deletions src/commands/app/uninstall.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { describe, expect, it } from "@jest/globals";
import { mapUninstallError } from "./uninstall.js";

function apiErrorLike(options: {
message?: string;
status: number;
type?: string;
}) {
return {
response: {
data: {
message: options.message,
type: options.type,
},
status: options.status,
},
};
}

describe("mapUninstallError", () => {
it("maps matching HTTP 412 primary-database preconditions", () => {
const original = apiErrorLike({
status: 412,
type: "PreconditionFailed",
message: "App has a linked primary database",
});
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.
expect((mapped as Error).cause).toBe(original);
});

it("passes through unrelated HTTP 412 errors unchanged", () => {
const original = apiErrorLike({
status: 412,
type: "PreconditionFailed",
message: "Some other precondition failed",
});
const mapped = mapUninstallError(original);

expect(mapped).toBe(original);
});

it("passes through non-412 API errors unchanged", () => {
const original = apiErrorLike({ status: 409, type: "Conflict" });
const mapped = mapUninstallError(original);

expect(mapped).toBe(original);
});

it("passes through unrelated errors unchanged", () => {
const original = new Error("something else");
const mapped = mapUninstallError(original);

expect(mapped).toBe(original);
});
});
39 changes: 35 additions & 4 deletions src/commands/app/uninstall.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,33 @@
import { assertStatus } from "@mittwald/api-client-commons";
import { DeleteBaseCommand } from "../../lib/basecommands/DeleteBaseCommand.js";
import { matchesAPIError } from "../../lib/error/apiError.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. We therefore require both the
* precondition status and a matching error signature before rewriting the
* message, so unrelated 412 cases are not accidentally swallowed.
*/
export function mapUninstallError(err: unknown): unknown {
if (
matchesAPIError(err, {
status: 412,
keywords: ["primary", "database"],
})
) {
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<typeof Uninstall> {
static description = "Uninstall an app";
static resourceName = "app installation";
Expand All @@ -12,10 +38,15 @@ export default class Uninstall extends DeleteBaseCommand<typeof Uninstall> {

protected async deleteResource(): Promise<void> {
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);
}
}
}
97 changes: 97 additions & 0 deletions src/lib/basecommands/DeleteBaseCommand.test.tsx
Original file line number Diff line number Diff line change
@@ -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<typeof TestDelete> {
static resourceName = "test resource";
public shouldThrow: unknown = undefined;

protected async deleteResource(): Promise<void> {
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<any> {
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);
});
});
3 changes: 1 addition & 2 deletions src/lib/basecommands/DeleteBaseCommand.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down Expand Up @@ -56,7 +55,7 @@ export abstract class DeleteBaseCommand<
);
} catch (err) {
deletingStep.error(err);
process.complete(<Text>Failed to delete {resourceName}</Text>);
await process.error(err);
ux.exit(1);
}
}
Expand Down
74 changes: 74 additions & 0 deletions src/lib/error/apiError.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe, expect, it } from "@jest/globals";
import { getAPIErrorDetails, matchesAPIError } from "./apiError.js";

function apiErrorLike(options: {
message?: string;
status: number;
type?: string;
}) {
return {
response: {
data: {
message: options.message,
type: options.type,
},
status: options.status,
},
};
}

describe("getAPIErrorDetails", () => {
it("extracts status and body from API-like errors", () => {
const details = getAPIErrorDetails(
apiErrorLike({
status: 412,
type: "PreconditionFailed",
message: "primary database is linked",
}),
);

expect(details).toEqual({
status: 412,
body: {
type: "PreconditionFailed",
message: "primary database is linked",
},
});
});

it("returns null for non-API errors", () => {
expect(getAPIErrorDetails(new Error("no response"))).toBeNull();
});
});

describe("matchesAPIError", () => {
it("matches by status and keywords", () => {
const err = apiErrorLike({
status: 412,
type: "PreconditionFailed",
message: "App has linked primary database",
});

expect(
matchesAPIError(err, {
status: 412,
keywords: ["primary", "database"],
}),
).toBe(true);
});

it("does not match when keywords are missing", () => {
const err = apiErrorLike({
status: 412,
type: "PreconditionFailed",
message: "Some other precondition failed",
});

expect(
matchesAPIError(err, {
status: 412,
keywords: ["primary", "database"],
}),
).toBe(false);
});
});
66 changes: 66 additions & 0 deletions src/lib/error/apiError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
interface APIErrorBody {
message?: string;
type?: string;
}

interface APIErrorLike {
response?: {
data?: unknown;
status?: number;
};
}

export interface APIErrorDetails {
body: APIErrorBody | undefined;
status: number;
}

export function getAPIErrorDetails(err: unknown): APIErrorDetails | null {
if (typeof err !== "object" || err === null) {
return null;
}

const response = (err as APIErrorLike).response;
if (typeof response?.status !== "number") {
return null;
}

const body =
typeof response.data === "object" && response.data !== null
? (response.data as APIErrorBody)
: undefined;

return {
status: response.status,
body,
};
}

export function matchesAPIError(
err: unknown,
options: {
keywords?: string[];
status?: number;
},
): boolean {
const details = getAPIErrorDetails(err);
if (details === null) {
return false;
}

if (options.status !== undefined && details.status !== options.status) {
return false;
}

if (!options.keywords || options.keywords.length === 0) {
return true;
}

const signature = `${details.body?.type ?? ""} ${details.body?.message ?? ""}`
.toLowerCase()
.trim();

return options.keywords.every((keyword) =>
signature.includes(keyword.toLowerCase()),
);
}
Loading