diff --git a/src/app/components/ChartExport.test.tsx b/src/app/components/ChartExport.test.tsx new file mode 100644 index 0000000..e3eba4d --- /dev/null +++ b/src/app/components/ChartExport.test.tsx @@ -0,0 +1,87 @@ +/** + * ChartExport — image-export failure vs not-ready (arch 05 §7 fail-loud). + * + * `getImageUrl` (injected by LivePreview) returns `null` ONLY when no view is + * live — the not-ready case — and rejects on a real rasterize/serialize failure. + * These two outcomes must land as DIFFERENT toasts: the not-ready one tells the + * user to wait (no `detail`), the failure one carries diagnostic `detail` so the + * problem can be reported. Burying the rejection behind the not-ready copy is the + * regression this guards. + */ + +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { useNotificationStore } from '../stores/NotificationStore'; +import { usePopoverStore } from '../stores/PopoverStore'; +import { useSnippetStore } from '../stores/SnippetStore'; +import { useDatasetStore } from '../stores/DatasetStore'; +import { ChartExport, type ChartExportProps } from './ChartExport'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + useNotificationStore.getState().clear(); + usePopoverStore.setState({ openId: null }); + useSnippetStore.getState().reset(); + useDatasetStore.getState().reset(); + // A non-empty draft makes `hasSpec` true so the export trigger is enabled. + useSnippetStore.setState({ draftText: '{"mark":"point"}' }); + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + useNotificationStore.getState().clear(); + usePopoverStore.setState({ openId: null }); + useSnippetStore.getState().reset(); + useDatasetStore.getState().reset(); +}); + +/** Mount, open the export popover (portaled to ), and click "Download PNG". + * `downloadImage` awaits `getImageUrl`, so the caller flushes microtasks after. */ +async function mountAndDownloadPng(getImageUrl: ChartExportProps['getImageUrl']) { + act(() => { + root.render(); + }); + act(() => usePopoverStore.getState().show('chart-export')); + const pngBtn = Array.from(document.body.querySelectorAll('button')).find( + (b) => b.textContent?.trim() === 'Download PNG', + ); + if (!pngBtn) throw new Error('Download PNG button not found — popover did not render'); + await act(async () => { + pngBtn.dispatchEvent(new MouseEvent('click', { bubbles: true })); + await Promise.resolve(); + }); +} + +describe('ChartExport image-export error handling', () => { + test('a rejecting getImageUrl surfaces the failure with diagnostic detail', async () => { + await mountAndDownloadPng(vi.fn().mockRejectedValue(new TypeError('tainted canvas'))); + + const toasts = useNotificationStore.getState().notifications; + expect(toasts).toHaveLength(1); + expect(toasts[0].kind).toBe('error'); + // A real failure carries its diagnostic detail (not the "wait, it's not ready" copy). + expect(toasts[0].detail).toBe('TypeError: tainted canvas'); + expect(toasts[0].message).not.toMatch(/ready yet/i); + }); + + test('a null getImageUrl reports not-ready, with no diagnostic detail', async () => { + await mountAndDownloadPng(vi.fn().mockResolvedValue(null)); + + const toasts = useNotificationStore.getState().notifications; + expect(toasts).toHaveLength(1); + expect(toasts[0].kind).toBe('error'); + expect(toasts[0].message).toMatch(/ready yet/i); + // Nothing for the user to report — the not-ready case omits detail. + expect(toasts[0].detail).toBeUndefined(); + }); +}); diff --git a/src/app/components/ChartExport.tsx b/src/app/components/ChartExport.tsx index 0f12fad..1fc99c3 100644 --- a/src/app/components/ChartExport.tsx +++ b/src/app/components/ChartExport.tsx @@ -89,8 +89,9 @@ export interface ChartExportProps { /** Whether a chart is currently rendered — gates the image (PNG/SVG) actions. * The spec actions only need text, so they ignore this. */ chartReady: boolean; - /** Rasterize/serialize the live view to a downloadable URL, or null if the view - * isn't available (caller owns the Vega view). */ + /** Rasterize/serialize the live view to a downloadable URL, or null if no view + * is live (the not-ready case). Rejects if rasterizing/serializing fails — a real + * error the caller reports, not a not-ready state (caller owns the Vega view). */ getImageUrl: ( format: 'png' | 'svg', options: { scale: number; background: string | null }, @@ -165,10 +166,24 @@ export function ChartExport({ chartReady, getImageUrl }: ChartExportProps) { const downloadImage = async (format: 'png' | 'svg') => { close(); - const url = await getImageUrl(format, { - scale: Number(scale), - background: resolveBackground(background), - }); + let url: string | null; + try { + url = await getImageUrl(format, { + scale: Number(scale), + background: resolveBackground(background), + }); + } catch (err) { + // The view is live but rasterizing/serializing it threw — a real failure + // (e.g. a tainted canvas or an SVG the browser won't serialize), not a + // not-ready state. Surface it with its detail rather than burying it. + notify({ + kind: 'error', + title: 'Couldn’t export image', + message: `Astrolabe couldn’t turn the chart into ${format.toUpperCase()}. This can happen with very large charts or content the browser won’t serialize.`, + detail: err instanceof Error ? `${err.name}: ${err.message}` : String(err), + }); + return; + } if (!url) { notify({ kind: 'error', diff --git a/src/app/components/LivePreview.tsx b/src/app/components/LivePreview.tsx index 4edd6c8..c57c7e3 100644 --- a/src/app/components/LivePreview.tsx +++ b/src/app/components/LivePreview.tsx @@ -342,8 +342,11 @@ export function LivePreview() { ]); // Rasterize/serialize the live view for the per-chart export (spec §08). Reads - // `handleRef` (the view LivePreview owns); returns null when no view is live or - // the export fails, so the export UI can report it. Stable identity (no deps). + // `handleRef` (the view LivePreview owns); returns null only when no view is live + // (the "not ready" case). A rasterize/serialize *failure* is a real error, not a + // not-ready state, so it propagates for the export UI to report with its detail — + // burying it here would misreport a tainted canvas or a serialization bug as + // "not ready yet" (arch 02 fail-loud). Stable identity (no deps). const getImageUrl = useCallback( async ( format: 'png' | 'svg', @@ -351,11 +354,7 @@ export function LivePreview() { ): Promise => { const handle = handleRef.current; if (!handle) return null; - try { - return await handle.toImageURL(format, options); - } catch { - return null; - } + return await handle.toImageURL(format, options); }, [], );