Skip to content

Commit 05e38e1

Browse files
committed
refactor(frontend): throw from api fetchers instead of returning status objects
Deletes the *ApiStatus enums. Fetchers now call a single request helper that throws ApiError on a non-ok response and RequestError on a network or parse failure, and returns the parsed body otherwise. Empty results come back as null, an empty array, or a discriminated union. Query hooks no longer translate statuses by hand, which is what caused the crash fixed in #4097. The filters store keeps its own load state as string unions. Closes #4096
1 parent 0b89ccf commit 05e38e1

22 files changed

Lines changed: 1598 additions & 3711 deletions

frontend/dashboard/__tests__/api/api_calls_test.ts

Lines changed: 384 additions & 579 deletions
Large diffs are not rendered by default.

frontend/dashboard/__tests__/components/filters_test.tsx

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,7 @@ jest.mock("@/app/stores/provider", () => ({
103103
useStore(storeInstance, selector ?? ((s: any) => s)),
104104
}));
105105

106-
import {
107-
App,
108-
AppsApiStatus,
109-
AppVersion,
110-
FiltersApiStatus,
111-
FilterSource,
112-
OsVersion,
113-
RootSpanNamesApiStatus,
114-
} from "@/app/api/api_calls";
106+
import { App, AppVersion, FilterSource, OsVersion } from "@/app/api/api_calls";
115107
import Filters, {
116108
AppVersionsInitialSelectionType,
117109
deserializeUrlFilters,
@@ -195,13 +187,13 @@ function setAppsPending() {
195187
function setAppsSuccess(apps: App[]) {
196188
appsQueryState = {
197189
status: "success",
198-
data: { status: AppsApiStatus.Success, data: apps },
190+
data: apps,
199191
};
200192
}
201193
function setAppsNoApps() {
202194
appsQueryState = {
203195
status: "success",
204-
data: { status: AppsApiStatus.NoApps, data: [] },
196+
data: [],
205197
};
206198
}
207199
function setAppsError() {
@@ -211,25 +203,25 @@ function setAppsError() {
211203
function setFiltersSuccess() {
212204
filterOptionsQueryState = {
213205
status: "success",
214-
data: { status: FiltersApiStatus.Success, data: filterOptionsFixture },
206+
data: { kind: "options", data: filterOptionsFixture },
215207
};
216208
}
217209
function setFiltersNoData() {
218210
filterOptionsQueryState = {
219211
status: "success",
220-
data: { status: FiltersApiStatus.NoData, data: null },
212+
data: { kind: "no-data" },
221213
};
222214
}
223215
function setFiltersNoBuilds() {
224216
filterOptionsQueryState = {
225217
status: "success",
226-
data: { status: FiltersApiStatus.NoBuilds, data: null },
218+
data: { kind: "no-builds" },
227219
};
228220
}
229221
function setFiltersNotOnboarded() {
230222
filterOptionsQueryState = {
231223
status: "success",
232-
data: { status: FiltersApiStatus.NotOnboarded, data: null },
224+
data: { kind: "not-onboarded" },
233225
};
234226
}
235227
function setFiltersError() {
@@ -246,9 +238,14 @@ function setFiltersPending() {
246238
function setRootSpansSuccess(names = ["root.a", "root.b"]) {
247239
rootSpanNamesQueryState = {
248240
status: "success",
249-
data: { status: RootSpanNamesApiStatus.Success, data: names },
241+
data: names,
250242
};
251243
}
244+
// An app that has never reported a trace answers with a null list. The
245+
// fetcher sends the null through, and does not change it to an empty array.
246+
function setRootSpansNoData() {
247+
rootSpanNamesQueryState = { status: "success", data: null };
248+
}
252249
function setRootSpansPending() {
253250
rootSpanNamesQueryState = { status: "pending", data: undefined };
254251
}
@@ -420,6 +417,17 @@ describe("Filters — Span filter source", () => {
420417
});
421418
});
422419

420+
it("shows the no-traces message when the app has never reported a trace", async () => {
421+
setRootSpansNoData();
422+
await renderFilters({ filterSource: FilterSource.Spans });
423+
await waitFor(() => {
424+
expect(
425+
screen.getByText(/No traces received for this app yet/),
426+
).toBeInTheDocument();
427+
});
428+
expect(storeInstance.getState().rootSpanNamesState).toBe("no-data");
429+
});
430+
423431
it("renders the trace name dropdown once root span names load", async () => {
424432
setRootSpansSuccess();
425433
await renderFilters({ filterSource: FilterSource.Spans });
@@ -447,8 +455,16 @@ describe("Filters — store state after queries resolve", () => {
447455
await waitFor(() => {
448456
const state = storeInstance.getState();
449457
expect(state.apps).toHaveLength(1);
450-
expect(state.appsApiStatus).toBe(AppsApiStatus.Success);
451-
expect(state.filtersApiStatus).toBe(FiltersApiStatus.Success);
458+
expect(state.appsState).toBe("loaded");
459+
expect(state.filterOptionsState).toBe("loaded");
460+
});
461+
});
462+
463+
it("mirrors an empty apps response as the no-apps state", async () => {
464+
setAppsNoApps();
465+
await renderFilters();
466+
await waitFor(() => {
467+
expect(storeInstance.getState().appsState).toBe("no-apps");
452468
});
453469
});
454470

@@ -493,7 +509,7 @@ describe("Filters — store state after queries resolve", () => {
493509
setAppsSuccess([makeApp("a")]);
494510
filterOptionsQueryState = {
495511
status: "success",
496-
data: { status: FiltersApiStatus.Success, data: fixture },
512+
data: { kind: "options", data: fixture },
497513
};
498514
await renderFilters();
499515
await waitFor(() => {
@@ -542,7 +558,7 @@ describe("Filters — selectedApp sync on refetch", () => {
542558
await act(async () => {
543559
// Mirrors the refetch landing in the store; forces the sync effect
544560
// (keyed on appsQuery.data) to re-run against the fresh apps list.
545-
storeInstance.getState().setApps([next], AppsApiStatus.Success);
561+
storeInstance.getState().setApps([next], "loaded");
546562
});
547563

548564
await waitFor(() => {
@@ -563,7 +579,7 @@ describe("Filters — selectedApp sync on refetch", () => {
563579
const next = makeApp("a", true);
564580
setAppsSuccess([next]);
565581
await act(async () => {
566-
storeInstance.getState().setApps([next], AppsApiStatus.Success);
582+
storeInstance.getState().setApps([next], "loaded");
567583
});
568584

569585
await waitFor(() => {
@@ -584,7 +600,7 @@ describe("Filters — selectedApp sync on refetch", () => {
584600
const refetched = makeApp("a");
585601
setAppsSuccess([refetched]);
586602
await act(async () => {
587-
storeInstance.getState().setApps([refetched], AppsApiStatus.Success);
603+
storeInstance.getState().setApps([refetched], "loaded");
588604
});
589605

590606
// appsEqual sees no change, so the store keeps the same object reference.

frontend/dashboard/__tests__/components/metrics_card_test.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ jest.mock("lucide-react", () => ({
9999
),
100100
}));
101101

102-
// MetricsCard now uses string union type 'pending' | 'success' | 'error' instead of MetricsApiStatus enum
103-
104102
describe("MetricsCard", () => {
105103
const createCrashFreeSessionsProps = (
106104
overrides = {},

0 commit comments

Comments
 (0)