@@ -10,6 +10,22 @@ import {
1010} from "./exec-approval-forwarder.js";
1111import type { ExecApprovalRequest } from "./exec-approvals.js";
121213+const { mockLogError } = vi.hoisted(() => ({ mockLogError: vi.fn() }));
14+vi.mock("../logging/subsystem.js", () => ({
15+createSubsystemLogger: () => ({
16+subsystem: "gateway/exec-approvals",
17+isEnabled: () => false,
18+trace: vi.fn(),
19+debug: vi.fn(),
20+info: vi.fn(),
21+warn: vi.fn(),
22+error: mockLogError,
23+fatal: vi.fn(),
24+raw: vi.fn(),
25+child: vi.fn(),
26+}),
27+}));
28+1329const baseRequest = {
1430id: "req-1",
1531request: {
@@ -704,4 +720,106 @@ describe("exec approval forwarder", () => {
704720705721expect(deliver).toHaveBeenCalledTimes(1);
706722});
723+724+describe("expiry delivery error handling (#83106)", () => {
725+afterEach(() => {
726+mockLogError.mockClear();
727+});
728+729+it("logs per-target error when expiry delivery fails without producing unhandled rejection", async () => {
730+vi.useFakeTimers();
731+const deliver = vi.fn().mockResolvedValue([]);
732+const { forwarder } = createForwarder({ cfg: TARGETS_CFG, deliver });
733+734+await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true);
735+await flushPendingDelivery();
736+737+// Make the expiry delivery throw — deliverToTargets catches this
738+// per-target and logs it, preventing an unhandled rejection.
739+deliver.mockRejectedValue(new Error("channel delivery crashed"));
740+741+// Trigger expiry
742+await vi.advanceTimersByTimeAsync(baseRequest.expiresAtMs - 1000);
743+await flushPendingDelivery();
744+745+// deliverToTargets catches per-target errors and logs them
746+expect(mockLogError).toHaveBeenCalledWith(expect.stringContaining("failed to deliver"));
747+expect(mockLogError).toHaveBeenCalledWith(
748+expect.stringContaining("channel delivery crashed"),
749+);
750+});
751+752+it("cleans up pending entry after successful expiry delivery", async () => {
753+vi.useFakeTimers();
754+const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG });
755+756+await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true);
757+await flushPendingDelivery();
758+deliver.mockClear();
759+760+// Trigger expiry
761+await vi.advanceTimersByTimeAsync(baseRequest.expiresAtMs - 1000);
762+await flushPendingDelivery();
763+764+expect(deliver).toHaveBeenCalledTimes(1);
765+const expiryText =
766+(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> }).payloads?.[0]
767+?.text ?? "";
768+expect(expiryText).toContain("expired");
769+770+// After expiry, the pending entry should be cleaned up.
771+deliver.mockClear();
772+await forwarder.handleResolved({
773+id: baseRequest.id,
774+decision: "allow-once",
775+resolvedBy: "slack:U123",
776+ts: 7000,
777+});
778+// No delivery because pending entry was already deleted before delivery
779+expect(deliver).not.toHaveBeenCalled();
780+});
781+782+it("deletes pending entry before starting expiry delivery", async () => {
783+vi.useFakeTimers();
784+let pendingDeletedDuringDelivery = false;
785+786+const deliver = vi
787+.fn()
788+.mockImplementation(async (deliveryParams: { payloads?: Array<{ text?: string }> }) => {
789+const text = deliveryParams.payloads?.[0]?.text ?? "";
790+if (text.includes("expired")) {
791+// During expiry delivery, try to resolve the same request.
792+// If pending.delete happened before delivery, handleResolved
793+// will not find the entry and will not deliver a resolved notice.
794+const resolveResult = await forwarder.handleResolved({
795+id: baseRequest.id,
796+decision: "allow-once",
797+resolvedBy: "slack:U123",
798+ts: 7000,
799+});
800+// handleResolved returns void, but if it tried to deliver,
801+// deliver would be called again. We track that below.
802+pendingDeletedDuringDelivery = true;
803+}
804+return [];
805+});
806+807+const { forwarder } = createForwarder({ cfg: TARGETS_CFG, deliver });
808+await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true);
809+await flushPendingDelivery();
810+deliver.mockClear();
811+812+// Trigger expiry
813+await vi.advanceTimersByTimeAsync(baseRequest.expiresAtMs - 1000);
814+for (let i = 0; i < 5; i += 1) {
815+await flushPendingDelivery();
816+}
817+818+expect(pendingDeletedDuringDelivery).toBe(true);
819+// Only 1 delivery call (the expiry notification itself).
820+// handleResolved during delivery found no pending entry because
821+// pending.delete ran before deliverToTargets, so no resolved notice.
822+expect(deliver).toHaveBeenCalledTimes(1);
823+});
824+});
707825});
此内容由惯性聚合(RSS阅读器)自动聚合整理，仅供阅读参考。 原文来自 — 版权归原作者所有。