From c424963d77dfd47faed74a86455fea979e081863 Mon Sep 17 00:00:00 2001 From: Jaydbrown <175232057+Jaydbrown@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:14:28 +0100 Subject: [PATCH] fix(safe-operations): share one fallback AbortSignal across a withBoundedParallel batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit withBoundedParallel built the signal handed to each handler as `options?.signal ?? new AbortController().signal` *inside* the worker loop. With no outer signal, that allocated a fresh AbortController per item and discarded it immediately — nothing held a reference to call .abort(), so signal.aborted was permanently false and per-item cancellation silently never worked (#221). Compute the fallback signal once per call, before the loop. Every item in an outer-signal-less call now receives the same AbortSignal instance. It's still not externally abortable without an API change (out of scope here), but it's a coherent shared object rather than a dead one built per item. The one real call site (components/stream/BulkWithdrawButton.tsx) always passes its own options.signal and ignores the handler's third parameter, so this is fully backward compatible. Closes #221 --- CHANGELOG.md | 1 + lib/safe-operations.test.ts | 22 ++++++++++++++++++++++ lib/safe-operations.ts | 12 +++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b31a422..9eaa685 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes are documented here. Format based on [Keep a Changelog](http - `force_cancel()` action in `StreamActions` for recipients (once contract support is merged) ### Fixed +- `withBoundedParallel` no longer manufactures and discards a fresh `AbortController` per item when the caller passes no outer signal — every handler in the batch now receives one shared, referenceable `AbortSignal` instead of a definitionally-dead one (#221) - `refreshStreamData` now invalidates active queries once instead of immediately refetching the same queries a second time - Removed the unused multisig transaction scaffold, which had no callers or tests and discarded the clipboard success result - `scValToU64`/`scValToI128` and `streamsBySender`/`streamsByRecipient` now boundary-check the RPC diff --git a/lib/safe-operations.test.ts b/lib/safe-operations.test.ts index c2b67b8..11243f9 100644 --- a/lib/safe-operations.test.ts +++ b/lib/safe-operations.test.ts @@ -636,6 +636,28 @@ describe('withBoundedParallel', () => { expect(receivedSignal!.aborted).toBe(false); }); + // Regression test for #221: withBoundedParallel built + // `new AbortController().signal` inline per item when no outer signal was + // passed, so every handler got a distinct signal that nothing could ever + // abort. The fix shares one controller across the whole batch. + it('shares a single fallback signal across items when no outer signal is provided', async () => { + const items = [1, 2, 3]; + const receivedSignals: AbortSignal[] = []; + + await withBoundedParallel( + items, + async (_item, _index, signal) => { + receivedSignals.push(signal); + return { success: true, data: 1 } as SafeOperationResult; + }, + { maxConcurrency: 1 }, // sequential for a deterministic order + ); + + expect(receivedSignals).toHaveLength(3); + expect(receivedSignals[1]).toBe(receivedSignals[0]); + expect(receivedSignals[2]).toBe(receivedSignals[0]); + }); + it('passes the same parent signal to handlers when provided', async () => { const controller = new AbortController(); const items = [1, 2]; diff --git a/lib/safe-operations.ts b/lib/safe-operations.ts index b50a00b..ca49b99 100644 --- a/lib/safe-operations.ts +++ b/lib/safe-operations.ts @@ -157,11 +157,21 @@ export async function withBoundedParallel( const results: SafeOperationResult[] = []; let index = 0; + // When the caller passes no outer signal, fall back to a single signal + // created once for the whole batch — not one per item. A fresh + // `new AbortController().signal` built inline per iteration is discarded + // the instant it's created: nothing holds the controller, so `.abort()` + // can never fire and every handler's signal reads `aborted: false` + // forever, silently defeating per-item cancellation (#221). One shared + // controller at least makes the signal a coherent, referenceable object + // — the same instance every item receives. + const fallbackSignal = options?.signal ?? new AbortController().signal; + const worker = async () => { while (index < items.length && !options?.signal?.aborted) { const i = index++; try { - const result = await handler(items[i]!, i, options?.signal ?? new AbortController().signal); + const result = await handler(items[i]!, i, fallbackSignal); results[i] = result; } catch (err) { results[i] = {