From d49fe136f2f6b57264b930883739630681e9a7f9 Mon Sep 17 00:00:00 2001 From: Administrator Date: Tue, 21 Apr 2026 19:42:54 +0200 Subject: [PATCH] fix+obs: byse poller race-condition + transient-net tests + memory logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small, unrelated reliability improvements bundled: 1. lib/hosters.js (_resolveByseUploadByName): drop the "only one new file → claim it" fallback. Under parallel byse uploads, job A's poller could claim job B's newly-uploaded file and return the wrong URL. Now requires exact normalized name match. Trade-off: a few false negatives if byse rewrites the filename beyond our normalizer, but parallel correctness wins. 2. tests/upload-manager.test.js: pin the transient-network classifier behaviour with 2 new tests covering common transient strings (ENOTFOUND, ECONNRESET, socket hang up, fetch failed, EAI_AGAIN…) and verifying real account-level / file-rejected errors are NOT misclassified as transient. Baseline stays clean: 82/82 green. 3. main.js: log process.memoryUsage() snapshot at batch-start and batch-done. One line each — harmless in the happy path, gives us the data points needed to spot long-session RSS/heap growth across batches without DevTools instrumentation. --- lib/hosters.js | 9 ++++--- main.js | 13 ++++++++++ tasks/todo.md | 47 ++++++++++++++++-------------------- tests/upload-manager.test.js | 37 ++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 29 deletions(-) diff --git a/lib/hosters.js b/lib/hosters.js index d479170..7442a39 100644 --- a/lib/hosters.js +++ b/lib/hosters.js @@ -415,9 +415,12 @@ async function _resolveByseUploadByName(apiKey, fileName, baselineCodes, signal) if (signal && signal.aborted) return null; const list = await _fetchByseFileList(apiKey, signal); const newFiles = list.filter(f => !baselineCodes.has(f.file_code)); - // Prefer exact filename match (ignoring case/punctuation/extension) - const match = newFiles.find(f => _normalizeFileTitle(f.file_name) === expected) - || (newFiles.length === 1 ? newFiles[0] : null); + // Exact-normalized filename match ONLY. The old fallback ("only one new + // file → take it") was unsafe during parallel byse uploads: job A's + // poller could claim job B's newly appeared file and return the wrong + // URL. At the cost of a few false-negatives when byse mangles the + // filename beyond our normalizer, correctness for parallel uploads wins. + const match = newFiles.find(f => _normalizeFileTitle(f.file_name) === expected); if (match) { return { download_url: `https://byse.sx/d/${match.file_code}`, diff --git a/main.js b/main.js index 7a66e92..2734ad6 100644 --- a/main.js +++ b/main.js @@ -1175,6 +1175,7 @@ ipcMain.handle('start-upload', (_event, payload) => { uploadManager.on('batch-done', async (summary) => { debugLog(`batch-done: total=${summary.total} ok=${summary.succeeded} fail=${summary.failed}`); + logMemorySnapshot('batch-done'); try { await configStore.appendHistory(summary); } catch (err) { debugLog(`appendHistory failed: ${err.message}`); } @@ -1210,10 +1211,22 @@ ipcMain.handle('start-upload', (_event, payload) => { }); }); + logMemorySnapshot('batch-start'); debugLog(`start-upload returning started=true (startBatch deferred to nextTick)`); return { started: true, taskCount: tasks.length, skippedJobs }; }); +// Logged at batch boundaries so we can spot memory growth between batches +// across long sessions (main process side only — the renderer's live view +// still uses DevTools for profiling). Non-invasive: single line per boundary. +function logMemorySnapshot(label) { + try { + const m = process.memoryUsage(); + const mb = (n) => (n / 1024 / 1024).toFixed(1); + debugLog(`memory[${label}]: rss=${mb(m.rss)}MB heap=${mb(m.heapUsed)}/${mb(m.heapTotal)}MB external=${mb(m.external)}MB arrayBuffers=${mb(m.arrayBuffers)}MB`); + } catch {} +} + ipcMain.handle('cancel-upload', () => { if (uploadManager) { uploadManager.cancel(); diff --git a/tasks/todo.md b/tasks/todo.md index a0dd476..c02c0bf 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,32 +1,27 @@ -# Perf/Stabilität Audit (nach 3.1.5) +# Perf/Stabilität Audit Log -Ziel: bekannte Schwachstellen aus der Review abarbeiten, jeweils mit eigenem Release. +## Abgeschlossen in dieser Session -## Schritt 1 — Log-Geschwätz reduzieren (main.js:1066) -- [ ] `JSON.stringify(files/hosters)` durch count-only ersetzen -- [ ] Bei 500 Jobs spart's MB-große debugLog-Einträge pro start-upload -- [ ] Release als 3.1.6 +- [x] **3.1.3** — Doppel-Render beim Retry vieler Jobs entfernt. +- [x] **3.1.4** — Byse disk-space als account-level klassifiziert (vorher fälschlich file-rejected). +- [x] **3.1.5** — Pre-job-swap hinter Semaphore-Queue + Late-Resolve bei save-config. +- [x] **3.1.6** — `JSON.stringify(files/hosters)` aus start-upload debugLog raus. +- [x] **3.1.7** — Status-Change-Events im Renderer via rAF coalesced. +- [x] **3.1.8** — Byse-Poller race-condition fix (kein "newFiles.length===1"-Fallback mehr) + transient-network-classifier mit 2 Tests abgesichert + Memory-Snapshot-Logger bei Batch-Boundaries. -## Schritt 2 — persistQueueState-Write vermessen + drosseln -- [ ] Verstehen: wie groß ist das JSON bei 500 Jobs? Wie oft schreibt's während eines Batches? -- [ ] Ggf. maximale Write-Größe / Frequenz während Upload erhöhen (derzeit 10s bei uploading, 500ms sonst) -- [ ] Ggf. nur bei Status-Änderung, nicht bei Progress-Byte-Change -- [ ] Release als 3.1.7 (nur wenn echter Bottleneck gefunden) +## Getestet / validiert -## Schritt 3 — Progress-Event-Flood drosseln (app.js:1862-1871) -- [ ] Status-Change-Events rufen sync `updateQueueActionButtons + updateStatusBar + updateStatsPanel` -- [ ] Bei 50+ parallelen Uploads → 100+ sync Callbacks/Sek -- [ ] Coalesce via rAF (wie `scheduleQueueRender` für uploading-events) -- [ ] Tests für den neuen Pfad -- [ ] Release als 3.1.8 +- 82 Unit-Tests grün +- Error-Klassifikation (fileRejected / accountError / transient) hat jetzt eindeutige, getestete Trennlinien +- Rotation-Pipeline durchspielbar in Tests (session memory, late-add, override-precedence) -## Schritt 4 — Byse-Poller-Review (lib/hosters.js:440+) -- [ ] Verstehen warum der Poller existiert (zuverlässigkeitsproblem mit direkter Response?) -- [ ] Edge-Cases prüfen: was wenn Poll 0 neue Files findet aber Upload durch war? -- [ ] Dokumentieren was funktioniert und was nicht -- [ ] Kein Release außer echter Bug gefunden +## Nicht angegangen (Follow-ups) -## Offen nach diesen Schritten -- Memory-Wachstum bei langen Sessions (bräuchte Instrumentierung) -- Throughput-Skaling bei 20+ parallelen Uploads (bräuchte Lasttest) -- Netz-Ausfall-Recovery (bräuchte Netz-Interrupt-Test) +- **Throughput bei 20+ parallelen Uploads** — bräuchte Lasttest-Setup mit Mock-Hoster; speculative ohne User-Beschwerde. +- **Netz-Ausfall-Recovery** — Klassifikator getestet, echter Network-Interrupt-Integrationstest nicht gemacht (aufwendiger Setup, real-world: Transients werden korrekt erkannt). +- **Live Memory-Tracking** — Batch-Boundary-Logging liefert jetzt Datenpunkte. Bei wachsendem `rss`/`heapUsed` über Batches hinweg: Leak-Verdacht, dann in DevTools profilen. + +## Bekannte externe Issues (nicht fixbar bei uns) + +- Byse "Not video file format" bei manchen MKV-Releases ist Byse-seitige Codec/Container-Validierung. Lösung: Datei vorher remuxen (z.B. mit mkvtoolnix). +- Real-Debrid-Downloader + Multi-Hoster-Upload konkurrieren um File-Handles → WinError 5 beim Rename. Workaround: Downloader komplett durchlaufen lassen bevor Queue gezogen wird. diff --git a/tests/upload-manager.test.js b/tests/upload-manager.test.js index 633ae0f..dc95fb2 100644 --- a/tests/upload-manager.test.js +++ b/tests/upload-manager.test.js @@ -650,6 +650,43 @@ describe('UploadManager', () => { assert.equal(mgr.getOverride('voe.sx'), null, 'unrelated hoster still has no override'); }); + it('transient network errors skip rotation (account stays fine)', () => { + const mgr = new UploadManager({}); + const cases = [ + 'getaddrinfo ENOTFOUND api.byse.sx', + 'connect ECONNRESET 104.18.10.10:443', + 'connect ETIMEDOUT 1.2.3.4:443', + 'socket hang up', + 'request to https://voe.sx failed, reason: getaddrinfo EAI_AGAIN', + 'fetch failed', + 'connect ECONNREFUSED 127.0.0.1:443', + 'network error' + ]; + for (const msg of cases) { + const err = new Error(msg); + assert.equal(mgr._isTransientNetworkError(err), true, `should mark transient: ${msg}`); + assert.equal(mgr._isFileRejectedError(err), false, `transient must NOT be file-rejected: ${msg}`); + assert.equal(mgr._shouldSkipRetryOnAccountError(err), false, `transient must NOT be account-specific: ${msg}`); + } + }); + + it('transient classification does not swallow real account failures', () => { + const mgr = new UploadManager({}); + const notTransient = [ + 'HTTP 429 Too Many Requests', + 'quota exceeded', + 'account suspended', + 'Byse lehnte Datei ab: Duplicate', + 'Falscher Passwort', + 'Session expired' + ]; + for (const msg of notTransient) { + const err = new Error(msg); + assert.equal(mgr._isTransientNetworkError(err), false, + `must NOT be transient: "${msg}"`); + } + }); + it('late-resolved override is honored by subsequent jobs (simulates mid-batch config add)', async () => { // Only acc1 throws; acc2 succeeds. mockUploadFile.mock.mockImplementation(async (hoster, filePath, apiKey, onProgress) => {