Merge pull request #361 from appdevforall/fix/ADFA-4954-kolibri-dashboard-defects

ADFA-4954 fix(kolibri): four defects the device found, 1.2.1

Author: luisguzman-adfa

static/dashboard/CHANGELOG.md

@@ -4,6 +4,7 @@ One line per version, newest first. Every REST-facing change bumps the version i
 (the app surfaces it via `/system/dashboard/update-check` and the "Update available" pill), so this
 file is the human record of what each bump enables. Keep entries short: `version - change (TICKET)`.
 
+- **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)
 - **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)
 - **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.
 - **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/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "dashboard-console",
-  "version": "1.2.0",
+  "version": "1.2.1",
   "description": "",
   "main": "index.js",
   "scripts": {

static/dashboard/package.json

@@ -14,6 +14,7 @@ import { parseBox, parseEstimate } from './sockets/maps.socket';
 import {
     preflight, listInstalledChannels, browseRemoteChannels, resolveIdentifier,
     browseChannelTree, estimateSelection, deleteChannel, getKolibriTask, verifyCredentials,
+    ChannelNotInstalledError,
 } from './sockets/kolibri.query';
 import { checkReadiness, KolibriAuthError, KolibriApiError, login as kolibriLogin } from './sockets/kolibri.session';
 import {
@@ -359,31 +360,43 @@ apiRouter.get('/system/dashboard/update-check', async (_req: Request, res: Respo
     }
 });
 
-// --- Kolibri: readiness, catálogo y selección (ADFA-4949) -------------------------
-// Consultas directas (no-job). La descarga en sí es un job durable
-// (POST /kolibri/download), que sale gratis al añadir 'kolibri' a VALID_TYPES.
+// --- Kolibri: readiness, catalogue and selection (ADFA-4949) ----------------------
+// Direct (non-job) queries. The download itself is a durable job
+// (POST /kolibri/download), which comes free from adding 'kolibri' to VALID_TYPES.
 //
-// Nota de puertos y prefijos: nginx expone Kolibri en /kolibri/ y además le enruta
-// /api/, /content/, /device/ y /learn/ en el :8085 compartido. Nosotros hablamos
-// con Kolibri directamente en 127.0.0.1:8009 desde dentro, así que no hay colisión.
+// A note on ports and prefixes: nginx exposes Kolibri at /kolibri/ and also routes
+// /api/, /content/, /device/ and /learn/ to it on the shared :8085. We talk to
+// Kolibri directly on 127.0.0.1:8009 from inside, so there is no collision.
 
-/** Traduce un KolibriAuthError al status HTTP correcto. */
+/** Translates a Kolibri failure into the right HTTP status. */
 function authStatus(e: unknown): number {
     if (e instanceof KolibriAuthError) {
         switch (e.reason) {
-            case 'unreachable': return 503;   // el servicio no está listo
+            case 'unreachable': return 503;   // the service is not ready
             case 'credentials': return 401;
             case 'permission': return 403;
             default: return 502;
         }
     }
-    // Kolibri respondió, pero con error: propagamos su semántica en lugar de un 500.
+    // Kolibri answered, but with an error. 502 in every case that is not an auth
+    // one: whatever Kolibri's status was, from here it is an upstream we could not
+    // get a usable answer from, and echoing its 4xx would blame the app's caller
+    // for a request the app itself composed. (This used to read
+    // `e.status >= 500 ? 502 : 502` — both arms the same, so the distinction it
+    // implied never existed.)
     if (e instanceof KolibriApiError) {
         if (e.status === 401 || e.status === 403) return e.status;
-        return e.status >= 500 ? 502 : 502;   // upstream en mal estado
+        return 502;
     }
-    // fetch aborta por timeout (AbortError) o no conecta (TypeError). Eso es
-    // "Kolibri no está listo", no "nuestro servidor se rompió": 503, no 500.
+    // The request was well formed but a precondition is not met — the channel is
+    // not on the device. That is the caller's state, not a server fault, so it
+    // must not be a 500: the readable message would arrive labelled as an
+    // internal error, which is as misleading as the bare 500 it replaced.
+    if (e instanceof ChannelNotInstalledError) {
+        return 409;
+    }
+    // fetch aborts on timeout (AbortError) or fails to connect (TypeError). That is
+    // "Kolibri is not ready", not "our server broke": 503, not 500.
     if (e instanceof Error && (e.name === 'AbortError' || e.name === 'TimeoutError'
         || e instanceof TypeError)) {
         return 503;
@@ -392,19 +405,19 @@ function authStatus(e: unknown): number {
 }
 
 // El "gate" de arranque: mientras ready sea false, no tiene sentido lanzar jobs.
-// Nunca lanza: siempre devuelve un diagnóstico con los bloqueadores en claro.
+// Never throws: it always returns a diagnosis with the blockers spelled out.
 apiRouter.get('/kolibri/ready', async (_req: Request, res: Response): Promise<void> => {
     try {
         const readiness = await checkReadiness();
-        // 200 siempre: es un endpoint de estado, no una operación. El cliente mira
+        // Always 200: this is a status endpoint, not an operation. The client reads
         // el campo `ready`, no el status.
         res.json(readiness);
     } catch (e: any) {
         res.status(500).json({ ready: false, blockers: [e?.message || 'readiness failed'] });
     }
 });
 
-// Diagnóstico ampliado: readiness + estado local (canales, bytes en disco, rutas).
+// Extended diagnosis: readiness plus local state (channels, bytes on disk, paths).
 apiRouter.get('/kolibri/preflight', async (_req: Request, res: Response): Promise<void> => {
     try {
         res.json(await preflight());
@@ -413,8 +426,8 @@ apiRouter.get('/kolibri/preflight', async (_req: Request, res: Response): Promis
     }
 });
 
-// Qué hay ya en el dispositivo. Lectura local en readonly: no necesita ni sesión
-// ni que Kolibri esté arriba.
+// What is already on the device. A local readonly read: it needs neither a session
+// nor Kolibri to be up.
 apiRouter.get('/kolibri/channels', (_req: Request, res: Response): void => {
     try {
         res.json(listInstalledChannels());
@@ -423,7 +436,7 @@ apiRouter.get('/kolibri/channels', (_req: Request, res: Response): void => {
     }
 });
 
-// Catálogo remoto para el selector del wizard. ?keyword= y ?language= opcionales.
+// Remote catalogue for the wizard's picker. ?keyword= and ?language= are optional.
 apiRouter.get('/kolibri/catalog', async (req: Request, res: Response): Promise<void> => {
     try {
         res.json(await browseRemoteChannels({
@@ -446,10 +459,10 @@ apiRouter.get('/kolibri/resolve/:identifier', async (req: Request, res: Response
     }
 });
 
-// Un nivel del árbol del canal, para elegir subárboles.
+// One level of the channel tree, for choosing subtrees.
 // PRECONDICIÓN: los metadatos del canal deben estar ya en la base local, o sea que
 // hay que haber importado el canal antes. Es el mismo flujo que la UI de Kolibri:
-// primero los metadatos (MB), luego la selección del contenido (GB).
+// metadata first (MB), then the content selection (GB).
 apiRouter.get('/kolibri/tree/:channelId', async (req: Request, res: Response): Promise<void> => {
     try {
         const nodeId = req.query.nodeId ? String(req.query.nodeId) : undefined;
@@ -459,7 +472,7 @@ apiRouter.get('/kolibri/tree/:channelId', async (req: Request, res: Response): P
     }
 });
 
-// Tamaño exacto de una selección + espacio libre, para el paso de consentimiento.
+// What a selection still has to transfer, plus free space, for the consent step.
 // Body: { channelId, nodeIds?, excludeNodeIds? }
 apiRouter.post('/kolibri/estimate', async (req: Request, res: Response): Promise<void> => {
     const body = req.body as { channelId?: unknown; nodeIds?: unknown; excludeNodeIds?: unknown };
@@ -477,7 +490,7 @@ apiRouter.post('/kolibri/estimate', async (req: Request, res: Response): Promise
 });
 
 // Borrar un canal. Devuelve el id del job de Kolibri, no del motor local: es una
-// operación corta y no merece un job durable.
+// a short operation and does not warrant a durable job.
 apiRouter.post('/kolibri/delete', async (req: Request, res: Response): Promise<void> => {
     const body = req.body as { channelId?: unknown; channelName?: unknown };
     const channelId = String(body?.channelId ?? '').trim();
@@ -493,7 +506,7 @@ apiRouter.post('/kolibri/delete', async (req: Request, res: Response): Promise<v
 });
 
 // Estado de una tarea lanzada directamente en Kolibri (hoy solo el borrado). Sin
-// esto, /kolibri/delete devolvía un id que ningún endpoint sabía consultar.
+// this, /kolibri/delete returned an id no endpoint knew how to query.
 apiRouter.get('/kolibri/task/:id', async (req: Request, res: Response): Promise<void> => {
     try {
         res.json(await getKolibriTask(String(req.params.id)));
@@ -504,8 +517,8 @@ apiRouter.get('/kolibri/task/:id', async (req: Request, res: Response): Promise<
 });
 
 // --- Credenciales de servicios (ADFA-4949) ---------------------------------------
-// Permite al webview actualizar usuario/contraseña sin recompilar. La contraseña
-// NUNCA se devuelve: el GET solo dice qué usuario hay y de dónde sale.
+// Lets the webview update the username/password without a rebuild. The password is
+// NEVER returned: the GET only says which user is set and where it came from.
 
 apiRouter.get('/credentials/:service', (req: Request, res: Response): void => {
     const service = String(req.params.service);
@@ -517,8 +530,8 @@ apiRouter.get('/credentials/:service', (req: Request, res: Response): void => {
     }
 });
 
-// Valida ANTES de persistir: así el error aparece en el formulario donde se cometió,
-// no media hora después en una descarga fallida.
+// Validates BEFORE persisting, so the error surfaces in the form where it was made
+// rather than half an hour later in a failed download.
 //   401 → credenciales rechazadas (no se guarda nada)
 //   403 → autentica pero no puede gestionar contenido (no se guarda nada)
 //   503 → Kolibri no responde; la capa de arranque debe resolverlo primero
@@ -579,7 +592,7 @@ apiRouter.post('/credentials/:service', async (req: Request, res: Response): Pro
     try {
         const check = await verifyCredentials(username, password);
         if (!check.canManageContent) {
-            // Distinguir esto de "contraseña mala" ahorra horas de diagnóstico: el
+            // Telling this apart from "wrong password" saves hours of diagnosis: the
             // usuario existe y la clave es correcta, pero no puede importar contenido.
             res.status(403).json({
                 error: `'${username}' autentica pero no tiene permiso para gestionar contenido`,
@@ -593,14 +606,14 @@ apiRouter.post('/credentials/:service', async (req: Request, res: Response): Pro
         const status = authStatus(e);
         res.status(status).json({
             error: status === 401
-                ? 'Kolibri rechazó estas credenciales'
+                ? 'Kolibri rejected these credentials'
                 : (e?.message || 'verification failed'),
             saved: false,
         });
     }
 });
 
-// Volver al valor de fábrica (o a la variable de entorno, si está puesta).
+// Back to the factory value (or to the environment variable, if one is set).
 apiRouter.delete('/credentials/:service', (req: Request, res: Response): void => {
     const service = String(req.params.service);
     if (!isServiceName(service)) { res.status(404).json({ error: 'unknown service' }); return; }

static/dashboard/routes.ts

@@ -23,8 +23,8 @@ import {
     KolibriSession, KolibriAuthError, KolibriApiError, STUDIO_URL,
 } from './kolibri.session';
 import {
-    buildTaskPayload, mapPercent, mapPhase, normalizeUuid, overallPercent,
-    sampleSpeed, PRE_RUN_STATES, TERMINAL_STATES,
+    buildTaskPayload, failureMessage, mapPercent, mapPhase, normalizeUuid,
+    overallPercent, sampleSpeed, PRE_RUN_STATES, TERMINAL_STATES,
 } from './kolibri.map';
 
 /** Positive integers from the environment, with a default. Adjustable in
@@ -368,7 +368,10 @@ async function pollKolibriJob(
             // Cleared on ALL terminal paths, not only on success: otherwise the
             // failed jobs pile up in Kolibri's queue.
             if (job.status === 'FAILED') {
-                const detail = job.exception || 'no detail';
+                // job.exception is only the class name — a bad channel id reports
+                // a bare "HTTPError". failureMessage digs the real line out of the
+                // traceback so the operator is told what actually went wrong.
+                const detail = failureMessage(job.exception, job.traceback);
                 ctx.log(`FAILURE in Kolibri: ${detail}`);
                 if (job.traceback) ctx.log(job.traceback.split('\n').slice(-6).join('\n'));
                 await clearInKolibri();

static/dashboard/sockets/kolibri.exec.ts

@@ -127,6 +127,91 @@ export function buildTaskPayload(item: TaskPayloadInput): Record<string, unknown
     return payload;
 }
 
+/**
+ * The most informative message available for a failed Kolibri task.
+ *
+ * Kolibri's `exception` field is the class name alone — a job that failed on a
+ * channel that does not exist reports `HTTPError`, which tells the reader
+ * nothing. The detail is in the last line of `traceback`, in Python's usual
+ * `module.ExceptionType: message` form, so that line is preferred when it says
+ * more than the class name already did. The module prefix is dropped: an
+ * operator does not need `requests.exceptions.` to understand the problem.
+ *
+ * Falls back to the class name, and then to a plain marker, so the caller always
+ * has something to put in front of a person.
+ */
+export function failureMessage(exception?: string | null,
+                               traceback?: string | null): string {
+    const cls = (exception || '').trim();
+
+    const lastLine = (traceback || '')
+        .split('\n')
+        .map((l) => l.trim())
+        .filter((l) => l.length > 0)
+        .pop() || '';
+
+    // "requests.exceptions.HTTPError: 404 Client Error" -> "HTTPError: 404 Client Error"
+    const match = /^([\w.]*?)(\w+(?:Error|Exception|Warning)):\s*(.+)$/.exec(lastLine);
+    if (match && match[3]) {
+        return `${match[2]}: ${match[3]}`;
+    }
+
+    // A traceback line that carries detail without matching the usual shape is
+    // still better than a bare class name, as long as it is not the class name.
+    if (lastLine && lastLine !== cls) {
+        return lastLine;
+    }
+    return cls || 'no detail';
+}
+
+/** A channel as the picker needs it, whichever endpoint it came from. */
+export interface RemoteChannel {
+    id: string;
+    name: string;
+    description: string;
+    version: number | null;
+    language: string | null;
+    totalResources: number | null;
+    publishedSize: number | null;
+}
+
+/**
+ * Reads a channel row into {@link RemoteChannel}.
+ *
+ * Lives here rather than next to the fetch because of the trap it encapsulates:
+ * **the same channel arrives under two different sets of field names.** Studio's
+ * own API says `total_resource_count` and `published_size`; Kolibri's
+ * `/api/content/remotechannel/` proxy renames them to `total_resources` and
+ * `total_file_size` on the way through. Reading only Studio's names against the
+ * proxy silently yields `null` for both — which is exactly what shipped, and
+ * what a device test caught. Both spellings are accepted, Kolibri's first,
+ * because that is the endpoint the box actually calls.
+ *
+ * Pure: no network here, so the renaming is covered by a unit test and cannot
+ * regress unnoticed.
+ */
+export function toRemoteChannel(row: Record<string, unknown>): RemoteChannel {
+    const num = (v: unknown): number | null =>
+        typeof v === 'number' ? v : (typeof v === 'string' && v.trim() !== '' ? Number(v) : null);
+    const first = (...vs: unknown[]): number | null => {
+        for (const v of vs) {
+            const n = num(v);
+            if (n !== null) return n;
+        }
+        return null;
+    };
+    return {
+        id: String(row.id ?? ''),
+        name: String(row.name ?? ''),
+        description: String(row.description ?? ''),
+        version: num(row.version),
+        language: typeof row.lang_code === 'string' ? row.lang_code
+            : (typeof row.language === 'string' ? row.language : null),
+        totalResources: first(row.total_resources, row.total_resource_count),
+        publishedSize: first(row.total_file_size, row.published_size),
+    };
+}
+
 /** Bytes per second from two samples of `transferred_file_size`.
  *  Returns null when there was no progress, so a 0 does not overwrite the previous
  *  reading. */