Skip to content

Commit f285b2d

Browse files
ADFA-4954 fix(kolibri): review nits on the dashboard defects
authStatus had `e.status >= 500 ? 502 : 502` — both arms the same, so the distinction it implied never existed. Settled as 502 for everything that is not 401/403, with the reason written down: echoing Kolibri's 4xx would blame the app's caller for a request the app composed itself. The installed check now answers present / absent / unknown, and only absent refuses. A database that exists but cannot be read — SQLITE_BUSY while Kolibri writes during an import — used to be reported as "not installed", telling the user to import a channel they may already have. Unknown lets the request through; Kolibri is the authority. RemoteChannel is re-exported with `export type`, and imported with `import type`: it is an interface with no runtime value, and a plain re-export stops compiling under isolatedModules. CHANGELOG: the English pass is REST-facing too — POST /credentials/kolibri returns a different string on 401. Verified: npm run typecheck clean, also clean with --isolatedModules, npm test 73/73.
1 parent 5de898b commit f285b2d

3 files changed

Lines changed: 30 additions & 13 deletions

File tree

‎static/dashboard/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ One line per version, newest first. Every REST-facing change bumps the version i
44
(the app surfaces it via `/system/dashboard/update-check` and the "Update available" pill), so this
55
file is the human record of what each bump enables. Keep entries short: `version - change (TICKET)`.
66

7-
- **1.2.1** - Four Kolibri defects found by testing against a real device. `/kolibri/estimate` now reports free space (the call was missing the mandatory `?path=Content` and had been failing silently since it was written) and answers **409** with a readable message for a channel that is not installed, instead of letting Kolibri return a bare 500; `/kolibri/catalog` reports real sizes and resource counts (it was reading Studio's field names on an endpoint that uses Kolibri's); a failed import now carries the cause instead of just the exception class name. Also finishes the English pass: `routes.ts` was the last file the 1.1.6 sweep missed. (ADFA-4954)
7+
- **1.2.1** - Four Kolibri defects found by testing against a real device. `/kolibri/estimate` now reports free space (the call was missing the mandatory `?path=Content` and had been failing silently since it was written) and answers **409** with a readable message for a channel that is not installed, instead of letting Kolibri return a bare 500; `/kolibri/catalog` reports real sizes and resource counts (it was reading Studio's field names on an endpoint that uses Kolibri's); a failed import now carries the cause instead of just the exception class name. Also finishes the English pass: `routes.ts` was the last file the 1.1.6 sweep missed — REST-facing too, since one response string changes (`POST /credentials/kolibri` on 401). (ADFA-4954)
88
- **1.2.0** - Self-update cutover baseline: no endpoint change, but from this version the app updates the dash-node core **live over REST** (`POST /system/dashboard/rebuild`, the blue-green rebuild from ADFA-5011) instead of a proot rebuild. Installs on < 1.2.0 still use the proot path to reach 1.2.0; 1.2.0+ update to 1.2.1+ via REST. (ADFA-5051)
99
- **1.1.6** - Kolibri modules now in English. REST-facing because the diagnostic text changed: the `blockers[]` strings from `GET /kolibri/ready` and the `error` text on Kolibri jobs. Shapes, status codes and field names are unchanged.
1010
- **1.1.5** - Credentials: `POST /credentials/calibre` now validates against live Calibre-Web (401 on reject, save-unverified when the service is down); `GET /credentials/:service` returns the default password only while still at the factory default, for full form prefill. (ADFA-5044)

‎static/dashboard/routes.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,10 +378,15 @@ function authStatus(e: unknown): number {
378378
default: return 502;
379379
}
380380
}
381-
// Kolibri answered, but with an error: pass its semantics through instead of a 500.
381+
// Kolibri answered, but with an error. 502 in every case that is not an auth
382+
// one: whatever Kolibri's status was, from here it is an upstream we could not
383+
// get a usable answer from, and echoing its 4xx would blame the app's caller
384+
// for a request the app itself composed. (This used to read
385+
// `e.status >= 500 ? 502 : 502` — both arms the same, so the distinction it
386+
// implied never existed.)
382387
if (e instanceof KolibriApiError) {
383388
if (e.status === 401 || e.status === 403) return e.status;
384-
return e.status >= 500 ? 502 : 502; // upstream in a bad state
389+
return 502;
385390
}
386391
// The request was well formed but a precondition is not met — the channel is
387392
// not on the device. That is the caller's state, not a server fault, so it

‎static/dashboard/sockets/kolibri.query.ts‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ import {
2424
KolibriSession, STUDIO_URL,
2525
} from './kolibri.session';
2626
import {
27-
TASK_DELETE_CHANNEL, TERMINAL_STATES, mapPercent, toRemoteChannel, RemoteChannel,
27+
TASK_DELETE_CHANNEL, TERMINAL_STATES, mapPercent, toRemoteChannel,
2828
} from './kolibri.map';
29+
import type { RemoteChannel } from './kolibri.map';
2930

3031
const KOLIBRI_HOME = process.env.KOLIBRI_HOME || '/library/kolibri';
3132
const MAIN_DB = path.join(KOLIBRI_HOME, 'db.sqlite3');
@@ -120,8 +121,11 @@ export function contentBytesOnDisk(): number {
120121

121122
// RemoteChannel and toRemoteChannel now live in kolibri.map.ts: the mapper is
122123
// pure, and the two competing sets of field names it reconciles deserve a unit
123-
// test rather than a comment. Re-exported so the routes keep their import path.
124-
export { RemoteChannel };
124+
// test rather than a comment. Re-exported so the routes keep their import path —
125+
// as `export type`, because RemoteChannel is an interface with no runtime value
126+
// and a plain re-export stops compiling the day isolatedModules or
127+
// verbatimModuleSyntax is switched on.
128+
export type { RemoteChannel };
125129

126130
/**
127131
* Remote catalogue for the wizard's picker, through the device's own proxy.
@@ -267,7 +271,9 @@ export class ChannelNotInstalledError extends Error {
267271
export async function estimateSelection(
268272
channelId: string, nodeIds?: string[], excludeNodeIds?: string[],
269273
): Promise<SelectionSize> {
270-
if (!isChannelInstalled(channelId)) {
274+
// ABSENT is a fact about the request; UNKNOWN is a fact about us. Only the
275+
// first one is the caller's problem, so only the first one refuses.
276+
if (installedState(channelId) === 'absent') {
271277
throw new ChannelNotInstalledError(
272278
`channel ${channelId} is not on the device: its remaining size can only be `
273279
+ 'measured once its metadata has been imported');
@@ -310,22 +316,28 @@ export async function estimateSelection(
310316
* A single-row lookup, not the full inventory query: this runs before every
311317
* estimate and only needs a yes or no.
312318
*/
313-
function isChannelInstalled(channelId: string): boolean {
314-
if (!fs.existsSync(MAIN_DB)) return false;
319+
function installedState(channelId: string): 'present' | 'absent' | 'unknown' {
320+
// No database at all means nothing has ever been imported, which is a real
321+
// answer rather than a failure to look.
322+
if (!fs.existsSync(MAIN_DB)) return 'absent';
315323
try {
316324
const db = new Database(MAIN_DB, { readonly: true });
317325
try {
318326
const row = db.prepare(
319327
'SELECT 1 FROM content_channelmetadata WHERE id = ? LIMIT 1',
320328
).get(channelId);
321-
return row !== undefined;
329+
return row === undefined ? 'absent' : 'present';
322330
} finally {
323331
db.close();
324332
}
325333
} catch {
326-
// Unreadable database: treat as not installed, which produces the same
327-
// actionable message rather than an opaque 500 from Kolibri.
328-
return false;
334+
// The database exists but could not be read — SQLITE_BUSY while Kolibri
335+
// writes during an import is the realistic case. Reporting "not installed"
336+
// here would tell the user to import a channel they may already have. Say
337+
// we do not know, and let the request through: Kolibri is the authority,
338+
// and if the channel really is missing it answers its own 500, which is
339+
// where we started but only in the case we cannot rule out.
340+
return 'unknown';
329341
}
330342
}
331343

0 commit comments

Comments
 (0)