refactor(errors): route fatal app errors
This commit is contained in:
1 parent
ead9475b71
commit
38e2807f6c
6 files changed
+84
-27
No files matched your search
@@ -78,26 +78,6 @@ export function createBrowserActionService({
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Returns the approval record for `approvalId`, or null if the record is
|
|
||||||
* missing. Isolates the "approval row disappeared" case so the caller does
|
|
||||||
* not need to nest a try/catch around the state machine.
|
|
||||||
*/
|
|
||||||
function getApprovalOrNull(approvalId: string): ReturnType<typeof db.core.approvals.getApprovalRecord> | null {
|
|
||||||
try {
|
|
||||||
return db.core.approvals.getApprovalRecord(approvalId);
|
|
||||||
} catch (err) {
|
|
||||||
const classified = classifyUnknownError(err, {
|
|
||||||
kind: "recoverable",
|
|
||||||
code: "browser_action.approval.lookup_failed",
|
|
||||||
message: `getApprovalRecord failed for ${approvalId}`,
|
|
||||||
context: { approvalId },
|
|
||||||
});
|
|
||||||
log.warn(`[${classified.code}]`, classified.message, classified.context);
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
async function dispatchBrowserAction(actionId: string, companyId: string) {
|
async function dispatchBrowserAction(actionId: string, companyId: string) {
|
||||||
const action = db.core.social.getBrowserAction(actionId, companyId);
|
const action = db.core.social.getBrowserAction(actionId, companyId);
|
||||||
if (!action) {
|
if (!action) {
|
||||||
@@ -107,7 +87,7 @@ export function createBrowserActionService({
|
|||||||
throw new Error(`Browser action ${actionId} is ${action.status} and cannot be dispatched.`);
|
throw new Error(`Browser action ${actionId} is ${action.status} and cannot be dispatched.`);
|
||||||
}
|
}
|
||||||
if (action.approvalId) {
|
if (action.approvalId) {
|
||||||
const approval = getApprovalOrNull(action.approvalId);
|
const approval = db.core.approvals.getApprovalRecord(action.approvalId);
|
||||||
if (!approval || approval.companyId !== companyId) {
|
if (!approval || approval.companyId !== companyId) {
|
||||||
throw new Error(`Browser action approval ${action.approvalId} does not belong to company ${companyId}.`);
|
throw new Error(`Browser action approval ${action.approvalId} does not belong to company ${companyId}.`);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,19 +1,33 @@
|
|||||||
import { getOAuthProvider } from "@mariozechner/pi-ai/oauth";
|
import { getOAuthProvider } from "@mariozechner/pi-ai/oauth";
|
||||||
import { getEnvApiKey } from "@mariozechner/pi-ai";
|
import { getEnvApiKey } from "@mariozechner/pi-ai";
|
||||||
import { getConnectorDefinition, isPiKnownProvider } from "@main/connectors";
|
import { getConnectorDefinition, isPiKnownProvider } from "@main/connectors";
|
||||||
|
import { UpstreamError } from "../errors/index.js";
|
||||||
import { CredentialStore } from "./credential-store";
|
import { CredentialStore } from "./credential-store";
|
||||||
import type { OAuthCredentialPayload, StoredCredentialPayload } from "./types";
|
import type { OAuthCredentialPayload, StoredCredentialPayload } from "./types";
|
||||||
|
|
||||||
const REFRESH_WINDOW_MS = 60_000;
|
const REFRESH_WINDOW_MS = 60_000;
|
||||||
|
|
||||||
export class CredentialError extends Error {
|
export type CredentialErrorCode =
|
||||||
|
| "CREDENTIAL_MISSING"
|
||||||
|
| "CREDENTIAL_EXPIRED"
|
||||||
|
| "CREDENTIAL_INVALID"
|
||||||
|
| "CREDENTIAL_REFRESH_FAILED";
|
||||||
|
|
||||||
|
export class CredentialError extends UpstreamError {
|
||||||
|
readonly connectorId: string;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
public readonly code: "CREDENTIAL_MISSING" | "CREDENTIAL_EXPIRED" | "CREDENTIAL_INVALID" | "CREDENTIAL_REFRESH_FAILED",
|
code: CredentialErrorCode,
|
||||||
public readonly connectorId: string,
|
connectorId: string,
|
||||||
message?: string,
|
message?: string,
|
||||||
) {
|
) {
|
||||||
super(message ?? code);
|
super({
|
||||||
|
code,
|
||||||
|
message: message ?? code,
|
||||||
|
context: { connectorId },
|
||||||
|
});
|
||||||
this.name = "CredentialError";
|
this.name = "CredentialError";
|
||||||
|
this.connectorId = connectorId;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -13,6 +13,22 @@ export interface Logger {
|
|||||||
warn?: (...args: unknown[]) => void;
|
warn?: (...args: unknown[]) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type FatalErrorRoute = (error: AppError) => never;
|
||||||
|
|
||||||
|
const defaultFatalErrorRoute: FatalErrorRoute = (error) => {
|
||||||
|
if (process.env.VITEST === "true" || process.env.NODE_ENV === "test") {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
process.stderr.write(`[fatal:${error.code}] ${error.message}\n`);
|
||||||
|
process.abort();
|
||||||
|
};
|
||||||
|
|
||||||
|
let fatalErrorRoute: FatalErrorRoute = defaultFatalErrorRoute;
|
||||||
|
|
||||||
|
export function setFatalErrorRouteForTesting(route: FatalErrorRoute | null): void {
|
||||||
|
fatalErrorRoute = route ?? defaultFatalErrorRoute;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Classify an unknown thrown value and emit a single structured log line.
|
* Classify an unknown thrown value and emit a single structured log line.
|
||||||
* Returns the `AppError` so the caller can still branch on `err.kind`
|
* Returns the `AppError` so the caller can still branch on `err.kind`
|
||||||
@@ -46,5 +62,8 @@ export function logAppError(
|
|||||||
} else if (typeof logger.warn === "function") {
|
} else if (typeof logger.warn === "function") {
|
||||||
logger.warn(...parts);
|
logger.warn(...parts);
|
||||||
}
|
}
|
||||||
|
if (err.kind === "fatal") {
|
||||||
|
fatalErrorRoute(err);
|
||||||
|
}
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
@@ -18,4 +18,4 @@ export {
|
|||||||
type ErrorKind,
|
type ErrorKind,
|
||||||
} from "./taxonomy.js";
|
} from "./taxonomy.js";
|
||||||
|
|
||||||
export { logAppError, type Logger } from "./handlers.js";
|
export { logAppError, setFatalErrorRouteForTesting, type Logger } from "./handlers.js";
|
||||||
@@ -0,0 +1,30 @@
|
|||||||
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { logAppError, setFatalErrorRouteForTesting } from "../../src/main/errors";
|
||||||
|
import type { AppError } from "../../src/main/errors";
|
||||||
|
|
||||||
|
describe("AppError fatal route", () => {
|
||||||
|
afterEach(() => {
|
||||||
|
setFatalErrorRouteForTesting(null);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("routes fatal errors through the process-level handler", () => {
|
||||||
|
const logger = { error: vi.fn(), warn: vi.fn() };
|
||||||
|
const routed: AppError[] = [];
|
||||||
|
const sentinel = new Error("fatal route called");
|
||||||
|
setFatalErrorRouteForTesting((error) => {
|
||||||
|
routed.push(error);
|
||||||
|
throw sentinel;
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(() => logAppError(logger, new Error("sqlite corrupt"), {
|
||||||
|
kind: "fatal",
|
||||||
|
code: "db.sqlite.corrupt",
|
||||||
|
message: "SQLite profile is corrupt",
|
||||||
|
context: { profile: "test" },
|
||||||
|
})).toThrow(sentinel);
|
||||||
|
|
||||||
|
expect(routed[0]?.kind).toBe("fatal");
|
||||||
|
expect(routed[0]?.code).toBe("db.sqlite.corrupt");
|
||||||
|
expect(logger.error).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -5,6 +5,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
|
|||||||
import { AppDatabase } from "../../src/main/database";
|
import { AppDatabase } from "../../src/main/database";
|
||||||
import { CredentialStore } from "../../src/main/credentials/credential-store";
|
import { CredentialStore } from "../../src/main/credentials/credential-store";
|
||||||
import { CredentialResolver, CredentialError } from "../../src/main/credentials/credential-resolver";
|
import { CredentialResolver, CredentialError } from "../../src/main/credentials/credential-resolver";
|
||||||
|
import { AppError } from "../../src/main/errors";
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
process.env.AGENTCOMPANY_ALLOW_INSECURE_VAULT = "true";
|
process.env.AGENTCOMPANY_ALLOW_INSECURE_VAULT = "true";
|
||||||
@@ -56,7 +57,20 @@ describe("CredentialResolver", () => {
|
|||||||
it("throws CREDENTIAL_MISSING when the env-backed key is unset", async () => {
|
it("throws CREDENTIAL_MISSING when the env-backed key is unset", async () => {
|
||||||
delete process.env.XAI_API_KEY;
|
delete process.env.XAI_API_KEY;
|
||||||
store.save("xai", { kind: "api_key", value: { source: "env", value: "XAI_API_KEY" } });
|
store.save("xai", { kind: "api_key", value: { source: "env", value: "XAI_API_KEY" } });
|
||||||
await expect(resolver.resolve("xai")).rejects.toBeInstanceOf(CredentialError);
|
let thrown: unknown;
|
||||||
|
try {
|
||||||
|
await resolver.resolve("xai");
|
||||||
|
} catch (error) {
|
||||||
|
thrown = error;
|
||||||
|
}
|
||||||
|
expect(thrown).toBeInstanceOf(CredentialError);
|
||||||
|
expect(thrown).toBeInstanceOf(AppError);
|
||||||
|
expect(thrown).toMatchObject({
|
||||||
|
kind: "upstream",
|
||||||
|
code: "CREDENTIAL_MISSING",
|
||||||
|
connectorId: "xai",
|
||||||
|
context: { connectorId: "xai" },
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("assembles AWS extraEnv for IAM auth mode", async () => {
|
it("assembles AWS extraEnv for IAM auth mode", async () => {
|
||||||
|
|||||||
Reference in new issue
Block a user