Skip to content

Commit 5de898b

Browse files
ADFA-4954 fix(kolibri): 409 for a channel that is not installed, and finish the English pass
The readable "channel is not on the device" message still arrived as HTTP 500, because authStatus falls through to it for any plain Error. That is the caller's state, not a server fault, so the status was as misleading as the bare 500 it replaced. ChannelNotInstalledError is now its own type and maps to 409. routes.ts still had 26 lines of Spanish comments — the whole Kolibri routes section, including authStatus. The 1.1.6 sweep missed it because I checked the seven files I had picked rather than the claim I was making; this time the whole directory was swept and no .ts file in the dashboard carries Spanish. CHANGELOG updated: the 409 changes what the REST surface returns. Verified: npm run typecheck clean, npm test 73/73.
1 parent b108c70 commit 5de898b

3 files changed

Lines changed: 55 additions & 32 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 refuses a channel that is not installed with a readable message 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. (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. (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: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { parseBox, parseEstimate } from './sockets/maps.socket';
1414
import {
1515
preflight, listInstalledChannels, browseRemoteChannels, resolveIdentifier,
1616
browseChannelTree, estimateSelection, deleteChannel, getKolibriTask, verifyCredentials,
17+
ChannelNotInstalledError,
1718
} from './sockets/kolibri.query';
1819
import { checkReadiness, KolibriAuthError, KolibriApiError, login as kolibriLogin } from './sockets/kolibri.session';
1920
import {
@@ -359,31 +360,38 @@ apiRouter.get('/system/dashboard/update-check', async (_req: Request, res: Respo
359360
}
360361
});
361362

362-
// --- Kolibri: readiness, catálogo y selección (ADFA-4949) -------------------------
363-
// Consultas directas (no-job). La descarga en sí es un job durable
364-
// (POST /kolibri/download), que sale gratis al añadir 'kolibri' a VALID_TYPES.
363+
// --- Kolibri: readiness, catalogue and selection (ADFA-4949) ----------------------
364+
// Direct (non-job) queries. The download itself is a durable job
365+
// (POST /kolibri/download), which comes free from adding 'kolibri' to VALID_TYPES.
365366
//
366-
// Nota de puertos y prefijos: nginx expone Kolibri en /kolibri/ y además le enruta
367-
// /api/, /content/, /device/ y /learn/ en el :8085 compartido. Nosotros hablamos
368-
// con Kolibri directamente en 127.0.0.1:8009 desde dentro, así que no hay colisión.
367+
// A note on ports and prefixes: nginx exposes Kolibri at /kolibri/ and also routes
368+
// /api/, /content/, /device/ and /learn/ to it on the shared :8085. We talk to
369+
// Kolibri directly on 127.0.0.1:8009 from inside, so there is no collision.
369370

370-
/** Traduce un KolibriAuthError al status HTTP correcto. */
371+
/** Translates a Kolibri failure into the right HTTP status. */
371372
function authStatus(e: unknown): number {
372373
if (e instanceof KolibriAuthError) {
373374
switch (e.reason) {
374-
case 'unreachable': return 503; // el servicio no está listo
375+
case 'unreachable': return 503; // the service is not ready
375376
case 'credentials': return 401;
376377
case 'permission': return 403;
377378
default: return 502;
378379
}
379380
}
380-
// Kolibri respondió, pero con error: propagamos su semántica en lugar de un 500.
381+
// Kolibri answered, but with an error: pass its semantics through instead of a 500.
381382
if (e instanceof KolibriApiError) {
382383
if (e.status === 401 || e.status === 403) return e.status;
383-
return e.status >= 500 ? 502 : 502; // upstream en mal estado
384+
return e.status >= 500 ? 502 : 502; // upstream in a bad state
384385
}
385-
// fetch aborta por timeout (AbortError) o no conecta (TypeError). Eso es
386-
// "Kolibri no está listo", no "nuestro servidor se rompió": 503, no 500.
386+
// The request was well formed but a precondition is not met — the channel is
387+
// not on the device. That is the caller's state, not a server fault, so it
388+
// must not be a 500: the readable message would arrive labelled as an
389+
// internal error, which is as misleading as the bare 500 it replaced.
390+
if (e instanceof ChannelNotInstalledError) {
391+
return 409;
392+
}
393+
// fetch aborts on timeout (AbortError) or fails to connect (TypeError). That is
394+
// "Kolibri is not ready", not "our server broke": 503, not 500.
387395
if (e instanceof Error && (e.name === 'AbortError' || e.name === 'TimeoutError'
388396
|| e instanceof TypeError)) {
389397
return 503;
@@ -392,19 +400,19 @@ function authStatus(e: unknown): number {
392400
}
393401

394402
// El "gate" de arranque: mientras ready sea false, no tiene sentido lanzar jobs.
395-
// Nunca lanza: siempre devuelve un diagnóstico con los bloqueadores en claro.
403+
// Never throws: it always returns a diagnosis with the blockers spelled out.
396404
apiRouter.get('/kolibri/ready', async (_req: Request, res: Response): Promise<void> => {
397405
try {
398406
const readiness = await checkReadiness();
399-
// 200 siempre: es un endpoint de estado, no una operación. El cliente mira
407+
// Always 200: this is a status endpoint, not an operation. The client reads
400408
// el campo `ready`, no el status.
401409
res.json(readiness);
402410
} catch (e: any) {
403411
res.status(500).json({ ready: false, blockers: [e?.message || 'readiness failed'] });
404412
}
405413
});
406414

407-
// Diagnóstico ampliado: readiness + estado local (canales, bytes en disco, rutas).
415+
// Extended diagnosis: readiness plus local state (channels, bytes on disk, paths).
408416
apiRouter.get('/kolibri/preflight', async (_req: Request, res: Response): Promise<void> => {
409417
try {
410418
res.json(await preflight());
@@ -413,8 +421,8 @@ apiRouter.get('/kolibri/preflight', async (_req: Request, res: Response): Promis
413421
}
414422
});
415423

416-
// Qué hay ya en el dispositivo. Lectura local en readonly: no necesita ni sesión
417-
// ni que Kolibri esté arriba.
424+
// What is already on the device. A local readonly read: it needs neither a session
425+
// nor Kolibri to be up.
418426
apiRouter.get('/kolibri/channels', (_req: Request, res: Response): void => {
419427
try {
420428
res.json(listInstalledChannels());
@@ -423,7 +431,7 @@ apiRouter.get('/kolibri/channels', (_req: Request, res: Response): void => {
423431
}
424432
});
425433

426-
// Catálogo remoto para el selector del wizard. ?keyword= y ?language= opcionales.
434+
// Remote catalogue for the wizard's picker. ?keyword= and ?language= are optional.
427435
apiRouter.get('/kolibri/catalog', async (req: Request, res: Response): Promise<void> => {
428436
try {
429437
res.json(await browseRemoteChannels({
@@ -446,10 +454,10 @@ apiRouter.get('/kolibri/resolve/:identifier', async (req: Request, res: Response
446454
}
447455
});
448456

449-
// Un nivel del árbol del canal, para elegir subárboles.
457+
// One level of the channel tree, for choosing subtrees.
450458
// PRECONDICIÓN: los metadatos del canal deben estar ya en la base local, o sea que
451459
// hay que haber importado el canal antes. Es el mismo flujo que la UI de Kolibri:
452-
// primero los metadatos (MB), luego la selección del contenido (GB).
460+
// metadata first (MB), then the content selection (GB).
453461
apiRouter.get('/kolibri/tree/:channelId', async (req: Request, res: Response): Promise<void> => {
454462
try {
455463
const nodeId = req.query.nodeId ? String(req.query.nodeId) : undefined;
@@ -459,7 +467,7 @@ apiRouter.get('/kolibri/tree/:channelId', async (req: Request, res: Response): P
459467
}
460468
});
461469

462-
// Tamaño exacto de una selección + espacio libre, para el paso de consentimiento.
470+
// What a selection still has to transfer, plus free space, for the consent step.
463471
// Body: { channelId, nodeIds?, excludeNodeIds? }
464472
apiRouter.post('/kolibri/estimate', async (req: Request, res: Response): Promise<void> => {
465473
const body = req.body as { channelId?: unknown; nodeIds?: unknown; excludeNodeIds?: unknown };
@@ -477,7 +485,7 @@ apiRouter.post('/kolibri/estimate', async (req: Request, res: Response): Promise
477485
});
478486

479487
// Borrar un canal. Devuelve el id del job de Kolibri, no del motor local: es una
480-
// operación corta y no merece un job durable.
488+
// a short operation and does not warrant a durable job.
481489
apiRouter.post('/kolibri/delete', async (req: Request, res: Response): Promise<void> => {
482490
const body = req.body as { channelId?: unknown; channelName?: unknown };
483491
const channelId = String(body?.channelId ?? '').trim();
@@ -493,7 +501,7 @@ apiRouter.post('/kolibri/delete', async (req: Request, res: Response): Promise<v
493501
});
494502

495503
// Estado de una tarea lanzada directamente en Kolibri (hoy solo el borrado). Sin
496-
// esto, /kolibri/delete devolvía un id que ningún endpoint sabía consultar.
504+
// this, /kolibri/delete returned an id no endpoint knew how to query.
497505
apiRouter.get('/kolibri/task/:id', async (req: Request, res: Response): Promise<void> => {
498506
try {
499507
res.json(await getKolibriTask(String(req.params.id)));
@@ -504,8 +512,8 @@ apiRouter.get('/kolibri/task/:id', async (req: Request, res: Response): Promise<
504512
});
505513

506514
// --- Credenciales de servicios (ADFA-4949) ---------------------------------------
507-
// Permite al webview actualizar usuario/contraseña sin recompilar. La contraseña
508-
// NUNCA se devuelve: el GET solo dice qué usuario hay y de dónde sale.
515+
// Lets the webview update the username/password without a rebuild. The password is
516+
// NEVER returned: the GET only says which user is set and where it came from.
509517

510518
apiRouter.get('/credentials/:service', (req: Request, res: Response): void => {
511519
const service = String(req.params.service);
@@ -517,8 +525,8 @@ apiRouter.get('/credentials/:service', (req: Request, res: Response): void => {
517525
}
518526
});
519527

520-
// Valida ANTES de persistir: así el error aparece en el formulario donde se cometió,
521-
// no media hora después en una descarga fallida.
528+
// Validates BEFORE persisting, so the error surfaces in the form where it was made
529+
// rather than half an hour later in a failed download.
522530
// 401 → credenciales rechazadas (no se guarda nada)
523531
// 403 → autentica pero no puede gestionar contenido (no se guarda nada)
524532
// 503 → Kolibri no responde; la capa de arranque debe resolverlo primero
@@ -579,7 +587,7 @@ apiRouter.post('/credentials/:service', async (req: Request, res: Response): Pro
579587
try {
580588
const check = await verifyCredentials(username, password);
581589
if (!check.canManageContent) {
582-
// Distinguir esto de "contraseña mala" ahorra horas de diagnóstico: el
590+
// Telling this apart from "wrong password" saves hours of diagnosis: the
583591
// usuario existe y la clave es correcta, pero no puede importar contenido.
584592
res.status(403).json({
585593
error: `'${username}' autentica pero no tiene permiso para gestionar contenido`,
@@ -593,14 +601,14 @@ apiRouter.post('/credentials/:service', async (req: Request, res: Response): Pro
593601
const status = authStatus(e);
594602
res.status(status).json({
595603
error: status === 401
596-
? 'Kolibri rechazó estas credenciales'
604+
? 'Kolibri rejected these credentials'
597605
: (e?.message || 'verification failed'),
598606
saved: false,
599607
});
600608
}
601609
});
602610

603-
// Volver al valor de fábrica (o a la variable de entorno, si está puesta).
611+
// Back to the factory value (or to the environment variable, if one is set).
604612
apiRouter.delete('/credentials/:service', (req: Request, res: Response): void => {
605613
const service = String(req.params.service);
606614
if (!isServiceName(service)) { res.status(404).json({ error: 'unknown service' }); return; }

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,21 @@ export interface SelectionSize {
229229
fitsOnDevice: boolean | null;
230230
}
231231

232+
/**
233+
* The request was well formed but the channel is not on the device yet.
234+
*
235+
* A distinct type so the route can answer 409 rather than 500: the caller asked
236+
* for something that needs a precondition it has not met, which is not the
237+
* server breaking. Without this the readable message still arrived wrapped in an
238+
* "internal error", which is as misleading as the bare 500 it replaced.
239+
*/
240+
export class ChannelNotInstalledError extends Error {
241+
constructor(message: string) {
242+
super(message);
243+
this.name = 'ChannelNotInstalledError';
244+
}
245+
}
246+
232247
/**
233248
* What a selection still has to transfer, plus the free space.
234249
*
@@ -253,7 +268,7 @@ export async function estimateSelection(
253268
channelId: string, nodeIds?: string[], excludeNodeIds?: string[],
254269
): Promise<SelectionSize> {
255270
if (!isChannelInstalled(channelId)) {
256-
throw new Error(
271+
throw new ChannelNotInstalledError(
257272
`channel ${channelId} is not on the device: its remaining size can only be `
258273
+ 'measured once its metadata has been imported');
259274
}

0 commit comments

Comments
 (0)