From 629f845eeffa373e4103636bb24302c41466d9e1 Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 24 Jul 2026 12:06:24 +0100 Subject: [PATCH 1/5] Handle train model failure with error dialog The progress dialog can only be dismissed by the stage leaving TrainingInProgress, so any unhandled throw in trainModel (e.g. from updateProject) would otherwise strand it at 100%. On failure, we fall through to TrainingError. --- src/store.ts | 100 ++++++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 44 deletions(-) diff --git a/src/store.ts b/src/store.ts index f81df032e..4f9c1a44c 100644 --- a/src/store.ts +++ b/src/store.ts @@ -1277,15 +1277,19 @@ const createMlStore = (logging: Logging) => { trainModelDialogStage: TrainModelDialogStage.Help, }); } else { - await trainModel(); - callback?.(); - // Push the trainModelDialogStage change to the back of the event queue so it happens - // after navigation changes. - setTimeout( - () => - set({ trainModelDialogStage: TrainModelDialogStage.Closed }), - 0 - ); + const success = await trainModel(); + // On failure, the stage is already TrainingError, so leave that + // dialog up rather than navigating away and closing it. + if (success) { + callback?.(); + // Push the trainModelDialogStage change to the back of the event queue so it happens + // after navigation changes. + setTimeout( + () => + set({ trainModelDialogStage: TrainModelDialogStage.Closed }), + 0 + ); + } } }, @@ -1303,44 +1307,52 @@ const createMlStore = (logging: Logging) => { trainModelDialogStage: TrainModelDialogStage.TrainingInProgress, trainModelProgress: 0, }); - const trainingResult = await trainModel( - actions, - dataWindow, - (trainModelProgress) => - set({ trainModelProgress }, false, "trainModelProgress") - ); - const model = trainingResult.error ? undefined : trainingResult.model; - const updatedProject = updateProject( - project, - projectEdited, - actions, - model, - dataWindow - ); - const timestamp = Date.now(); - set( - { + try { + const trainingResult = await trainModel( + actions, + dataWindow, + (trainModelProgress) => + set({ trainModelProgress }, false, "trainModelProgress") + ); + const model = trainingResult.error + ? undefined + : trainingResult.model; + const updatedProject = updateProject( + project, + projectEdited, + actions, model, - trainModelDialogStage: model - ? TrainModelDialogStage.TrainingInProgress - : TrainModelDialogStage.TrainingError, - timestamp, - ...updatedProject, - }, - false, - actionName - ); - await storageWriteWithErrHandling(() => - storage.updateMakeCodeProject( - id, + dataWindow + ); + const timestamp = Date.now(); + set( { - project: updatedProject.project, - projectEdited: updatedProject.projectEdited, + model, + trainModelDialogStage: model + ? TrainModelDialogStage.TrainingInProgress + : TrainModelDialogStage.TrainingError, + timestamp, + ...updatedProject, }, - timestamp - ) - ); - return !trainingResult.error; + false, + actionName + ); + await storageWriteWithErrHandling(() => + storage.updateMakeCodeProject( + id, + { + project: updatedProject.project, + projectEdited: updatedProject.projectEdited, + }, + timestamp + ) + ); + return !trainingResult.error; + } catch (e) { + logging.error("Model training failed", e); + set({ trainModelDialogStage: TrainModelDialogStage.TrainingError }); + return false; + } }, removeModel(): void { From 134253337a6d8898dfc62a89cce7a3cea178ab23 Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 24 Jul 2026 12:52:56 +0100 Subject: [PATCH 2/5] Add ml worker watchdog to ensure it is alive and well. Otherwise, timeout after 60s and raise error dialog. --- src/ml-worker-client.ts | 69 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/ml-worker-client.ts b/src/ml-worker-client.ts index b1498a855..cc5f06bb1 100644 --- a/src/ml-worker-client.ts +++ b/src/ml-worker-client.ts @@ -17,9 +17,23 @@ export interface WorkerTrainResult { machineCode: Uint8Array; } +/** + * If the worker sends no message at all for this long during a model + * operation — not even a training-progress tick — it's treated as hung + * (e.g. suspended or killed by the OS while the app is backgrounded) and the + * request fails so the UI can recover instead of waiting forever. + */ +const workerInactivityTimeoutMs = 60000; + interface PendingRequest { resolve(response: MlWorkerResponse | undefined): void; onProgress?: (value: number) => void; + /** + * Inactivity timeout, if this request is watchdogged. Reset on every + * message (including progress) and cleared when the request settles. + */ + timeoutMs?: number; + timer?: ReturnType; } export class MlWorkerClient { @@ -86,13 +100,43 @@ export class MlWorkerClient { return; } if (message.kind === "progress") { + // The worker is alive and making progress; restart the watchdog. + this.resetWatchdog(message.id); pending.onProgress?.(message.value); return; } + this.cancelWatchdog(message.id); this.pending.delete(message.id); pending.resolve(message); } + /** + * Start (or restart) the inactivity watchdog for a request that opted into + * one. Any message from the worker restarts the countdown; silence past the + * timeout fails the request via handleFatalError. + */ + private resetWatchdog(id: number) { + const pending = this.pending.get(id); + if (!pending?.timeoutMs) { + return; + } + if (pending.timer !== undefined) { + clearTimeout(pending.timer); + } + pending.timer = setTimeout( + () => this.handleFatalError(), + pending.timeoutMs + ); + } + + private cancelWatchdog(id: number) { + const pending = this.pending.get(id); + if (pending?.timer !== undefined) { + clearTimeout(pending.timer); + pending.timer = undefined; + } + } + /** * Called when the worker itself fails (e.g. its script or wasm binary * failed to load). Fail all pending requests and discard the worker so @@ -100,6 +144,11 @@ export class MlWorkerClient { */ private handleFatalError() { const pending = Array.from(this.pending.values()); + pending.forEach((p) => { + if (p.timer !== undefined) { + clearTimeout(p.timer); + } + }); this.pending.clear(); this.worker?.terminate(); this.worker = undefined; @@ -109,12 +158,14 @@ export class MlWorkerClient { private postRequest( createRequest: (id: number) => MlWorkerRequest, worker: Worker = this.ensureWorker(), - onProgress?: (value: number) => void + onProgress?: (value: number) => void, + timeoutMs?: number ): Promise { const id = this.nextRequestId++; return new Promise((resolve) => { - this.pending.set(id, { resolve, onProgress }); + this.pending.set(id, { resolve, onProgress, timeoutMs }); worker.postMessage(createRequest(id)); + this.resetWatchdog(id); }); } @@ -136,7 +187,8 @@ export class MlWorkerClient { const response = await this.postRequest( (id) => ({ kind: "train", id, features, labels, options }), this.ensureWorker(), - onProgress + onProgress, + workerInactivityTimeoutMs ); if (response?.kind !== "trainComplete") { return undefined; @@ -163,11 +215,12 @@ export class MlWorkerClient { ): Promise<{ machineCode: Uint8Array } | undefined> { this.modelOpInFlight = true; try { - const response = await this.postRequest((id) => ({ - kind: "loadModel", - id, - artifacts, - })); + const response = await this.postRequest( + (id) => ({ kind: "loadModel", id, artifacts }), + this.ensureWorker(), + undefined, + workerInactivityTimeoutMs + ); if (response?.kind !== "loadModelComplete") { return undefined; } From 2769cb26bd884bab90b20b6f049c0308a1668efa Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 24 Jul 2026 13:06:42 +0100 Subject: [PATCH 3/5] Add storage write timeout --- src/store.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/store.ts b/src/store.ts index 4f9c1a44c..a42386f96 100644 --- a/src/store.ts +++ b/src/store.ts @@ -2226,20 +2226,46 @@ const storageWithErrHandling = async ( return value; }; +const storageWriteTimeoutMs = 15000; + +/** + * A promise that rejects with {@link message} after {@link ms}, plus a cancel + * to clear its timer once the operation it guards has settled (preventing a + * late rejection). + */ +const rejectAfter = ( + ms: number, + message: string +): { promise: Promise; cancel: () => void } => { + let timer: ReturnType; + const promise = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(message)), ms); + }); + return { promise, cancel: () => clearTimeout(timer) }; +}; + /** * Like {@link storageWithErrHandling} but sets a storage error in the * store (triggering a toast) and swallows the error. Use for * fire-and-forget writes where the in-memory state has already been * optimistically updated. + * + * Bounded by {@link storageWriteTimeoutMs}. */ const storageWriteWithErrHandling = async ( callback: () => Promise, broadcastEvent: boolean | "settings" = true ) => { + const timeout = rejectAfter(storageWriteTimeoutMs, "Storage write timed out"); try { - await storageWithErrHandling(callback, broadcastEvent); + await Promise.race([ + storageWithErrHandling(callback, broadcastEvent), + timeout.promise, + ]); } catch (err) { setStorageError(err); + } finally { + timeout.cancel(); } }; From e79c6d894709583db9bde218c4f0bb0c694cfd4c Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 24 Jul 2026 13:10:22 +0100 Subject: [PATCH 4/5] Remove comment --- src/ml-worker-client.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ml-worker-client.ts b/src/ml-worker-client.ts index cc5f06bb1..575ac32e5 100644 --- a/src/ml-worker-client.ts +++ b/src/ml-worker-client.ts @@ -17,12 +17,6 @@ export interface WorkerTrainResult { machineCode: Uint8Array; } -/** - * If the worker sends no message at all for this long during a model - * operation — not even a training-progress tick — it's treated as hung - * (e.g. suspended or killed by the OS while the app is backgrounded) and the - * request fails so the UI can recover instead of waiting forever. - */ const workerInactivityTimeoutMs = 60000; interface PendingRequest { From 59acb0edfcc88072e3d231694e1685d3b2b8328b Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 24 Jul 2026 14:15:18 +0100 Subject: [PATCH 5/5] Add Sentry logging for when model training fails with reason as to failure cause (timeout, worker error, or request error) --- src/ml-worker-client.test.ts | 24 ++++++++----- src/ml-worker-client.ts | 68 ++++++++++++++++++++++++++---------- src/store.ts | 6 ++++ src/train-model.ts | 14 ++++---- 4 files changed, 77 insertions(+), 35 deletions(-) diff --git a/src/ml-worker-client.test.ts b/src/ml-worker-client.test.ts index d48cb2fa5..06bbd7833 100644 --- a/src/ml-worker-client.test.ts +++ b/src/ml-worker-client.test.ts @@ -42,17 +42,20 @@ test("trains a model, reporting progress and returning artifacts and machine cod const result = await client.train(features, labels, trainOptions, (v) => progress.push(v) ); - expect(result).toBeDefined(); - expect(result!.artifacts.modelTopology).toBeDefined(); - expect(result!.artifacts.weightData).toBeDefined(); - expect(result!.machineCode.length).toBeGreaterThan(0); + expect(result.success).toBe(true); + if (!result.success) { + throw new Error("expected training to succeed"); + } + expect(result.result.artifacts.modelTopology).toBeDefined(); + expect(result.result.artifacts.weightData).toBeDefined(); + expect(result.result.machineCode.length).toBeGreaterThan(0); expect(progress.length).toBeGreaterThan(0); expect(progress[progress.length - 1]).toBe(1); }); -test("train with no data resolves with undefined", async () => { +test("train with no data fails with a requestError reason", async () => { const result = await client.train([], [], trainOptions, () => {}); - expect(result).toBeUndefined(); + expect(result).toEqual({ success: false, reason: "requestError" }); }); test("predicts using the model kept live in the worker after training", async () => { @@ -72,19 +75,22 @@ test("predict returns undefined when no model is loaded", async () => { test("predict during training is dropped rather than queued", async () => { const trainPromise = client.train(features, labels, trainOptions, () => {}); expect(await client.predict(features[0])).toBeUndefined(); - expect(await trainPromise).toBeDefined(); + expect((await trainPromise).success).toBe(true); }); test("loadModel restores a trained model for prediction with identical machine code", async () => { const trained = await client.train(features, labels, trainOptions, () => {}); + if (!trained.success) { + throw new Error("expected training to succeed"); + } const original = await client.predict(features[0]); // Fresh client/worker as after a page reload with artifacts from storage. const reloadClient = new MlWorkerClient(); - const loaded = await reloadClient.loadModel(trained!.artifacts); + const loaded = await reloadClient.loadModel(trained.result.artifacts); expect(loaded).toBeDefined(); expect(Array.from(loaded!.machineCode)).toEqual( - Array.from(trained!.machineCode) + Array.from(trained.result.machineCode) ); const reloaded = await reloadClient.predict(features[0]); diff --git a/src/ml-worker-client.ts b/src/ml-worker-client.ts index 575ac32e5..ee3d057bd 100644 --- a/src/ml-worker-client.ts +++ b/src/ml-worker-client.ts @@ -17,10 +17,32 @@ export interface WorkerTrainResult { machineCode: Uint8Array; } +/** + * Why a model operation failed, for diagnostics: + * - `timeout`: the worker went silent past the inactivity watchdog. + * - `workerError`: the worker itself crashed (onerror — script/wasm failure). + * - `requestError`: the worker ran the request but reported an error (e.g. an + * exception during training, or empty training data). + */ +export type WorkerFailureReason = "timeout" | "workerError" | "requestError"; + +export type WorkerTrainOutcome = + | { success: true; result: WorkerTrainResult } + | { success: false; reason: WorkerFailureReason }; + +/** + * What a pending request settles to: a real worker response, or a synthetic + * failure carrying the reason when the worker never delivered one (watchdog + * timeout or a fatal worker error). + */ +type RequestOutcome = + | MlWorkerResponse + | { kind: "failure"; reason: WorkerFailureReason }; + const workerInactivityTimeoutMs = 60000; interface PendingRequest { - resolve(response: MlWorkerResponse | undefined): void; + resolve(outcome: RequestOutcome): void; onProgress?: (value: number) => void; /** * Inactivity timeout, if this request is watchdogged. Reset on every @@ -70,7 +92,7 @@ export class MlWorkerClient { }); this.worker.onmessage = (event: MessageEvent) => this.handleMessage(event.data); - this.worker.onerror = () => this.handleFatalError(); + this.worker.onerror = () => this.handleFatalError("workerError"); if (this.loadedArtifacts) { // Restore the model after a respawn so prediction can resume. this.postRequest( @@ -118,7 +140,7 @@ export class MlWorkerClient { clearTimeout(pending.timer); } pending.timer = setTimeout( - () => this.handleFatalError(), + () => this.handleFatalError("timeout"), pending.timeoutMs ); } @@ -132,11 +154,12 @@ export class MlWorkerClient { } /** - * Called when the worker itself fails (e.g. its script or wasm binary - * failed to load). Fail all pending requests and discard the worker so - * the next request respawns it. + * Called when the worker fails fatally: it crashed (onerror — e.g. its + * script or wasm binary failed to load) or went silent past the watchdog. + * Fail all pending requests with the reason and discard the worker so the + * next request respawns it. */ - private handleFatalError() { + private handleFatalError(reason: WorkerFailureReason) { const pending = Array.from(this.pending.values()); pending.forEach((p) => { if (p.timer !== undefined) { @@ -146,7 +169,7 @@ export class MlWorkerClient { this.pending.clear(); this.worker?.terminate(); this.worker = undefined; - pending.forEach((p) => p.resolve(undefined)); + pending.forEach((p) => p.resolve({ kind: "failure", reason })); } private postRequest( @@ -154,7 +177,7 @@ export class MlWorkerClient { worker: Worker = this.ensureWorker(), onProgress?: (value: number) => void, timeoutMs?: number - ): Promise { + ): Promise { const id = this.nextRequestId++; return new Promise((resolve) => { this.pending.set(id, { resolve, onProgress, timeoutMs }); @@ -168,29 +191,36 @@ export class MlWorkerClient { * prediction; the returned artifacts and ml4f machine code are for * persistence and MakeCode project generation. * - * Returns undefined on training or worker failure. + * On failure returns `{ success: false, reason }` so callers can report why + * (a watchdog timeout, a worker crash, or a training error). */ async train( features: number[][], labels: number[][], options: TrainModelOptions, onProgress: (value: number) => void - ): Promise { + ): Promise { this.modelOpInFlight = true; try { - const response = await this.postRequest( + const outcome = await this.postRequest( (id) => ({ kind: "train", id, features, labels, options }), this.ensureWorker(), onProgress, workerInactivityTimeoutMs ); - if (response?.kind !== "trainComplete") { - return undefined; + if (outcome.kind === "trainComplete") { + this.loadedArtifacts = outcome.artifacts; + return { + success: true, + result: { + artifacts: outcome.artifacts, + machineCode: outcome.machineCode, + }, + }; } - this.loadedArtifacts = response.artifacts; return { - artifacts: response.artifacts, - machineCode: response.machineCode, + success: false, + reason: outcome.kind === "failure" ? outcome.reason : "requestError", }; } finally { this.modelOpInFlight = false; @@ -215,7 +245,7 @@ export class MlWorkerClient { undefined, workerInactivityTimeoutMs ); - if (response?.kind !== "loadModelComplete") { + if (response.kind !== "loadModelComplete") { return undefined; } this.loadedArtifacts = artifacts; @@ -239,7 +269,7 @@ export class MlWorkerClient { id, features, })); - return response?.kind === "predictComplete" + return response.kind === "predictComplete" ? response.confidences : undefined; } diff --git a/src/store.ts b/src/store.ts index a42386f96..f87f3333f 100644 --- a/src/store.ts +++ b/src/store.ts @@ -1314,6 +1314,12 @@ const createMlStore = (logging: Logging) => { (trainModelProgress) => set({ trainModelProgress }, false, "trainModelProgress") ); + if (trainingResult.error) { + logging.error( + "Model training failed", + new Error(`Model training failed: ${trainingResult.reason}`) + ); + } const model = trainingResult.error ? undefined : trainingResult.model; diff --git a/src/train-model.ts b/src/train-model.ts index 57317d56f..ee2894178 100644 --- a/src/train-model.ts +++ b/src/train-model.ts @@ -9,14 +9,14 @@ * only its serialised artifacts and ml4f machine code. */ import { prepareFeaturesAndLabels } from "./ml"; -import { mlWorker } from "./ml-worker-client"; +import { mlWorker, type WorkerFailureReason } from "./ml-worker-client"; import { mlSettings } from "./mlConfig"; import { ActionData, TrainedModel } from "./model"; import { DataWindow } from "./project-utils"; export type TrainModelResult = | { error: false; model: TrainedModel } - | { error: true }; + | { error: true; reason: WorkerFailureReason }; const minTrainingDurationMs = 2000; @@ -49,7 +49,7 @@ export const trainModel = async ( }; requestAnimationFrame(tick); - const trainResult = await mlWorker.train( + const outcome = await mlWorker.train( features, labels, { @@ -60,15 +60,15 @@ export const trainModel = async ( actualProgress = value; } ); - const result: TrainModelResult = trainResult + const result: TrainModelResult = outcome.success ? { error: false, model: { - artifacts: trainResult.artifacts, - machineCode: trainResult.machineCode, + artifacts: outcome.result.artifacts, + machineCode: outcome.result.machineCode, }, } - : { error: true }; + : { error: true, reason: outcome.reason }; const remaining = minTrainingDurationMs - (Date.now() - startTime); if (!result.error && remaining > 0) { await new Promise((resolve) => setTimeout(resolve, remaining));