app: api helpers report failure instead of returning an empty value

Six helpers turned a non-OK response into an empty value, so their
consumers rendered a server error as "nothing here":

* getConfigBackups -> {dates: []}: "No backups available for this
  config."  A thrown request also left "Loading backups..." forever.
* getInventoryBackups -> []: "No backups yet."
* getDownloaders -> {downloaders: []} on a throw, the error body on a
  non-OK response (which then crashed the .map): the RSS form's
  Downloader select sat empty.
* searchMap -> {results: []}: "No map locations found".
* getMapSearchStatus -> {indexed: [], missing: []}: "No maps are
  installed".
* flasherSearch -> [null, null], then `|| []`: "No .bin firmware
  found".

Invariants now enforced and tested:
* On a non-OK response each helper returns undefined; an empty value
  only ever comes from the server.
* Each consumer shows an error for a failed or thrown fetch and keeps
  its "none" message for a confirmed empty result.  The Downloader
  select is disabled and says it is loading or could not fetch.  A
  failed map status is unknown, never "no maps are installed".
* The map search tab loads from the first render, and a query shorter
  than the two characters the request needs now says so instead of
  "No map locations found".

Archive browsers and recent tags are left as they are: a failed fetch
there hides an optional picker or suggestion list without claiming
anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-09-26 17:33:24 -06:00
parent eead78007d
commit fc1ffc0d1d
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
13 changed files with 375 additions and 44 deletions

View file

@ -0,0 +1,84 @@
import {
flasherSearch, getConfigBackups, getDownloaders, getInventoryBackups, getMapSearchStatus, searchMap,
} from './api';
// A helper that fails must say so. These used to turn a non-OK response into an empty value
// ([], {dates: []}, {results: []}, ...), so every consumer rendered "nothing here" for a server
// error. They now return undefined; an empty value only ever comes from the server.
jest.mock('./components/ui', () => ({
...jest.requireActual('./components/ui'),
toast: jest.fn(),
}));
const response = (ok, body = {}) => {
const r = {
ok,
status: ok ? 200 : 500,
json: async () => body,
text: async () => JSON.stringify(body),
};
r.clone = () => r;
return r;
};
let originalFetch;
beforeEach(() => {
originalFetch = global.fetch;
});
afterEach(() => {
global.fetch = originalFetch;
});
const failWith = () => {
global.fetch = jest.fn(() => Promise.resolve(response(false, {error: 'boom'})));
};
const succeedWith = (body) => {
global.fetch = jest.fn(() => Promise.resolve(response(true, body)));
};
describe('on a non-OK response, the helper returns undefined', () => {
test.each([
['getConfigBackups', () => getConfigBackups('wrolpi.yaml')],
['getInventoryBackups', () => getInventoryBackups('food')],
['getDownloaders', () => getDownloaders()],
['searchMap', () => searchMap('camp')],
['getMapSearchStatus', () => getMapSearchStatus()],
['flasherSearch', () => flasherSearch(null, null)],
])('%s', async (_name, call) => {
failWith();
expect(await call()).toBeUndefined();
});
});
describe('a confirmed empty result is still empty', () => {
test('getConfigBackups', async () => {
succeedWith({dates: []});
expect(await getConfigBackups('wrolpi.yaml')).toEqual({dates: []});
});
test('getInventoryBackups', async () => {
succeedWith({dates: []});
expect(await getInventoryBackups('food')).toEqual([]);
});
test('getDownloaders', async () => {
succeedWith({downloaders: []});
expect(await getDownloaders()).toEqual({downloaders: []});
});
test('searchMap', async () => {
succeedWith({results: [], total: 0});
expect(await searchMap('camp')).toEqual({results: [], total: 0});
});
test('getMapSearchStatus', async () => {
succeedWith({indexed: [], missing: []});
expect(await getMapSearchStatus()).toEqual({indexed: [], missing: []});
});
test('flasherSearch', async () => {
succeedWith({file_groups: [], totals: {file_groups: 0}});
expect(await flasherSearch(null, null)).toEqual([[], 0]);
});
});

View file

@ -522,7 +522,7 @@ export async function getConfigBackups(fileName) {
if (response.ok) {
return await response.json();
}
return {dates: []};
// Undefined, not {dates: []}, so the modal can tell a failed fetch from having no backups.
}
export async function postConfigBackupPreview(fileName, backupDate, mode) {
@ -720,7 +720,7 @@ export async function getInventoryBackups(slug) {
if (response.ok) {
return (await response.json())['dates'];
}
return [];
// Undefined, not [], so the modal can tell a failed fetch from having no backups.
}
export async function postInventoryRestorePreview(slug, backupDate, mode) {
@ -1259,12 +1259,15 @@ export async function startDownloads() {
}
export async function getDownloaders() {
// Not toasting because this will happen often. Undefined on any failure, not an empty list, so
// the form can say the downloaders could not be fetched.
try {
const response = await apiGet(`${API_URI}/downloaders`);
// Not toasting because this will happen often.
return await response.json();
if (response.ok) {
return await response.json();
}
} catch (e) {
return {downloaders: []};
// The API is down; reported as undefined below.
}
}
@ -1351,7 +1354,7 @@ export async function flasherSearch(chip, path) {
}
const message = await getErrorMessage(response, 'Cannot search firmware. See server logs.');
toast({type: 'error', title: 'Unable to search firmware', description: message, time: 5000});
return [null, null];
// Undefined, not [null, null], so the picker can tell a failed search from finding nothing.
}
export async function getFlasherConfigs() {
@ -1658,7 +1661,8 @@ export async function searchMap(query, limit = 12, offset = 0, lat = null, lon =
}
const response = await apiGet(url);
if (!response.ok) {
return {results: [], total: 0};
// Undefined, not {results: []}, so a failed search is not shown as finding nothing.
return undefined;
}
return await response.json();
}
@ -1666,7 +1670,8 @@ export async function searchMap(query, limit = 12, offset = 0, lat = null, lon =
export async function getMapSearchStatus() {
const response = await apiGet(`${API_URI}/map/search/status`);
if (!response.ok) {
return {indexed: [], missing: []};
// Undefined, not {indexed: [], missing: []}, which read as "no maps are installed".
return undefined;
}
return await response.json();
}

View file

@ -247,28 +247,38 @@ export function DownloadFrequencySelector({
}
export function DownloaderSelector({form, name = 'sub_downloader', path = 'sub_downloader'}) {
const [downloaders, setDownloaders] = React.useState([]);
// null = pending, undefined = could not be fetched, a list = loaded. Until loaded, the select is
// disabled and says why, rather than sitting empty as though there were no downloaders.
const [downloaders, setDownloaders] = React.useState(null);
const fetchDownloaders = async () => {
let {downloaders: downloaders_} = await getDownloaders();
downloaders_ = downloaders_.map((i) => {
const result = await getDownloaders();
if (!result || !Array.isArray(result.downloaders)) {
setDownloaders(undefined);
return;
}
setDownloaders(result.downloaders.map((i) => {
return {key: i.name, text: i.pretty_name || i.name, value: i.name}
})
setDownloaders(downloaders_);
}));
}
React.useEffect(() => {
fetchDownloaders();
}, []);
const placeholder = downloaders === null ? 'Loading downloaders…'
: downloaders === undefined ? 'Could not fetch downloaders'
: 'Select a downloader';
return <SelectField
form={form}
name={name}
path={path}
required
label='Downloader'
placeholder='Select a downloader'
options={downloaders}
placeholder={placeholder}
options={downloaders || []}
disabled={!Array.isArray(downloaders)}
/>
}

View file

@ -0,0 +1,43 @@
import React from 'react';
import {act, render, screen} from '../test-utils';
import {createTestForm} from '../test-utils';
import {DownloaderSelector} from './Download';
import {getDownloaders} from '../api';
// The RSS form's Downloader select is filled from the API. Until the list arrives, or if it could
// not be fetched, the select must say so rather than sit empty as though there were no downloaders.
jest.mock('../api', () => ({
...jest.requireActual('../api'),
getDownloaders: jest.fn(),
}));
const makeForm = () => createTestForm({}, {
overrides: {getSelectionProps: () => [{value: null, onChange: jest.fn(), disabled: false, error: null}]},
});
beforeEach(() => getDownloaders.mockReset());
test('pending: the select says it is loading and is disabled', () => {
getDownloaders.mockReturnValue(new Promise(() => {
}));
render(<DownloaderSelector form={makeForm()}/>);
const input = screen.getByPlaceholderText('Loading downloaders…');
expect(input).toBeDisabled();
});
test('failed: the select says the downloaders could not be fetched', async () => {
getDownloaders.mockResolvedValue(undefined);
render(<DownloaderSelector form={makeForm()}/>);
await act(async () => {
});
expect(screen.getByPlaceholderText('Could not fetch downloaders')).toBeDisabled();
});
test('loaded: the select offers the downloaders', async () => {
getDownloaders.mockResolvedValue({downloaders: [{name: 'video', pretty_name: 'Videos'}]});
render(<DownloaderSelector form={makeForm()}/>);
await act(async () => {
});
expect(screen.getByPlaceholderText('Select a downloader')).toBeEnabled();
});

View file

@ -270,7 +270,8 @@ export function FlasherPage() {
// Media-directory firmware picker.
const [mediaFilter, setMediaFilter] = useState('');
const [mediaResults, setMediaResults] = useState([]);
// null = not searched yet, undefined = the search failed, a list = the server's results.
const [mediaResults, setMediaResults] = useState(null);
const [mediaLoading, setMediaLoading] = useState(false);
// When set (e.g. "ESP32-S2"), the picker only shows firmware for that chip (detected from the device).
const [deviceChip, setDeviceChip] = useState(null);
@ -393,14 +394,15 @@ export function FlasherPage() {
const fetchMediaFirmware = React.useCallback(async (pathFilter, chip) => {
setMediaLoading(true);
try {
const [fileGroups] = await flasherSearch(chip || null, pathFilter || null);
setMediaResults(fileGroups || []);
const result = await flasherSearch(chip || null, pathFilter || null);
// The api helper returns undefined (and toasts) on a non-OK response.
setMediaResults(result ? result[0] || [] : undefined);
} catch (e) {
// Surface connectivity errors instead of showing the empty "no firmware found" state, which would
// be indistinguishable from genuinely having no firmware.
setError(e && e.message ? e.message : String(e));
setBootHint(false);
setMediaResults([]);
setMediaResults(undefined);
} finally {
setMediaLoading(false);
}
@ -767,8 +769,10 @@ export function FlasherPage() {
onChange={(e) => handleMediaFilterChange(e.currentTarget.value)}
/>
<div style={{marginTop: '1em', overflowX: 'auto'}}>
{mediaLoading
{mediaLoading || mediaResults === null
? <Loading/>
: mediaResults === undefined
? <ErrorMessage>Could not search firmware.</ErrorMessage>
: (mediaResults.length === 0
? <p style={{opacity: 0.7}}>
No <code>.bin</code> firmware found

View file

@ -290,6 +290,18 @@ describe('FlasherPage', () => {
expect((await screen.findAllByText('No saved configurations yet.')).length).toBeGreaterThan(0);
});
// "No .bin firmware found" may only appear once the server searched and found none.
it('shows an error, not "no firmware found", when the firmware search failed', async () => {
Object.defineProperty(global.navigator, 'serial', {value: {}, configurable: true});
// The api helper returns undefined on a non-OK response.
flasherSearch.mockResolvedValue(undefined);
render(<FlasherPage/>);
await waitFor(() => expect(flasherSearch).toHaveBeenCalled());
switchTab('Choose from your WROLPi');
expect((await screen.findAllByText(/Could not search firmware/)).length).toBeGreaterThan(0);
expect(screen.queryAllByText(/firmware found/)).toHaveLength(0);
});
// PageContainer renders both the mobile and desktop breakpoints (fresnel Media), so each label appears twice.
it('warns when Web Serial is unavailable', () => {
delete global.navigator.serial;

View file

@ -2,7 +2,7 @@ import React, {useEffect, useState} from "react";
import {Link, useNavigate} from "react-router";
import {getMapSearchStatus, searchMap} from "../api";
import {Card, CardGroup, Header, Loading, Panel} from "./ui";
import {Paginator} from "./Common";
import {ErrorMessage, Paginator} from "./Common";
import {useLatestRequest, usePages} from "../hooks/customHooks";
import {QueryContext} from "../contexts/contexts";
import {MAP_VIEWER_URI} from "./Vars";
@ -68,16 +68,24 @@ export function MapSearchView() {
const {offset, limit, activePage, setPage, totalPages, setTotal} = usePages(12);
// useLatestRequest discards stale responses and catches errors, so rapid pagination
// can't show out-of-order results and a network failure can't leave the spinner stuck.
const {data, sendRequest, loading} = useLatestRequest(300);
// Loading from the first render: a query of two or more characters always sends a request (below),
// so this settles, and the shorter-query branches render before the loading check.
const {data, sendRequest, loading} = useLatestRequest(300, true);
// null = unknown (pending, or the status could not be fetched), otherwise whether any map is
// installed. Only a real status may claim that no maps are installed.
const [hasMaps, setHasMaps] = useState(null);
useEffect(() => {
let cancelled = false;
(async () => {
const status = await getMapSearchStatus();
if (!cancelled) {
const installed = (status.indexed || []).length + (status.missing || []).length;
setHasMaps(installed > 0);
try {
const status = await getMapSearchStatus();
if (!cancelled && status) {
const installed = (status.indexed || []).length + (status.missing || []).length;
setHasMaps(installed > 0);
}
} catch (e) {
console.error(e);
}
})();
return () => {
@ -106,12 +114,22 @@ export function MapSearchView() {
</Panel>;
}
if (!searchStr) {
return <Panel><Header as="h4">Enter a search query to find map locations.</Header></Panel>;
}
if (searchStr.length < 2) {
return <Panel><Header as="h4">Enter at least two characters to find map locations.</Header></Panel>;
}
if (loading) {
return <Panel><Loading/></Panel>;
}
if (!searchStr) {
return <Panel><Header as="h4">Enter a search query to find map locations.</Header></Panel>;
// Settled with no data: the request failed (the api helper returns undefined on a non-OK
// response, and useLatestRequest leaves null when it throws).
if (data == null) {
return <Panel><ErrorMessage>Could not search the map.</ErrorMessage></Panel>;
}
if (results.length === 0) {

View file

@ -0,0 +1,61 @@
import React from 'react';
import {render, screen} from '../test-utils';
import {MapSearchView} from './MapSearchView';
import {getMapSearchStatus, searchMap} from '../api';
// The map search tab says "No map locations found" only when the server searched and found none,
// and "No maps are installed" only when the server said so. A failed request is an error.
jest.mock('../api', () => ({
...jest.requireActual('../api'),
getMapSearchStatus: jest.fn(),
searchMap: jest.fn(),
}));
const NOT_FOUND = /No map locations found/;
const NO_MAPS = /No maps are installed/;
const FAILED = /Could not search the map/;
const renderAt = (q) => render(<MapSearchView/>, {query: {searchParams: new URLSearchParams({q})}});
beforeEach(() => {
getMapSearchStatus.mockReset();
searchMap.mockReset();
getMapSearchStatus.mockResolvedValue({indexed: ['utah'], missing: []});
});
test('found nothing: says so', async () => {
searchMap.mockResolvedValue({results: [], total: 0});
renderAt('camp');
expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument();
});
test('a failed search shows an error, not "No map locations found"', async () => {
searchMap.mockResolvedValue(undefined);
renderAt('camp');
expect(await screen.findByText(FAILED)).toBeInTheDocument();
expect(screen.queryByText(NOT_FOUND)).not.toBeInTheDocument();
});
test('a failed status check does not claim no maps are installed', async () => {
getMapSearchStatus.mockResolvedValue(undefined);
searchMap.mockResolvedValue({results: [], total: 0});
renderAt('camp');
expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument();
expect(screen.queryByText(NO_MAPS)).not.toBeInTheDocument();
});
test('a thrown status check does not claim no maps are installed', async () => {
getMapSearchStatus.mockRejectedValue(new Error('down'));
searchMap.mockResolvedValue({results: [], total: 0});
renderAt('camp');
expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument();
expect(screen.queryByText(NO_MAPS)).not.toBeInTheDocument();
});
test('the server saying no maps are installed is shown', async () => {
getMapSearchStatus.mockResolvedValue({indexed: [], missing: []});
searchMap.mockResolvedValue({results: [], total: 0});
renderAt('camp');
expect(await screen.findByText(NO_MAPS)).toBeInTheDocument();
});

View file

@ -421,10 +421,13 @@ function MapSearch({map}) {
lon = center.lng;
}
// A failed search (undefined) shows no dropdown, same as finding nothing: this is an
// as-you-type suggestion list, not a results page.
searchMap(q, 8, 0, lat, lon).then(data => {
setResults(data.results || []);
setShowResults((data.results || []).length > 0);
});
const found = data?.results || [];
setResults(found);
setShowResults(found.length > 0);
}).catch(e => console.error(e));
}, [map]);
const handleInput = useCallback((e) => {

View file

@ -0,0 +1,43 @@
import React from 'react';
import {render, screen} from '../../test-utils';
import {BackupsModal} from './Configs';
import {getConfigBackups} from '../../api';
// The config backups modal says "No backups available" only once the server confirmed there are
// none; a failed fetch is an error, and a thrown one must not leave the loader up forever.
jest.mock('../../api', () => ({
...jest.requireActual('../../api'),
getConfigBackups: jest.fn(),
postConfigBackupPreview: jest.fn(() => new Promise(() => {
})),
}));
const NONE = /No backups available/;
const FAILED = /Could not fetch backups/;
const renderModal = () => render(
<BackupsModal open fileName='wrolpi.yaml' onClose={jest.fn()} fetchConfigs={jest.fn()}/>,
);
beforeEach(() => getConfigBackups.mockReset());
test('empty: says there are no backups', async () => {
getConfigBackups.mockResolvedValue({dates: []});
renderModal();
expect(await screen.findByText(NONE)).toBeInTheDocument();
});
test('failed: shows an error, not "No backups available"', async () => {
getConfigBackups.mockResolvedValue(undefined);
renderModal();
expect(await screen.findByText(FAILED)).toBeInTheDocument();
expect(screen.queryByText(NONE)).not.toBeInTheDocument();
});
test('thrown: shows an error, not a loader forever', async () => {
getConfigBackups.mockRejectedValue(new Error('down'));
renderModal();
expect(await screen.findByText(FAILED)).toBeInTheDocument();
expect(screen.queryByText(/Loading backups/)).not.toBeInTheDocument();
});

View file

@ -1,4 +1,4 @@
import {APIButton} from "../Common";
import {APIButton, ErrorMessage} from "../Common";
import {Button, Header, Icon, IconButton, Loader, Loading, Modal, Placeholder, Table, Tooltip} from "../ui";
import React, {useEffect, useState} from "react";
import {getConfigText, getConfigBackups, postConfigBackupImport, postConfigBackupPreview} from "../../api";
@ -117,9 +117,11 @@ function BackupDateRow({date, previews, previewsLoaded, onSelect}) {
</Table.Row>;
}
function BackupsModal({open, onClose, fileName, fetchConfigs}) {
export function BackupsModal({open, onClose, fileName, fetchConfigs}) {
const [dates, setDates] = useState([]);
const [datesLoading, setDatesLoading] = useState(true);
// The backups could not be fetched; distinct from the server confirming there are none.
const [datesFailed, setDatesFailed] = useState(false);
const [previews, setPreviews] = useState({});
const [previewsLoaded, setPreviewsLoaded] = useState({});
const [selectedAction, setSelectedAction] = useState(null); // {date, mode, preview}
@ -130,17 +132,30 @@ function BackupsModal({open, onClose, fileName, fetchConfigs}) {
if (!open) return;
let cancelled = false;
setDatesLoading(true);
setDatesFailed(false);
setDates([]);
setPreviews({});
setPreviewsLoaded({});
setSelectedAction(null);
setApplying(false);
getConfigBackups(fileName).then(result => {
if (cancelled) return;
setDates(result?.dates || []);
setDatesLoading(false);
});
getConfigBackups(fileName)
.then(result => {
if (cancelled) return;
// The api helper returns undefined on a non-OK response.
if (result) {
setDates(result.dates || []);
} else {
setDatesFailed(true);
}
})
.catch(e => {
console.error(e);
if (!cancelled) setDatesFailed(true);
})
.finally(() => {
if (!cancelled) setDatesLoading(false);
});
return () => { cancelled = true; };
}, [open, fileName]);
@ -253,6 +268,8 @@ function BackupsModal({open, onClose, fileName, fetchConfigs}) {
<Modal.Content>
{datesLoading
? <Loading>Loading backups...</Loading>
: datesFailed
? <ErrorMessage>Could not fetch backups for this config.</ErrorMessage>
: dates.length === 0
? <p>No backups available for this config.</p>
: <div style={{maxHeight: '400px', overflowY: 'auto'}}>

View file

@ -25,7 +25,7 @@ function ItemNameList({items}) {
* `onChanged` refreshes the page's inventory state after a successful action.
*/
export function InventoryImportModal({open, onClose, slug, name, onChanged}) {
const [dates, setDates] = useState(null); // null while loading
const [dates, setDates] = useState(null); // null while loading, undefined if the fetch failed
const [reimporting, setReimporting] = useState(false);
const [selected, setSelected] = useState(null); // {date, mode, preview} confirmation view
const [previewing, setPreviewing] = useState(null); // `${date}:${mode}` currently loading a preview
@ -41,11 +41,19 @@ export function InventoryImportModal({open, onClose, slug, name, onChanged}) {
setReimporting(false);
setApplying(false);
setPreviewing(null);
getInventoryBackups(slug).then(result => {
if (!cancelled) {
setDates(result || []);
}
});
getInventoryBackups(slug)
.then(result => {
if (!cancelled) {
// The api helper returns undefined on a non-OK response.
setDates(result === undefined ? undefined : result || []);
}
})
.catch(e => {
console.error(e);
if (!cancelled) {
setDates(undefined);
}
});
return () => {
cancelled = true;
};
@ -153,6 +161,8 @@ export function InventoryImportModal({open, onClose, slug, name, onChanged}) {
</p>
{dates === null
? <Loading>Loading backups…</Loading>
: dates === undefined
? <p style={{color: 'var(--danger)'}}>Could not fetch backups for this inventory.</p>
: dates.length === 0
? <p>No backups yet. A backup is saved automatically whenever this inventory changes.</p>
: <div style={{maxHeight: '320px', overflowY: 'auto'}}>

View file

@ -65,4 +65,25 @@ describe('InventoryImportModal', () => {
renderModal();
await waitFor(() => expect(getInventoryBackups).toHaveBeenCalledWith('food-storage'));
});
// "No backups yet" may only appear once the server confirmed there are none.
test('says there are no backups when the server returned none', async () => {
getInventoryBackups.mockResolvedValue([]);
renderModal();
expect(await screen.findByText(/No backups yet/)).toBeInTheDocument();
});
test('a failed backups fetch shows an error, not "No backups yet"', async () => {
getInventoryBackups.mockResolvedValue(undefined);
renderModal();
expect(await screen.findByText(/Could not fetch backups/)).toBeInTheDocument();
expect(screen.queryByText(/No backups yet/)).not.toBeInTheDocument();
});
test('a thrown backups fetch shows an error, not a loader forever', async () => {
getInventoryBackups.mockRejectedValue(new Error('down'));
renderModal();
expect(await screen.findByText(/Could not fetch backups/)).toBeInTheDocument();
expect(screen.queryByText(/Loading backups/)).not.toBeInTheDocument();
});
});