fix: show aggregation errors in Chart.svelte instead of an infinite spinner

web-common/src/features/components/charts/Chart.spec.ts

@@ -0,0 +1,74 @@
+import Chart from "@rilldata/web-common/features/components/charts/Chart.svelte";
+import type { ChartDataResult } from "@rilldata/web-common/features/components/charts";
+import { render } from "@testing-library/svelte";
+import chroma from "chroma-js";
+import { readable } from "svelte/store";
+import { describe, expect, it } from "vitest";
+
+const measure = { name: "operation_jx_cnt", displayName: "JX count" };
+
+function chartData(partial: Partial<ChartDataResult>): ChartDataResult {
+  return {
+    data: [],
+    isFetching: false,
+    fields: {},
+    isDarkMode: false,
+    hasComparison: false,
+    theme: {
+      primary: chroma("#1d4ed8"),
+      secondary: chroma("#7c3aed"),
+    },
+    ...partial,
+  };
+}
+
+const chartSpec = {
+  metrics_view: "ncar_jx_detail_metrics",
+  x: { field: "cal_date", type: "temporal" as const },
+  y: { field: "operation_jx_cnt", type: "quantitative" as const },
+};
+
+function renderChart(data: ChartDataResult, measures = [measure]) {
+  return render(Chart, {
+    props: {
+      chartType: "line_chart",
+      chartSpec,
+      chartData: readable(data),
+      measures,
+      isCanvas: false,
+      view: undefined,
+    },
+  });
+}
+
+describe("Chart loading and error states", () => {
+  it("shows the query error instead of the spinner", () => {
+    const { container, queryByText } = renderChart(
+      chartData({
+        isFetching: true,
+        error: new Error("time range has too many bins"),
+      }),
+    );
+
+    expect(queryByText("time range has too many bins")).toBeInTheDocument();
+    expect(container.querySelector(".status")).toBeNull();
+  });
+
+  it("shows the spinner while fetching without an error", () => {
+    const { container, queryByText } = renderChart(
+      chartData({ isFetching: true }),
+    );
+
+    expect(container.querySelector(".status")).not.toBeNull();
+    expect(queryByText("No Data to Display")).toBeNull();
+  });
+
+  it("shows an empty state when the query resolves with no rows", () => {
+    const { container, queryByText } = renderChart(
+      chartData({ isFetching: false, data: [] }),
+    );
+
+    expect(queryByText("No Data to Display")).toBeInTheDocument();
+    expect(container.querySelector(".status")).toBeNull();
+  });
+});

web-common/src/features/components/charts/Chart.svelte

@@ -243,12 +243,12 @@
   });
 </script>
 
-{#if isFetching || measures.length === 0}
+{#if error}
+  <ComponentError error={error.message} />
+{:else if isFetching || measures.length === 0}
   <div class="flex items-center justify-center h-full w-full">
     <Spinner status={EntityStatus.Running} size="20px" />
   </div>
-{:else if error}
-  <ComponentError error={error.message} />
 {:else if hasNoData}
   <div
     class="flex w-full h-full p-2 text-xl text-fg-disabled items-center justify-center"

web-common/vitest-setup.ts

@@ -22,6 +22,40 @@ Object.defineProperty(window, "scrollTo", {
   value: vi.fn(),
 });
 
+// Node 22+ exposes a global `localStorage` that is undefined unless
+// --localstorage-file is set. Theme and other stores read the global at
+// import time, so pin it to a memory-backed mock for jsdom tests.
+if (
+  typeof globalThis.localStorage === "undefined" ||
+  typeof globalThis.localStorage?.getItem !== "function"
+) {
+  const memory = new Map<string, string>();
+  const localStorageMock: Storage = {
+    get length() {
+      return memory.size;
+    },
+    clear: () => memory.clear(),
+    getItem: (key: string) => memory.get(key) ?? null,
+    key: (index: number) => [...memory.keys()][index] ?? null,
+    removeItem: (key: string) => {
+      memory.delete(key);
+    },
+    setItem: (key: string, value: string) => {
+      memory.set(key, value);
+    },
+  };
+  Object.defineProperty(globalThis, "localStorage", {
+    configurable: true,
+    writable: true,
+    value: localStorageMock,
+  });
+  Object.defineProperty(window, "localStorage", {
+    configurable: true,
+    writable: true,
+    value: localStorageMock,
+  });
+}
+
 Settings.defaultWeekSettings = {
   minimalDays: 4,
   firstDay: 1,