From 5ded7419f2e211bd6d91db368cd234b6c9fc2b03 Mon Sep 17 00:00:00 2001 From: kn Date: Thu, 5 Nov 2026 15:29:04 +0100 Subject: [PATCH 1/2] Run the upload finish request without a timeout /upload/finish makes the server hash the whole stream, so its duration grows with the file size; on slow hardware such as a Raspberry Pi a large file exceeds the default 10 s request timeout. axios then aborted the request, the rejection was swallowed, and the upload widget stayed at 100 % although the upload never completed. The finish request of build and image uploads now runs without a timeout, like the chunk POSTs. A failed init, chunk or finish request calls the new AMCUploadState.failUpload(), which invokes _onError, so the frontend shows the failure instead of hanging. --- Client/core/common/AMCApplication.js | 19 ++-- .../core/common/AMCImplementation_Upload.js | 7 ++ .../common/AMCImplementation_UploadState.js | 20 ++++ Client/tests/uploadFinish.test.mjs | 95 +++++++++++++++++++ 4 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 Client/tests/uploadFinish.test.mjs diff --git a/Client/core/common/AMCApplication.js b/Client/core/common/AMCApplication.js index a488f40be..738afacc8 100644 --- a/Client/core/common/AMCApplication.js +++ b/Client/core/common/AMCApplication.js @@ -74,6 +74,9 @@ import AMCApplicationDialog from "./AMCDialog.js" const CONFIG_REQUEST_TIMEOUT_MS = 2000; const DEFAULT_REQUEST_TIMEOUT_MS = 10000; +// /upload/finish makes the server hash the whole stream, so its duration grows with the file size +// (a large file can take more than 10 s on slow hardware). Like the chunk POSTs it runs without a timeout. +const UPLOAD_FINISH_TIMEOUT_MS = 0; const MAX_CONSECUTIVE_FAILURES = 5; // Client reactivity metrics: aggregate /api/frontend roundtrips over this window @@ -1825,11 +1828,11 @@ export default class AMCApplication extends Common.AMCObject { uploadObject.setStateMessageToWaiting (); - application.axiosPostRequest("/upload/" + uploadObject.streamuuid + "/finish", { + return application.axiosPostRequest("/upload/" + uploadObject.streamuuid + "/finish", { "streamuuid": uploadObject.streamuuid, "context": "build", "sha256": checkSum - }) + }, { timeout: UPLOAD_FINISH_TIMEOUT_MS }) .then(resultUploadFinish => { uploadObject.clearUploadState (); @@ -1879,7 +1882,7 @@ export default class AMCApplication extends Common.AMCObject { } }) .catch(err => { - err; + uploadObject.failUpload (err); }); @@ -1914,7 +1917,7 @@ export default class AMCApplication extends Common.AMCObject { }) .catch(err => { - err; + itemState.failUpload (err); /* if (failureeventname) this.triggerUIEvent(failureeventname, itemuuid, {}); */ }); @@ -1955,11 +1958,11 @@ export default class AMCApplication extends Common.AMCObject { uploadObject.setStateMessageToWaiting (); - application.axiosPostRequest("/upload/finish", { + return application.axiosPostRequest("/upload/finish", { "streamuuid": uploadObject.streamuuid, "context": "image", "sha256": checkSum - }) + }, { timeout: UPLOAD_FINISH_TIMEOUT_MS }) .then(resultUploadFinish => { uploadObject.clearUploadState (); @@ -2010,7 +2013,7 @@ export default class AMCApplication extends Common.AMCObject { } }) .catch(err => { - err; + uploadObject.failUpload (err); }); @@ -2047,7 +2050,7 @@ export default class AMCApplication extends Common.AMCObject { }) .catch(err => { - err; + itemState.failUpload (err); /* if (failureeventname) this.triggerUIEvent(failureeventname, itemuuid, {}); */ }); diff --git a/Client/core/common/AMCImplementation_Upload.js b/Client/core/common/AMCImplementation_Upload.js index 7f254c58d..bdf98a7f4 100644 --- a/Client/core/common/AMCImplementation_Upload.js +++ b/Client/core/common/AMCImplementation_Upload.js @@ -220,6 +220,13 @@ export default class AMCUpload extends Common.AMCObject { } } + failUpload (reason) + { + if (this.itemState) { + this.itemState.failUpload (reason); + } + } + clearUploadState () { if (this.itemState) { diff --git a/Client/core/common/AMCImplementation_UploadState.js b/Client/core/common/AMCImplementation_UploadState.js index cda370110..fb1527c8d 100644 --- a/Client/core/common/AMCImplementation_UploadState.js +++ b/Client/core/common/AMCImplementation_UploadState.js @@ -46,6 +46,7 @@ export default class AMCUploadState extends Common.AMCObject { this.messages = []; this.itemUUID = itemUUID; this.cancelMessage = "Upload canceled"; + this.failMessage = "Upload failed"; this.finishMessage = "Upload finished"; this.startMessage = "Starting upload"; this.progressMessage = "Uploading..."; @@ -95,6 +96,25 @@ export default class AMCUploadState extends Common.AMCObject { } } + // Ends the upload after a failed request: the frontend widget learns about it through _onError + // instead of waiting forever. + failUpload (reason) + { + if (reason) + console.error ("Upload failed:", reason); + + this.uploadID = 0; + this.chosenFile = null; + this.progress = 0; + this.setMessage (this.failMessage); + if (this._onError) { + try { this._onError (); } catch (_) { /* noop */ } + } + this._onComplete = null; + this._onError = null; + this._onProgress = null; + } + setChosenFile (file) { Assert.ObjectValue (file); diff --git a/Client/tests/uploadFinish.test.mjs b/Client/tests/uploadFinish.test.mjs new file mode 100644 index 000000000..d0b1f0f31 --- /dev/null +++ b/Client/tests/uploadFinish.test.mjs @@ -0,0 +1,95 @@ +// Run with: cd Client/tests && npm test (or from the repository root: node --test 'Client/tests/*.test.mjs') +// +// /upload/finish makes the server hash the whole stream (twice: SHA-256 and blockwise SHA-256), so its +// duration grows with the file size. On slow hardware a large file takes longer than the default 10 s +// request timeout: the client aborted the request, swallowed the error and the upload widget stayed at +// 100 % forever, although the server had stored and validated the file. The finish request therefore +// runs without a timeout (like the chunk POSTs), and any failed upload request is reported to the +// upload state so the frontend can show it. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { importCore } from "./support/coreLoader.mjs"; + +const AMCApplication = await importCore ("common/AMCApplication.js"); +const AMCUploadState = await importCore ("common/AMCImplementation_UploadState.js"); + +const ITEM_UUID = "00000000-0000-4000-8000-000000000001"; +const STREAM_UUID = "00000000-0000-4000-8000-000000000002"; +const CONTEXT_UUID = "00000000-0000-4000-8000-000000000003"; + +// Node has Blob and File but no FileReader; AMCUpload only uses readAsArrayBuffer + onload. +globalThis.FileReader = class { + readAsArrayBuffer (blob) { + blob.arrayBuffer ().then ((buffer) => { this.result = buffer; this.onload (); }); + } +}; + +function flushPromises () { + return new Promise ((resolve) => setImmediate (resolve)); +} + +async function settle () { + for (let i = 0; i < 20; i++) + await flushPromises (); +} + +// finish: how the /upload/finish request ends ("resolve" or "reject"). +function createUpload (finish) { + const app = new AMCApplication ("http://127.0.0.1:1/api", null); + app.API.authToken = "1".repeat (64); + + const requests = { posts: [], events: [] }; + app.axiosPostRequest = (url, data, config) => { + requests.posts.push ({ url, data, config }); + if (url === "/upload/") + return Promise.resolve ({ data: { streamuuid: STREAM_UUID } }); + if (url.endsWith ("/finish")) { + if (finish === "reject") + return Promise.reject (new Error ("timeout of 10000ms exceeded")); + return Promise.resolve ({ data: { contextuuid: CONTEXT_UUID } }); + } + return Promise.reject (new Error ("unexpected POST " + url)); + }; + app.axiosPostFormData = () => Promise.resolve ({ data: {} }); + app.triggerUIEvent = (eventName, sender, values) => requests.events.push ({ eventName, sender, values }); + + const state = new AMCUploadState (ITEM_UUID); + state.setChosenFile (new File ([new Uint8Array (1024)], "part.mp4", { type: "video/mp4" })); + state.generateUploadID (); + + const callbacks = { complete: 0, error: 0 }; + state._onComplete = () => { callbacks.complete++; }; + state._onError = () => { callbacks.error++; }; + + return { app, state, requests, callbacks }; +} + +for (const [label, start] of [ + ["image", (app, state) => app.performImageUpload (state, "onsuccess", "onfailure")], + ["build", (app, state) => app.performJobUpload (state, "onsuccess", "onfailure")], +]) { + test (`${label} upload: the finish request has no timeout`, async () => { + const { app, state, requests, callbacks } = createUpload ("resolve"); + start (app, state); + await settle (); + + const finish = requests.posts.find ((post) => post.url.endsWith ("/finish")); + assert.ok (finish, "finish request sent"); + assert.equal (finish.config?.timeout, 0); + assert.equal (callbacks.complete, 1); + assert.deepEqual (requests.events.map ((event) => event.eventName), ["onsuccess"]); + }); + + test (`${label} upload: a failed finish request is reported to the upload state`, async () => { + const { app, state, requests, callbacks } = createUpload ("reject"); + start (app, state); + await settle (); + + assert.equal (callbacks.error, 1); + assert.equal (callbacks.complete, 0); + assert.deepEqual (requests.events, []); + assert.equal (state.getUploadID (), 0, "upload no longer active"); + }); +} From 1a2de8a11d187e477428c03cb1e475e453690f2d Mon Sep 17 00:00:00 2001 From: kn Date: Thu, 5 Nov 2026 15:29:05 +0100 Subject: [PATCH 2/2] Fail only the active upload after a failed request An upload widget reuses one AMCUploadState, and the user can start a new upload while one is still running; the new upload takes over with a new upload ID. A failing request of the replaced upload (e.g. a finish for a 3MF that does not parse) then reset the upload ID and fired _onError, which silently stopped the newer upload. AMCUpload.failUpload() now only acts while its upload is the active one, as the chunk handlers already check. --- .../core/common/AMCImplementation_Upload.js | 4 +- Client/tests/uploadFinish.test.mjs | 44 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/Client/core/common/AMCImplementation_Upload.js b/Client/core/common/AMCImplementation_Upload.js index bdf98a7f4..e5fa4e140 100644 --- a/Client/core/common/AMCImplementation_Upload.js +++ b/Client/core/common/AMCImplementation_Upload.js @@ -220,9 +220,11 @@ export default class AMCUpload extends Common.AMCObject { } } + // Only the upload that is still active may fail the state: a newer upload in the same widget + // (the user picked another file meanwhile) is not ended by a request of the one it replaced. failUpload (reason) { - if (this.itemState) { + if (this.checkIfUploadIsActive ()) { this.itemState.failUpload (reason); } } diff --git a/Client/tests/uploadFinish.test.mjs b/Client/tests/uploadFinish.test.mjs index d0b1f0f31..fba3a111e 100644 --- a/Client/tests/uploadFinish.test.mjs +++ b/Client/tests/uploadFinish.test.mjs @@ -93,3 +93,47 @@ for (const [label, start] of [ assert.equal (state.getUploadID (), 0, "upload no longer active"); }); } + +// A finish request of an upload that a newer upload in the same widget has replaced (the user picked or +// dropped another file meanwhile) must not end the newer upload when it fails. +for (const [label, start] of [ + ["image", (app, state) => app.performImageUpload (state, "onsuccess", "onfailure")], + ["build", (app, state) => app.performJobUpload (state, "onsuccess", "onfailure")], +]) { + test (`${label} upload: a failed finish of a replaced upload leaves the newer upload alone`, async () => { + const { app, state, requests } = createUpload ("resolve"); + + const pendingFinishes = []; + app.axiosPostRequest = (url, data, config) => { + requests.posts.push ({ url, data, config }); + if (url === "/upload/") + return Promise.resolve ({ data: { streamuuid: STREAM_UUID } }); + if (url.endsWith ("/finish")) + return new Promise ((resolve, reject) => pendingFinishes.push ({ resolve, reject })); + return Promise.reject (new Error ("unexpected POST " + url)); + }; + + start (app, state); + await settle (); + assert.equal (pendingFinishes.length, 1, "first finish request pending"); + + const second = { complete: 0, error: 0 }; + state.setChosenFile (new File ([new Uint8Array (1024)], "second.mp4", { type: "video/mp4" })); + const secondUploadID = state.generateUploadID (); + state._onComplete = () => { second.complete++; }; + state._onError = () => { second.error++; }; + start (app, state); + await settle (); + assert.equal (pendingFinishes.length, 2, "second finish request pending"); + + pendingFinishes[0].reject (new Error ("checksum mismatch")); + await settle (); + assert.equal (second.error, 0, "newer upload not failed"); + assert.equal (state.getUploadID (), secondUploadID, "newer upload still active"); + + pendingFinishes[1].resolve ({ data: { contextuuid: CONTEXT_UUID } }); + await settle (); + assert.equal (second.complete, 1); + assert.deepEqual (requests.events.map ((event) => event.eventName), ["onsuccess"]); + }); +}