3 commits

Author	SHA1	Message	Date
Gergő Magyar	e00959dfb6	test(gitnexus): stabilize rel-csv-split stream teardown on Windows (expect.poll) (#1052) ... * test(lbug): stabilize rel-csv-split Windows CI with expect.poll Fixed sleeps assumed readline had already created the first mock stream within 20ms; windows-latest can lag, causing streams.length===0 and ENOTEMPTY tempdir cleanup. Poll up to 10s instead (Vitest 4). Refs #1051 Made-with: Cursor * test(lbug): use exact toBe assertions in rel-csv-split (DoD §2.7) - Poll for streams.length === 2 after unblock (two pair keys only) - disk-full test: streams.length === 1 for single Function|Class row Made-with: Cursor * test(lbug): replace rel-csv-split setTimeout waits with expect.poll Shared pollOpts; drain-listener and disk-full tests now wait on streams.length instead of fixed 50ms sleeps (DoD §2.7 deterministic tests). Made-with: Cursor	2026-04-23 20:11:54 +01:00
Gergő Magyar	28ddbe5d54	fix(lbug): wait for read stream close in splitRelCsvByLabelPair (Windows ENOTEMPTY) (#832) ... * fix(lbug): wait for read stream close in splitRelCsvByLabelPair (Windows ENOTEMPTY) The windows-latest CI job intermittently failed: FAIL test/unit/rel-csv-split.test.ts > splitRelCsvByLabelPair > handles empty CSV (header only) without errors Error: ENOTEMPTY: directory not empty, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\rel-csv-test-XW5KOu' Cause: splitRelCsvByLabelPair resolved its Promise on readline's 'close' event, but the underlying fs.ReadStream's file descriptor is released asynchronously after that — especially on Windows. For the empty-CSV test the function returns so quickly that afterEach fires rmSync while the relations.csv fd is still held, so Windows reports ENOTEMPTY on the directory. Fixes: - Production: after readline 'close', wait for inputStream 'close' (or resolve immediately if already closed/destroyed). Call inputStream .destroy() defensively so we never hang if the fd never emits 'close'. - Test: afterEach now retries rmSync up to 5 times on ENOTEMPTY/EBUSY/ EPERM with a brief back-off — defense-in-depth so the test doesn't flake on slow CI runners independent of the production change. The production fix benefits every caller, not just the test: any code that deletes the CSV's parent directory right after the Promise resolves previously hit the same race on Windows. * refactor(lbug): replace custom stream state machines with stdlib primitives Full audit of splitRelCsvByLabelPair's stream usage after the original ENOTEMPTY fix. Replaced three hand-rolled mechanisms with their standard-library equivalents — 147 -> 71 lines in the function, and the caller's WriteStream closure dropped from 13 lines to 5. - readline: 'on(line)' + pause/resume/waitingForDrain state machine -> 'for await (const line of rl)'. Async-iterator delivery naturally serializes line processing with our awaits, so at most one ws is in backpressure at a time. We just 'await once(ws, "drain")' when 'write()' returns false — the custom Set, the settled flag and the 'only resume when all streams have drained' logic all go away. - Multi-stream error coordination: hand-rolled cleanup() that had to be entered exactly once and had to destroy the inputStream and every pair ws -> single AbortController shared across every 'once(ws, 'drain', { signal })'. Any stream error aborts every pending wait. - 'stream/promises.finished(inputStream)' in the 'finally' block replaces the manual 'rl.on('close', () => inputStream.once('close', ...))' dance, and covers both the success and error paths with the same primitive. This closes the Windows ENOTEMPTY race root cause — we never return while the fd might still be in flight. - Caller closure: 'new Promise((res, rej) => ws.end(cb) + remove listener on error)' -> 'ws.end(); await finished(ws)'. - Test 'afterEach': custom retry loop -> 'fs.rmSync(..., { maxRetries: 5, retryDelay: 50 })' (Node added these options specifically for cross-platform tmpdir cleanup). - Test 'destroys all streams when one errors': old code leaked backpressure and created multiple pair streams before the first blocked; new strict serial backpressure doesn't, so the test now unblocks the first stream once to advance the loop and create the second stream before triggering the error.	2026-04-15 08:59:09 +01:00
Md. Mekayel Anik	b340c5d87a	fix: prevent drain listener leak in relationship CSV streaming (#818) ... * fix: add setMaxListeners(50) to relationship pair WriteStreams Dynamically-created per-pair WriteStreams for relationship CSV splitting default to Node.js's maxListeners limit of 10. On large repositories with many relationship types, readline backpressure causes repeated ws.once('drain', ...) calls that exceed this limit, flooding stderr with MaxListenersExceededWarning messages. This matches the existing pattern in csv-generator.ts where BufferedCSVWriter already calls this.ws.setMaxListeners(50). * fix: address all 3 stream bugs in relationship CSV splitting Addresses review feedback from @magyargergo and Claude CI analysis: Bug 1 (High): Add error handlers to per-pair WriteStreams. Previously, if a WriteStream errored (disk full, EMFILE) while rl was paused waiting for drain, the drain callback never fired, rl.resume() was never called, and the outer Promise hung forever — leaking all open file descriptors until process kill. Now each WriteStream gets an error handler that destroys all streams, closes the readline interface + its input ReadStream, and rejects the Promise. Bug 2 (Medium): Add waitingForDrain Set to prevent drain listener accumulation. rl.pause() is not synchronous — buffered line events continue firing after pause(), and multiple lines targeting the same pairKey each added another ws.once('drain', ...) listener. This was the root cause of MaxListenersExceededWarning. Now a Set<string> tracks which streams are already waiting for drain. Only the first backpressure event registers the listener; subsequent lines for the same stream are silently skipped (they're already written to the stream buffer). This eliminates listener accumulation entirely and makes setMaxListeners(50) a safety net rather than a band-aid. Bug 3 (Low): Close readline and destroy input ReadStream in error handler. Previously only the WriteStreams were destroyed on error, leaving the ReadStream FD to linger until GC. * fix: address review feedback — remove setMaxListeners, harden cleanup - Remove setMaxListeners(50) entirely. The waitingForDrain guard guarantees at most 1 drain listener per stream at any time. Tested with 200 pairs x 500 lines (100k total) — max listeners was always 1, zero warnings. No hard-coded limit needed. - Wrap destroy() calls in cleanup() with try/catch so already-destroyed streams don't throw synchronously (addresses @xkonjin review point 1). - Add ws.once('error', reject) to the ws.end() phase so flush errors during stream close properly reject instead of hanging Promise.all (addresses Claude CI Bug 3b finding). * test: add 8 regression tests for relationship CSV stream fixes Covers all bugs fixed in this PR: - Bug 1: WriteStream error rejects Promise and destroys all streams - Bug 2: waitingForDrain guard keeps drain listeners at max 1 per stream - Bug 3: cleanup() handles already-destroyed streams safely Tests use a MockWriteStream with controllable backpressure and error injection to verify the exact patterns in loadGraphToLbug() without needing a real LadybugDB instance. * style: run prettier on changed files * fix(test): use backpressure to keep promise pending during error tests The error tests were racing — readline finished reading the tiny CSV and resolved the Promise before setTimeout fired the error. Now the mock streams use blocked=true to trigger backpressure, keeping the Promise pending so the error fires while the split is still in progress. * fix: use named error handler in ws.end() to prevent listener leak ws.once() wraps the callback, so removeListener with the original function reference won't match. Switch to ws.on() with a named onError function so removeListener correctly detaches it after successful close. * refactor: extract splitRelCsvByLabelPair, fix multi-stream drain 1. Extract splitRelCsvByLabelPair as an exported function with optional wsFactory parameter for dependency injection. loadGraphToLbug now delegates to it. Tests import and call the real function instead of a local reimplementation. 2. Fix multi-stream drain coordination: rl.resume() is now guarded by waitingForDrain.size === 0, so readline only resumes when ALL backpressured streams have drained. Previously, any single stream draining would resume readline while other streams were still full, allowing unbounded buffer growth. 3. Export WriteStreamFactory type and RelCsvSplitResult interface for test consumption.	2026-04-14 12:26:38 +01:00