From ec641e82c085112a4a5e61726c2683a1009f6c94 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Sun, 9 Aug 2026 21:42:40 +0200 Subject: [PATCH 1/3] Keep the upload destination file open across chunks uploadChunk() was opening, writing, and closing the destination file on every single chunk - real per-chunk overhead (open/close syscalls, plus whatever flush happens on close), especially costly against the FAT-formatted USB target this writes to. For a ~1.2GB image at 3MB chunks, that's ~400 chunks each paying this cost. uploadMagicChunk already gets this right - it opens the file handle once (lazily, on the first chunk) and reuses it for the rest. This brings the regular upload path in line with that: uploadStart opens the file once and stores the handle on state.File, uploadChunk just writes to it, and uploadFinish (uploadCancel already did) closes it. Also fixes a pre-existing bug found along the way: the base64 decode error in uploadChunk was captured but never actually checked. One piece of a larger investigation into #75/#61 (slow/stalling uploads) - this addresses the server-side per-chunk overhead specifically; the client-side serialized, non-pipelined chunk loop and the base64/JSON wire overhead are separate, larger changes not included here. Co-Authored-By: Claude Sonnet 5 --- reflash/server.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/reflash/server.go b/reflash/server.go index cbb286a..bba9b11 100644 --- a/reflash/server.go +++ b/reflash/server.go @@ -578,7 +578,11 @@ func uploadStart(w http.ResponseWriter, r *http.Request) { timeStart = time.Now() logInfo("Starting upload at " + timeStart.Format("15:04:05")) logInfo("Filename: " + state.Filename) - os.Create(images_folder + "/" + state.Filename) + f, err := os.Create(images_folder + "/" + state.Filename) + if err != nil { + log.Fatal(err) + } + state.File = f sendResponse(w, nil) } @@ -674,25 +678,26 @@ func uploadChunk(w http.ResponseWriter, r *http.Request) { reqBody, _ := io.ReadAll(r.Body) json.Unmarshal(reqBody, &chunk) - decoded, err := base64.StdEncoding.DecodeString(chunk.Encoded[37:len(chunk.Encoded)]) - - path := images_folder + "/" + state.Filename - if state.State == CANCELLED { response := map[string]bool{"success": false} json.NewEncoder(w).Encode(response) return } - f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0644) + decoded, err := base64.StdEncoding.DecodeString(chunk.Encoded[37:len(chunk.Encoded)]) if err != nil { - log.Fatal(err) - } - if _, err := f.Write(decoded); err != nil { - log.Fatal(err) + http.Error(w, "Failed to decode base64", http.StatusBadRequest) + return } - if err := f.Close(); err != nil { - log.Fatal(err) + + // state.File is opened once in uploadStart and closed in + // uploadFinish/uploadCancel - opening, writing and closing it on + // every chunk here was real per-chunk overhead (syscalls plus an + // implicit flush on close), especially costly against the + // FAT-formatted USB target. + if _, err := state.File.Write(decoded); err != nil { + http.Error(w, "Failed to write chunk to file", http.StatusInternalServerError) + return } state.BytesNow += len(decoded) state.Progress = float64(state.BytesNow) * 100 / float64(state.BytesTotal) @@ -702,6 +707,12 @@ func uploadChunk(w http.ResponseWriter, r *http.Request) { } func uploadFinish(w http.ResponseWriter, r *http.Request) { + if state.File != nil { + if err := state.File.Close(); err != nil { + log.Fatal(err) + } + state.File = nil + } mountUsb(MODE_RO) duration := time.Since(timeStart) logInfo(fmt.Sprintf("Upload finished in %d minutes and %d seconds", int(duration.Minutes()), int(duration.Seconds())%60)) From 5e8df512560406d26061d46ed6936495eafe1779 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Sun, 9 Aug 2026 21:49:41 +0200 Subject: [PATCH 2/3] Add an end-to-end test for the multi-chunk upload path Drives upload_start -> ~22 upload_chunk calls -> upload_finish through the real handlers (not mocked), and checks the resulting file on disk matches the original payload byte-for-byte. Exercises the state.File handle kept open across chunks, so this would actually catch truncation, overwriting, or interleaving bugs - a build-only check can't. Co-Authored-By: Claude Sonnet 5 --- reflash/server_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/reflash/server_test.go b/reflash/server_test.go index 7e6bc4d..c32f64f 100644 --- a/reflash/server_test.go +++ b/reflash/server_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "encoding/base64" "encoding/json" "net/http/httptest" "os" @@ -393,3 +395,65 @@ func TestGetUncompressedSizeRealXz(t *testing.T) { t.Errorf("getUncompressedSize = %d, want %d", got, size) } } + +// End-to-end against the real handlers: drives upload_start -> several +// upload_chunk calls -> upload_finish exactly like the client does, and +// checks the file on disk matches byte-for-byte. Exercises the real +// state.File handle kept open across chunks (rather than the old +// open/write/close-per-chunk code), so this would catch truncation, +// overwriting, or interleaving bugs a build-only check can't. +func TestUploadChunkRoundTrip(t *testing.T) { + setupTest(t) + state = &State{State: IDLE} + + filename := "roundtrip-test.img.xz" + payload := bytes.Repeat([]byte("The quick brown fox jumps over the lazy dog. "), 5000) // ~225KB + + startBody, _ := json.Marshal(map[string]any{ + "filename": filename, + "size": len(payload), + "start_time": 0, + }) + w := httptest.NewRecorder() + uploadStart(w, httptest.NewRequest("PUT", "/api/upload_start", bytes.NewReader(startBody))) + if state.File == nil { + t.Fatal("uploadStart did not open state.File") + } + + const chunkSize = 10 * 1024 // small relative to payload so this genuinely exercises multiple sequential chunk writes, not just one + for i := 0; i < len(payload); i += chunkSize { + end := i + chunkSize + if end > len(payload) { + end = len(payload) + } + encoded := "data:application/octet-stream;base64," + base64.StdEncoding.EncodeToString(payload[i:end]) + chunkBody, _ := json.Marshal(map[string]string{"chunk": encoded}) + w := httptest.NewRecorder() + uploadChunk(w, httptest.NewRequest("POST", "/api/upload_chunk", bytes.NewReader(chunkBody))) + + var resp map[string]bool + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("chunk at offset %d: bad response body: %v", i, err) + } + if !resp["success"] { + t.Fatalf("chunk at offset %d: server reported failure", i) + } + } + + uploadFinish(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/upload_finish", nil)) + + if state.File != nil { + t.Error("uploadFinish did not clear state.File") + } + + got, err := os.ReadFile(filepath.Join(images_folder, filename)) + if err != nil { + t.Fatalf("reading uploaded file: %v", err) + } + if !bytes.Equal(got, payload) { + t.Errorf("uploaded file mismatch: got %d bytes, want %d bytes", len(got), len(payload)) + } + if state.BytesNow != len(payload) { + t.Errorf("state.BytesNow = %d, want %d", state.BytesNow, len(payload)) + } +} From 1a068b1890f881c2426429816de8e1b9e1b631e5 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Mon, 10 Aug 2026 20:28:24 +0200 Subject: [PATCH 3/3] Send upload chunks as raw binary, add client-side retry/backoff base64-encoding each chunk inflated the wire size by ~33% on an already WiFi-constrained upload path, and cost real CPU/JSON overhead on both ends. Chunks are now posted as raw octet-stream bodies instead - the client posts the File.slice() Blob directly, the server reads it straight off the request body. Separately, uploadLocalFile()'s request chain had no .catch() and no timeout at all, so a single dropped/hung request (this link has real multi-hundred-ms latency spikes and occasional dead spells) silently froze the whole upload forever with no feedback - see #61. Added a 20s per-chunk timeout and retry with exponential backoff (capped at 30s, up to 20 attempts) so a bad stretch gets ridden out instead. Chunk pipelining/concurrency was also tried as a further speed improvement, but reverted: this board's WiFi NIC and USB storage share a single USB hub with a single Transaction Translator (confirmed via the hub's own datasheet), so concurrent chunks caused real instability (a disk-write pileup requiring a board reboot, then persistent connection resets) rather than just being faster. Chunks stay serial, one at a time. Live-tested end to end: a full ~1.2GB image upload completed in 12m48s with no errors, faster than the pre-fix baseline (~20min) and with no hangs, on top of #91/#92/#93/#94. Closes #75 --- client/src/App.vue | 76 ++++++++++++++++++++++++++++++++++-------- reflash/server.go | 21 ++++++++---- reflash/server_test.go | 5 +-- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/client/src/App.vue b/client/src/App.vue index fa87ac2..b040fb0 100644 --- a/client/src/App.vue +++ b/client/src/App.vue @@ -470,39 +470,89 @@ export default { }, async uploadLocalFile() { const CHUNK_SIZE = 3 * 1024 * 1024; + const MAX_CHUNK_RETRIES = 20; + const CHUNK_TIMEOUT_MS = 20000; + const RETRY_BACKOFF_MS = 2000; + const RETRY_BACKOFF_MAX_MS = 30000; let self = this; - var reader = new FileReader(); var offset = 0; var filesize = this.file.size; - reader.onload = function () { - var result = reader.result; - var chunk = result; + // Chunks are posted as raw binary Blob slices, not base64-encoded + // inside a JSON body - base64 inflates the wire size by ~33%, + // which mattered a lot on the WiFi-constrained upload path. This + // also drops the FileReader/readAsDataURL round trip entirely. + // + // Chunks are sent one at a time, not pipelined. Concurrent chunks + // were tried and reverted - this board's USB hub has WiFi and + // storage sharing a single Transaction Translator (confirmed from + // the hub's datasheet), so simultaneous network-receive and + // disk-write transactions destabilize the connection rather than + // just being slower. + // + // This link also has occasional multi-minute dead spells (no + // error, no response, ever - axios has no default timeout for + // that case) that look like genuine WiFi/AP hiccups rather than + // one-off blips. Without an explicit timeout and a generous retry + // budget, one bad stretch used to freeze the whole upload forever + // with no feedback - see #61. Retries use exponential backoff + // (capped) so a real multi-minute dropout can be ridden out + // instead of giving up after a few seconds. + function sendChunk(retriesLeft = MAX_CHUNK_RETRIES) { + var slice = self.file.slice(offset, offset + CHUNK_SIZE); axios - .post(`/api/upload_chunk`, { - chunk: chunk, + .post(`/api/upload_chunk`, slice, { + headers: { "Content-Type": "application/octet-stream" }, + timeout: CHUNK_TIMEOUT_MS, }) .then(function (response) { const status = response.data; if (status.success && self.state == "UPLOADING") { offset += CHUNK_SIZE; - if (offset <= filesize) { - var slice = self.file.slice(offset, offset + CHUNK_SIZE); - reader.readAsDataURL(slice); + if (offset < filesize) { + sendChunk(); } else { - offset = filesize; self.apiCall("upload_finish"); } } else { self.apiCall("upload_cancel"); } + }) + .catch(function (error) { + if (self.state != "UPLOADING") { + return; + } + if (retriesLeft > 0) { + const attempt = MAX_CHUNK_RETRIES - retriesLeft; + const backoff = Math.min( + RETRY_BACKOFF_MS * Math.pow(2, attempt), + RETRY_BACKOFF_MAX_MS + ); + console.log( + "upload_chunk failed, retrying in " + + backoff + + "ms (" + + retriesLeft + + " left): " + + error + ); + setTimeout(function () { + sendChunk(retriesLeft - 1); + }, backoff); + } else { + self.$waveui.notify( + "Upload failed after repeated retries: " + error, + "error", + 0 + ); + self.apiCall("upload_cancel"); + } }); - }; + } if (this.file) { - var slice = this.file.slice(offset, offset + CHUNK_SIZE); - reader.readAsDataURL(slice); self.fileName = this.file.name; + sendChunk(); } }, onMagicButtonClick() { diff --git a/reflash/server.go b/reflash/server.go index bba9b11..c2b4b23 100644 --- a/reflash/server.go +++ b/reflash/server.go @@ -674,19 +674,19 @@ func uploadMagicFinish(w http.ResponseWriter, r *http.Request) { } func uploadChunk(w http.ResponseWriter, r *http.Request) { - var chunk *Chunk = &Chunk{} - reqBody, _ := io.ReadAll(r.Body) - json.Unmarshal(reqBody, &chunk) - if state.State == CANCELLED { response := map[string]bool{"success": false} json.NewEncoder(w).Encode(response) return } - decoded, err := base64.StdEncoding.DecodeString(chunk.Encoded[37:len(chunk.Encoded)]) + // The client posts the chunk as a raw binary body (not base64/JSON - + // that inflated the wire size by ~33% and cost real time on the + // WiFi-constrained upload path). Read it straight off the request + // body. + decoded, err := io.ReadAll(r.Body) if err != nil { - http.Error(w, "Failed to decode base64", http.StatusBadRequest) + http.Error(w, "Failed to read chunk body", http.StatusBadRequest) return } @@ -695,6 +695,15 @@ func uploadChunk(w http.ResponseWriter, r *http.Request) { // every chunk here was real per-chunk overhead (syscalls plus an // implicit flush on close), especially costly against the // FAT-formatted USB target. + // + // Chunks are sent one at a time (not pipelined/concurrent) - this + // board's USB hub (WiFi NIC + storage share a single Transaction + // Translator, confirmed from the hub's datasheet) can't reliably + // handle simultaneous network-receive and disk-write transactions. + // Concurrent chunks caused a real disk-write pileup (kernel threads + // stuck in D-state) and, even after serializing the writes, + // intermittent connection resets - a hardware constraint, not + // something fixable by tuning the write path further. if _, err := state.File.Write(decoded); err != nil { http.Error(w, "Failed to write chunk to file", http.StatusInternalServerError) return diff --git a/reflash/server_test.go b/reflash/server_test.go index c32f64f..161de6f 100644 --- a/reflash/server_test.go +++ b/reflash/server_test.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "encoding/base64" "encoding/json" "net/http/httptest" "os" @@ -426,10 +425,8 @@ func TestUploadChunkRoundTrip(t *testing.T) { if end > len(payload) { end = len(payload) } - encoded := "data:application/octet-stream;base64," + base64.StdEncoding.EncodeToString(payload[i:end]) - chunkBody, _ := json.Marshal(map[string]string{"chunk": encoded}) w := httptest.NewRecorder() - uploadChunk(w, httptest.NewRequest("POST", "/api/upload_chunk", bytes.NewReader(chunkBody))) + uploadChunk(w, httptest.NewRequest("POST", "/api/upload_chunk", bytes.NewReader(payload[i:end]))) var resp map[string]bool if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {