From b9b9dfdfbfb2aee10442cd8050452d22ed630696 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 30 May 2026 23:04:04 +0000 Subject: [PATCH] fix: stop spamming Sentry when draft publish/discard hits expected 4xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent sidebar's DraftActionsWidget unconditionally called console.error whenever the publish/discard request returned a non-OK response. The frontend Sentry SDK is configured with captureConsoleIntegration({ levels: ['warn', 'error'] }), so a normal business-logic outcome — most commonly a 400 'only draft versions can be published' when the version was already published from another tab/CLI — was being shipped to Sentry as a noisy error event. Handle 4xx responses as expected outcomes: surface the server's message inline (no console.error, no Sentry event) and silently dismiss the widget when the response indicates the draft is stale (version already published, missing, or owned by someone else). Reserve console.error / Sentry for true failures (5xx and network errors). --- .../widgets/DraftActionsWidget.spec.tsx | 107 +++++++++++++++ .../widgets/DraftActionsWidget.tsx | 124 +++++++++++++----- 2 files changed, 197 insertions(+), 34 deletions(-) create mode 100644 web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.spec.tsx diff --git a/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.spec.tsx b/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.spec.tsx new file mode 100644 index 0000000000..f11c275cff --- /dev/null +++ b/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.spec.tsx @@ -0,0 +1,107 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { DraftActionsWidget } from "./DraftActionsWidget"; + +type FetchResponseInit = { + ok: boolean; + status: number; + body: string; +}; + +function mockFetchResponse({ ok, status, body }: FetchResponseInit): ReturnType { + const fetchMock = vi.fn().mockResolvedValue({ + ok, + status, + text: async () => body, + }); + vi.stubGlobal("fetch", fetchMock); + return fetchMock; +} + +describe("DraftActionsWidget", () => { + const baseProps = { + versionId: "ver-1", + canvasId: "canvas-1", + organizationId: "org-1", + isEditing: false, + }; + + let errorSpy: ReturnType; + + beforeEach(() => { + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + errorSpy.mockRestore(); + vi.unstubAllGlobals(); + }); + + it("dismisses silently when the API reports the draft is no longer publishable", async () => { + mockFetchResponse({ + ok: false, + status: 400, + body: JSON.stringify({ code: 9, message: "only draft versions can be published", details: [] }), + }); + const onDismiss = vi.fn(); + + render(); + fireEvent.click(screen.getByRole("button", { name: /publish/i })); + + await waitFor(() => expect(onDismiss).toHaveBeenCalledTimes(1)); + + // Critically: we don't log to console.error for an expected 4xx outcome, so + // captureConsoleIntegration doesn't ship a Sentry event for this case. + expect(errorSpy).not.toHaveBeenCalled(); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("shows an inline error (and stays mounted) on other 4xx responses without logging", async () => { + mockFetchResponse({ + ok: false, + status: 400, + body: JSON.stringify({ code: 9, message: "change management is enabled for this canvas", details: [] }), + }); + const onDismiss = vi.fn(); + + render(); + fireEvent.click(screen.getByRole("button", { name: /publish/i })); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("change management is enabled for this canvas"); + expect(onDismiss).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it("logs a console.error for unexpected 5xx responses so they reach Sentry", async () => { + mockFetchResponse({ + ok: false, + status: 500, + body: JSON.stringify({ message: "internal error" }), + }); + const onDismiss = vi.fn(); + + render(); + fireEvent.click(screen.getByRole("button", { name: /publish/i })); + + await waitFor(() => expect(errorSpy).toHaveBeenCalledTimes(1)); + expect(errorSpy.mock.calls[0]?.[0]).toContain("publish failed:"); + expect(onDismiss).not.toHaveBeenCalled(); + expect(await screen.findByRole("alert")).toHaveTextContent("internal error"); + }); + + it("invokes onDismiss after a successful publish", async () => { + const fetchMock = mockFetchResponse({ ok: true, status: 200, body: "" }); + const onDismiss = vi.fn(); + + render(); + fireEvent.click(screen.getByRole("button", { name: /publish/i })); + + await waitFor(() => expect(onDismiss).toHaveBeenCalledTimes(1)); + expect(fetchMock).toHaveBeenCalledWith( + "/api/v1/canvases/canvas-1/versions/ver-1/publish", + expect.objectContaining({ method: "PATCH" }), + ); + expect(errorSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.tsx b/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.tsx index 1086bbe89a..877eefe76b 100644 --- a/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.tsx +++ b/web_src/src/components/AgentSidebar/widgets/DraftActionsWidget.tsx @@ -11,6 +11,13 @@ export interface DraftActionsWidgetProps { onDismiss?: () => void; } +type DraftAction = "publish" | "discard"; + +// Server messages that indicate the widget is showing a stale draft (it was +// already published / discarded by someone else, or no longer exists). We +// silently dismiss in those cases so the user is not blocked by a stale UI. +const STALE_DRAFT_MARKERS = ["only draft versions can be published", "version not found", "version owner mismatch"]; + export function DraftActionsWidget({ versionId, message, @@ -19,14 +26,16 @@ export function DraftActionsWidget({ isEditing, onDismiss, }: DraftActionsWidgetProps) { - const [busy, setBusy] = useState<"publish" | "discard" | null>(null); + const [busy, setBusy] = useState(null); + const [error, setError] = useState(null); const handleViewInEditor = () => { window.dispatchEvent(new CustomEvent("agent:view-version", { detail: { versionId } })); }; - const callApi = async (method: string, url: string, action: "publish" | "discard") => { + const callApi = async (method: string, url: string, action: DraftAction) => { setBusy(action); + setError(null); try { const response = await fetch(url, { method, @@ -38,11 +47,29 @@ export function DraftActionsWidget({ }); if (response.ok) { onDismiss?.(); - } else { - const text = await response.text(); - console.error(`${action} failed:`, response.status, text); + return; + } + + const text = await response.text(); + const apiMessage = extractApiMessage(text); + + // 4xx responses are expected business-logic outcomes (e.g. the draft was + // already published from another tab/CLI). Surface them inline and, when + // the widget is clearly stale, dismiss it — but never `console.error`, + // since that ships noise to Sentry via captureConsoleIntegration. + if (response.status >= 400 && response.status < 500) { + if (isStaleDraftMessage(apiMessage)) { + onDismiss?.(); + return; + } + setError(apiMessage ?? `Failed to ${action} (HTTP ${response.status}).`); + return; } + + setError(apiMessage ?? `Failed to ${action} (HTTP ${response.status}).`); + console.error(`${action} failed:`, response.status, text); } catch (err) { + setError(err instanceof Error ? err.message : `Failed to ${action}.`); console.error(`Failed to ${action}:`, err); } finally { setBusy(null); @@ -54,41 +81,70 @@ export function DraftActionsWidget({ const handleDiscard = () => callApi("DELETE", `/api/v1/canvases/${canvasId}/versions/${versionId}`, "discard"); return ( -
- {message && {message}} - {!message && Draft ready} - {!isEditing && ( +
+ {error && ( +
+ {error} +
+ )} +
+ {message && {message}} + {!message && Draft ready} + {!isEditing && ( + + )} - )} - - + +
); } + +function extractApiMessage(text: string): string | null { + const trimmed = text.trim(); + if (!trimmed) return null; + + try { + const parsed = JSON.parse(trimmed) as { message?: unknown }; + if (typeof parsed.message === "string" && parsed.message.trim().length > 0) { + return parsed.message.trim(); + } + } catch { + // Body was not JSON; fall through to the raw text. + } + + return trimmed; +} + +function isStaleDraftMessage(message: string | null): boolean { + if (!message) return false; + const lower = message.toLowerCase(); + return STALE_DRAFT_MARKERS.some((marker) => lower.includes(marker)); +}