feat(hk): connect delivery failures to precise diagnostic records
This commit is contained in:
1 parent
275a574203
commit
60e49c1ec9
17 files changed
+253
-20
No files matched your search
@@ -14,3 +14,16 @@ The delivery screen separates saved content from its subsequent background updat
|
||||
Content titles and links require the corresponding NEWS_VIEW or CATALOG_VIEW grant in addition to DEVOPS_VIEW. Raw result JSON and internal error text are never forwarded to the card. Each record shows its recorded time, staff/system actor, attempts, and scheduled retry time when applicable. Failures point to service diagnostics; exhausted attempts expose the existing permission-protected retry without repeating the content mutation. Delivery success does not imply Git publication or client refresh acknowledgement.
|
||||
|
||||
Browser fixtures verify the real cards on desktop/mobile, including long titles, denied metadata, exact links, unknown states and collapsed technical references. The fixture retry slot does not execute a backend action; backend authorization and state changes have separate tests.
|
||||
|
||||
|
||||
Delivery failures retain the original exception in the redacted CMS diagnostic store,
|
||||
alongside a delivery UUID and the persisted operation UUID. Request correlation is
|
||||
kept separately. Only the diagnostic UUID is stored in the outbox error field.
|
||||
The delivery card opens the matching error record; that record links back to the
|
||||
exact delivery, including records older than the latest 100 entries. Refresh keeps
|
||||
this selection. A historical diagnostic lookup shows that occurrence's context and
|
||||
stack while retaining the current group's assignment and resolution status.
|
||||
|
||||
These links require existing DevOps access. Expired diagnostic records may no longer
|
||||
be available after the error store's retention window; the delivery state remains
|
||||
in SQL. Retry continues to require edit permission and an exhausted delivery.
|
||||
@@ -2,6 +2,7 @@ import { useFormatter, useTranslations } from "next-intl";
|
||||
import type { ReactNode } from "react";
|
||||
import Link from "@/components/link";
|
||||
import { deliveryTopic } from "./delivery-context";
|
||||
import { deliveryDiagnosticLink } from "./delivery-diagnostics";
|
||||
|
||||
export interface DeliveryCardItem {
|
||||
id: string;
|
||||
@@ -13,6 +14,7 @@ export interface DeliveryCardItem {
|
||||
createdAt: string;
|
||||
availableAt: string;
|
||||
hasError: boolean;
|
||||
errorId?: string | null;
|
||||
context: {
|
||||
title: string | null;
|
||||
href: string | null;
|
||||
@@ -87,7 +89,10 @@ export function DeliveryCard({
|
||||
</Link>
|
||||
)}
|
||||
{item.hasError && state !== "done" && (
|
||||
<Link href="/admin/devops/installation" className="btn btn-outline">
|
||||
<Link
|
||||
href={deliveryDiagnosticLink(item.errorId)}
|
||||
className="btn btn-outline"
|
||||
>
|
||||
{t("diagnostics")}
|
||||
</Link>
|
||||
)}
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
import { expect, it } from "vitest";
|
||||
import {
|
||||
deliveryDiagnosticId,
|
||||
deliveryDiagnosticLink,
|
||||
} from "./delivery-diagnostics";
|
||||
|
||||
it("uses only an exact server-issued reference for a diagnostic link", () => {
|
||||
const id = "4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2";
|
||||
expect(deliveryDiagnosticId(`Diagnostic reference: ${id}`)).toBe(id);
|
||||
expect(deliveryDiagnosticLink(id)).toBe(`/admin/devops/cms-errors?q=${id}`);
|
||||
for (const input of [
|
||||
null,
|
||||
"",
|
||||
"private SQL",
|
||||
"Diagnostic reference: ../settings",
|
||||
`Diagnostic reference: ${id}?secret=value`,
|
||||
`Diagnostic reference: ${id}\n`,
|
||||
]) {
|
||||
expect(deliveryDiagnosticId(input)).toBeNull();
|
||||
expect(deliveryDiagnosticLink(input)).toBe("/admin/devops/installation");
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
/** Only server-issued UUID references can become diagnostic/navigation targets. */
|
||||
export function isDeliveryReference(value: unknown): value is string {
|
||||
return (
|
||||
typeof value === "string" &&
|
||||
/^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i.test(
|
||||
value,
|
||||
)
|
||||
);
|
||||
}
|
||||
export function deliveryDiagnosticId(value: string | null): string | null {
|
||||
const prefix = "Diagnostic reference: ";
|
||||
const id = value?.startsWith(prefix) ? value.slice(prefix.length) : null;
|
||||
return isDeliveryReference(id) ? id : null;
|
||||
}
|
||||
export function deliveryDiagnosticLink(errorId: unknown): string {
|
||||
return isDeliveryReference(errorId)
|
||||
? `/admin/devops/cms-errors?q=${encodeURIComponent(errorId)}`
|
||||
: "/admin/devops/installation";
|
||||
}
|
||||
@@ -0,0 +1,53 @@
|
||||
import { MySqlDialect } from "drizzle-orm/mysql-core";
|
||||
import { beforeEach, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({ execute: vi.fn(), error: vi.fn() }));
|
||||
vi.mock("@/lib/db", () => ({ db: { execute: mocks.execute } }));
|
||||
vi.mock("@/lib/logger", () => ({ logger: { error: mocks.error } }));
|
||||
|
||||
import { effectRepository, listEffects } from "./server";
|
||||
|
||||
const id = "840f4023-04cd-4d7b-92c5-047aa620029c";
|
||||
const operationId = "1dd40f61-b710-48c8-8534-c3a3f726780a";
|
||||
const errorId = "4f3df172-f1ca-4c26-a23e-cb4e6c13e9f2";
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
mocks.error.mockReturnValue(errorId);
|
||||
mocks.execute.mockResolvedValue([[]]);
|
||||
});
|
||||
it("records the original failure with durable identity and stores only a safe diagnostic reference", async () => {
|
||||
const error = new Error("private transport details");
|
||||
await effectRepository.fail(
|
||||
{
|
||||
id,
|
||||
operationId,
|
||||
token: "private-lease",
|
||||
topic: "news.refresh",
|
||||
attempts: 2,
|
||||
},
|
||||
error,
|
||||
);
|
||||
expect(mocks.error).toHaveBeenCalledWith(
|
||||
"Operation delivery failed",
|
||||
expect.objectContaining({
|
||||
module: "operations",
|
||||
deliveryId: id,
|
||||
durableOperationId: operationId,
|
||||
error,
|
||||
}),
|
||||
);
|
||||
const query = new MySqlDialect().sqlToQuery(mocks.execute.mock.calls[0][0]);
|
||||
expect(query.params).toContain(`Diagnostic reference: ${errorId}`);
|
||||
expect(query.params).not.toContain(error.message);
|
||||
expect(query.params).toContain("private-lease");
|
||||
});
|
||||
it("looks up an older delivery by exact ID independently of the latest-page limit", async () => {
|
||||
await listEffects(id);
|
||||
const query = new MySqlDialect().sqlToQuery(mocks.execute.mock.calls[0][0]);
|
||||
expect(query.sql).toContain("WHERE e.id=?");
|
||||
expect(query.params).toContain(id);
|
||||
});
|
||||
it("does not query the database for a malformed delivery reference", async () => {
|
||||
await expect(listEffects("../settings")).resolves.toEqual([]);
|
||||
expect(mocks.execute).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -31,8 +31,9 @@ it("persists failures and continues to other pending work", async () => {
|
||||
complete: vi.fn(),
|
||||
fail: vi.fn(),
|
||||
};
|
||||
await dispatchEffects(repo, vi.fn().mockRejectedValue(Error("private")));
|
||||
expect(repo.fail).toHaveBeenCalledWith(effect);
|
||||
const failure = Error("private");
|
||||
await dispatchEffects(repo, vi.fn().mockRejectedValue(failure));
|
||||
expect(repo.fail).toHaveBeenCalledWith(effect, failure);
|
||||
expect(repo.complete).not.toHaveBeenCalled();
|
||||
});
|
||||
it("bounds work per tick instead of draining indefinitely", async () => {
|
||||
|
||||
@@ -2,7 +2,7 @@ import type { EffectClaim } from "./model";
|
||||
export interface EffectRepository {
|
||||
claim(): Promise<EffectClaim | null>;
|
||||
complete(claim: EffectClaim): Promise<unknown>;
|
||||
fail(claim: EffectClaim): Promise<unknown>;
|
||||
fail(claim: EffectClaim, error: unknown): Promise<unknown>;
|
||||
}
|
||||
/** At-least-once delivery: handlers must tolerate replay after an expired claim. */
|
||||
export async function dispatchEffects(
|
||||
@@ -14,8 +14,8 @@ export async function dispatchEffects(
|
||||
if (!claim) return;
|
||||
try {
|
||||
await deliver(claim);
|
||||
} catch {
|
||||
await repo.fail(claim);
|
||||
} catch (error) {
|
||||
await repo.fail(claim, error);
|
||||
continue;
|
||||
}
|
||||
await repo.complete(claim);
|
||||
|
||||
@@ -59,6 +59,7 @@ export type EffectTopic =
|
||||
| "news.refresh";
|
||||
export interface EffectClaim {
|
||||
id: string;
|
||||
operationId?: string;
|
||||
token: string;
|
||||
topic: EffectTopic;
|
||||
attempts: number;
|
||||
|
||||
@@ -2,6 +2,8 @@ import "server-only";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { sql } from "drizzle-orm";
|
||||
import { db } from "@/lib/db";
|
||||
import { logger } from "@/lib/logger";
|
||||
import { isDeliveryReference } from "./delivery-diagnostics";
|
||||
import {
|
||||
type EffectClaim,
|
||||
type EffectTopic,
|
||||
@@ -60,11 +62,12 @@ export const effectRepository = {
|
||||
async claim(): Promise<EffectClaim | null> {
|
||||
return db.transaction(async (tx) => {
|
||||
const [rows] = await tx.execute(
|
||||
sql`SELECT id,topic,attempts FROM cms_outbox WHERE (status='pending' AND available_at<=UTC_TIMESTAMP(3)) OR (status='running' AND lease_until<UTC_TIMESTAMP(3)) ORDER BY available_at,id LIMIT 1 FOR UPDATE`,
|
||||
sql`SELECT id,operation_id AS operationId,topic,attempts FROM cms_outbox WHERE (status='pending' AND available_at<=UTC_TIMESTAMP(3)) OR (status='running' AND lease_until<UTC_TIMESTAMP(3)) ORDER BY available_at,id LIMIT 1 FOR UPDATE`,
|
||||
);
|
||||
const row = (
|
||||
rows as unknown as Array<{
|
||||
id: string;
|
||||
operationId: string;
|
||||
topic: EffectTopic;
|
||||
attempts: number;
|
||||
}>
|
||||
@@ -82,15 +85,28 @@ export const effectRepository = {
|
||||
sql`UPDATE cms_outbox SET status='done',lease_token=NULL,lease_until=NULL,last_error=NULL WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`,
|
||||
);
|
||||
},
|
||||
async fail(claim: EffectClaim) {
|
||||
async fail(claim: EffectClaim, error: unknown) {
|
||||
const errorId = logger.error("Operation delivery failed", {
|
||||
module: "operations",
|
||||
deliveryId: claim.id,
|
||||
durableOperationId: claim.operationId,
|
||||
topic: claim.topic,
|
||||
attempt: claim.attempts,
|
||||
error,
|
||||
});
|
||||
const reference = isDeliveryReference(errorId)
|
||||
? `Diagnostic reference: ${errorId}`
|
||||
: "Delivery failed; service diagnostics unavailable.";
|
||||
await db.execute(
|
||||
sql`UPDATE cms_outbox SET status=${claim.attempts >= 8 ? "failed" : "pending"},available_at=DATE_ADD(UTC_TIMESTAMP(3),INTERVAL ${retryDelay(claim.attempts)} SECOND),lease_token=NULL,lease_until=NULL,last_error='Delivery failed; inspect the linked operation and service diagnostics.' WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`,
|
||||
sql`UPDATE cms_outbox SET status=${claim.attempts >= 8 ? "failed" : "pending"},available_at=DATE_ADD(UTC_TIMESTAMP(3),INTERVAL ${retryDelay(claim.attempts)} SECOND),lease_token=NULL,lease_until=NULL,last_error=${reference} WHERE id=${claim.id} AND status='running' AND lease_token=${claim.token}`,
|
||||
);
|
||||
},
|
||||
};
|
||||
export async function listEffects() {
|
||||
export async function listEffects(id?: string) {
|
||||
if (id !== undefined && !isDeliveryReference(id)) return [];
|
||||
const filter = id === undefined ? sql`` : sql`WHERE e.id=${id}`;
|
||||
const [rows] = await db.execute(
|
||||
sql`SELECT e.id,e.operation_id AS operationId,e.topic,e.status,e.attempts,e.last_error AS lastError,e.created_at AS createdAt,e.available_at AS availableAt,o.actor_id AS actorId,o.kind,o.result_json AS resultJson FROM cms_outbox e JOIN cms_operations o ON o.id=e.operation_id ORDER BY e.created_at DESC,e.id DESC LIMIT 100`,
|
||||
sql`SELECT e.id,e.operation_id AS operationId,e.topic,e.status,e.attempts,e.last_error AS lastError,e.created_at AS createdAt,e.available_at AS availableAt,o.actor_id AS actorId,o.kind,o.result_json AS resultJson FROM cms_outbox e JOIN cms_operations o ON o.id=e.operation_id ${filter} ORDER BY e.created_at DESC,e.id DESC LIMIT 100`,
|
||||
);
|
||||
return rows as unknown as Array<{
|
||||
id: string;
|
||||
|
||||
@@ -49,8 +49,9 @@ it("invalidates the shared news cache before acknowledging delivery", async () =
|
||||
expect(mocks.fail).not.toHaveBeenCalled();
|
||||
});
|
||||
it("keeps a failed news invalidation retryable", async () => {
|
||||
mocks.news.mockRejectedValue(new Error("Redis unavailable"));
|
||||
const error = new Error("Redis unavailable");
|
||||
mocks.news.mockRejectedValue(error);
|
||||
await drainOperationEffects();
|
||||
expect(mocks.fail).toHaveBeenCalledWith(effect);
|
||||
expect(mocks.fail).toHaveBeenCalledWith(effect, error);
|
||||
expect(mocks.complete).not.toHaveBeenCalled();
|
||||
});
|
||||
Reference in new issue
Block a user