From 6c709e0e844c859db7955d084af1ac710c771710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Mon, 27 Jul 2026 19:40:55 -0300 Subject: [PATCH] Rename Extensions to RxExtensions, clarify FromEventBuffered's bufferSize risk Extensions read too generically for a class this specific to RxSharp's own combinators. RxExtensions in the existing RxSharp.Extras namespace keeps the same one-name-for-the- whole-surface benefit without the genericness. Also expanded the bufferSize doc comment on FromEventBuffered: the default of 1 is a real footgun for a caller filtering for a specific match among payloads that could plausibly arrive in the pre-subscribe gap this method exists to cover - a matching payload can be silently evicted by a later non-matching one before anyone subscribes. Found via a puppeteer-sharp code review on the two call sites doing exactly that (WaitForTargetAsync/WaitForFrameAsync), which are being fixed to pass a larger bufferSize as a follow-up in that repo. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s --- src/RxSharp/Extras/AssumeNeverEmits.cs | 4 ++-- src/RxSharp/Extras/FilterAsync.cs | 2 +- src/RxSharp/Extras/FromCancellationToken.cs | 2 +- src/RxSharp/Extras/FromEventBuffered.cs | 22 +++++++++++++++---- src/RxSharp/Extras/RaceWithSignalAndTimer.cs | 10 ++++----- .../Extras/RetryAndRaceWithSignalAndTimer.cs | 6 ++--- src/RxSharp/Extras/Timeout.cs | 2 +- src/RxSharp/RxSharp.csproj | 6 ++--- .../Extras/CancellationExtrasTests.cs | 8 +++---- .../Extras/FromEventBufferedExtrasTests.cs | 12 +++++----- .../Extras/TimeoutExtrasTests.cs | 6 ++--- 11 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/RxSharp/Extras/AssumeNeverEmits.cs b/src/RxSharp/Extras/AssumeNeverEmits.cs index 6727380..e5de4f0 100644 --- a/src/RxSharp/Extras/AssumeNeverEmits.cs +++ b/src/RxSharp/Extras/AssumeNeverEmits.cs @@ -3,11 +3,11 @@ namespace RxSharp.Extras; /// Extension methods widening an error-only to another element type. -public static partial class Extensions +public static partial class RxExtensions { /// /// Widens an that only ever calls (such as - /// or ) so it + /// or ) so it /// type-checks anywhere an is expected - most commonly as one of the branches /// passed to alongside a source that actually produces /// values. Mirrors how rxjs relies on TypeScript structurally accepting diff --git a/src/RxSharp/Extras/FilterAsync.cs b/src/RxSharp/Extras/FilterAsync.cs index e789517..df46de6 100644 --- a/src/RxSharp/Extras/FilterAsync.cs +++ b/src/RxSharp/Extras/FilterAsync.cs @@ -3,7 +3,7 @@ namespace RxSharp.Extras; /// Extension methods providing an async-predicate flavor of Filter. -public static partial class Extensions +public static partial class RxExtensions { /// /// An operator supporting an async predicate, mirroring Puppeteer's own filterAsync helper (implemented, diff --git a/src/RxSharp/Extras/FromCancellationToken.cs b/src/RxSharp/Extras/FromCancellationToken.cs index 7c3a587..c058642 100644 --- a/src/RxSharp/Extras/FromCancellationToken.cs +++ b/src/RxSharp/Extras/FromCancellationToken.cs @@ -1,7 +1,7 @@ namespace RxSharp.Extras; /// Puppeteer-flavored combinators built on top of the core primitives — the C# analogues of the helpers Puppeteer itself layers on top of rxjs (see CLAUDE.md's "Puppeteer-essential surface"). -public static partial class Extensions +public static partial class RxExtensions { /// /// An observable that never emits and errors as soon as is cancelled. diff --git a/src/RxSharp/Extras/FromEventBuffered.cs b/src/RxSharp/Extras/FromEventBuffered.cs index 7a7dad4..caf3aa0 100644 --- a/src/RxSharp/Extras/FromEventBuffered.cs +++ b/src/RxSharp/Extras/FromEventBuffered.cs @@ -4,7 +4,7 @@ namespace RxSharp.Extras; /// /// A handle to an eagerly-attached, buffered .NET event source created by -/// . Disposing detaches the underlying event handler. +/// . Disposing detaches the underlying event handler. /// /// The type of the event's payload. public sealed class BufferedEventSource : IDisposable @@ -42,7 +42,7 @@ public void Dispose() } /// Extension methods providing an eagerly-attached, buffered variant of . -public static partial class Extensions +public static partial class RxExtensions { /// /// Like , but attaches the underlying event handler @@ -68,7 +68,14 @@ public static partial class Extensions /// Called immediately, with the handler to add to the event. /// Called on , with the same handler, to remove it from the event. /// Converts an callback into the event's actual delegate shape. - /// The maximum number of most-recent payloads kept for replay. Defaults to 1. + /// + /// The maximum number of most-recent payloads kept for replay. Defaults to 1 - fine if the caller only + /// cares about the single most recent payload, but a real risk if the caller filters for a specific match + /// among possibly-several payloads that could arrive in the pre-subscribe gap this method exists to cover: + /// with the default of 1, a matching payload can be silently evicted from the buffer by a later + /// non-matching one before anyone subscribes to see it. Pass a larger value whenever more than one payload + /// could plausibly arrive before subscription and only some of them are what the caller is looking for. + /// /// A handle exposing the buffered payloads as an observable, and detaching the handler on disposal. public static BufferedEventSource FromEventBuffered( Action addHandler, @@ -86,7 +93,14 @@ public static BufferedEventSource FromEventBufferedThe type of the event's payload. /// Called immediately, with the handler to add to the event. /// Called on , with the same handler, to remove it from the event. - /// The maximum number of most-recent payloads kept for replay. Defaults to 1. + /// + /// The maximum number of most-recent payloads kept for replay. Defaults to 1 - fine if the caller only + /// cares about the single most recent payload, but a real risk if the caller filters for a specific match + /// among possibly-several payloads that could arrive in the pre-subscribe gap this method exists to cover: + /// with the default of 1, a matching payload can be silently evicted from the buffer by a later + /// non-matching one before anyone subscribes to see it. Pass a larger value whenever more than one payload + /// could plausibly arrive before subscription and only some of them are what the caller is looking for. + /// /// A handle exposing the buffered payloads as an observable, and detaching the handler on disposal. public static BufferedEventSource FromEventBuffered( Action> addHandler, diff --git a/src/RxSharp/Extras/RaceWithSignalAndTimer.cs b/src/RxSharp/Extras/RaceWithSignalAndTimer.cs index 3478f77..6253f0f 100644 --- a/src/RxSharp/Extras/RaceWithSignalAndTimer.cs +++ b/src/RxSharp/Extras/RaceWithSignalAndTimer.cs @@ -3,11 +3,11 @@ namespace RxSharp.Extras; /// Extension methods racing a single subscription against cancellation and a timeout. -public static partial class Extensions +public static partial class RxExtensions { /// /// Races against cancellation and a timeout, whichever fires first. The - /// non-retrying half of + /// non-retrying half of /// - use this directly for a single wait (e.g. "wait for the next matching event") that doesn't need /// retrying, and reach for the retrying combinator when it does. /// @@ -17,7 +17,7 @@ public static partial class Extensions /// /// Produces the exception used for both the cancellation and timeout branches. Defaults to /// for cancellation and for the timeout, - /// via the defaults of and respectively. + /// via the defaults of and respectively. /// Since one factory covers both branches, a caller needing to tell the two apart by exception type should /// pass here (so each branch keeps its own distinct default type) and catch/rethrow /// as needed at the call site. @@ -30,8 +30,8 @@ public static Observable RaceWithSignalAndTimer( Func? causeFactory, CancellationToken cancellationToken) => source.RaceWith( - Extensions.FromCancellationToken(cancellationToken, causeFactory).AssumeNeverEmits(), - Extensions.Timeout(timeout, causeFactory).AssumeNeverEmits()); + RxExtensions.FromCancellationToken(cancellationToken, causeFactory).AssumeNeverEmits(), + RxExtensions.Timeout(timeout, causeFactory).AssumeNeverEmits()); /// /// Overload of diff --git a/src/RxSharp/Extras/RetryAndRaceWithSignalAndTimer.cs b/src/RxSharp/Extras/RetryAndRaceWithSignalAndTimer.cs index 79e95d1..1cc2dd9 100644 --- a/src/RxSharp/Extras/RetryAndRaceWithSignalAndTimer.cs +++ b/src/RxSharp/Extras/RetryAndRaceWithSignalAndTimer.cs @@ -3,7 +3,7 @@ namespace RxSharp.Extras; /// Extension methods composing retry, cancellation, and timeout into the single combinator Puppeteer's Locator actions rely on. -public static partial class Extensions +public static partial class RxExtensions { /// /// The combinator behind Puppeteer's Locator actions (click/fill/hover/wait): retry the source @@ -13,7 +13,7 @@ public static partial class Extensions /// that may not have rendered yet") while still giving up promptly, either because the caller cancelled it /// or because it took longer than — whichever happens first. Since the cancellation /// and timeout branches are sources that only ever error (see - /// and ), the only way + /// and ), the only way /// this combinator produces a value is if the retried itself emits one before either /// branch fires. /// @@ -23,7 +23,7 @@ public static partial class Extensions /// /// Produces the exception used for both the cancellation and timeout branches. Defaults to /// for cancellation and for the timeout, - /// via the defaults of and respectively. + /// via the defaults of and respectively. /// /// The delay between retry attempts. Defaults to 50 milliseconds. /// A token that, when cancelled, aborts the whole operation immediately. diff --git a/src/RxSharp/Extras/Timeout.cs b/src/RxSharp/Extras/Timeout.cs index d8dce67..b692bd7 100644 --- a/src/RxSharp/Extras/Timeout.cs +++ b/src/RxSharp/Extras/Timeout.cs @@ -3,7 +3,7 @@ namespace RxSharp.Extras; /// Extension methods providing a standalone, -throwing timer observable. -public static partial class Extensions +public static partial class RxExtensions { /// /// An observable that never emits and errors after elapses, or never errors at all if diff --git a/src/RxSharp/RxSharp.csproj b/src/RxSharp/RxSharp.csproj index 21e79bc..e3bf595 100644 --- a/src/RxSharp/RxSharp.csproj +++ b/src/RxSharp/RxSharp.csproj @@ -16,9 +16,9 @@ https://github.com/hardkoded/ReactiveExtensions-Sharp https://github.com/hardkoded/ReactiveExtensions-Sharp README.md - 0.1.2 - 0.1.2.0 - 0.1.2.0 + 0.1.3 + 0.1.3.0 + 0.1.3.0 diff --git a/test/RxSharp.Tests/Extras/CancellationExtrasTests.cs b/test/RxSharp.Tests/Extras/CancellationExtrasTests.cs index 2611d4f..052be97 100644 --- a/test/RxSharp.Tests/Extras/CancellationExtrasTests.cs +++ b/test/RxSharp.Tests/Extras/CancellationExtrasTests.cs @@ -12,7 +12,7 @@ public void ShouldErrorImmediatelyIfTokenIsAlreadyCancelled() cts.Cancel(); Exception? received = null; - Extensions.FromCancellationToken(cts.Token).Subscribe(onError: err => received = err); + RxExtensions.FromCancellationToken(cts.Token).Subscribe(onError: err => received = err); Assert.That(received, Is.InstanceOf()); } @@ -22,7 +22,7 @@ public void ShouldErrorAsSoonAsTheTokenIsCancelled() { using var cts = new CancellationTokenSource(); Exception? received = null; - Extensions.FromCancellationToken(cts.Token).Subscribe(onError: err => received = err); + RxExtensions.FromCancellationToken(cts.Token).Subscribe(onError: err => received = err); Assert.That(received, Is.Null); @@ -36,7 +36,7 @@ public void ShouldNeverEmitAValue() { using var cts = new CancellationTokenSource(); var nextCalled = false; - var subscription = Extensions.FromCancellationToken(cts.Token).Subscribe(_ => nextCalled = true); + var subscription = RxExtensions.FromCancellationToken(cts.Token).Subscribe(_ => nextCalled = true); subscription.Dispose(); cts.Cancel(); @@ -52,7 +52,7 @@ public void ShouldUseTheCustomCauseFactory() var cause = new InvalidOperationException("custom cause"); Exception? received = null; - Extensions.FromCancellationToken(cts.Token, () => cause).Subscribe(onError: err => received = err); + RxExtensions.FromCancellationToken(cts.Token, () => cause).Subscribe(onError: err => received = err); Assert.That(received, Is.SameAs(cause)); } diff --git a/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs b/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs index 0a68745..5aa8491 100644 --- a/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs +++ b/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs @@ -17,7 +17,7 @@ public void ShouldAttachTheHandlerImmediatelyRatherThanAtSubscribeTime() { var manager = new FakeTargetManager(); - using var source = Extensions.FromEventBuffered( + using var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h); @@ -35,7 +35,7 @@ public void ShouldAttachTheHandlerImmediatelyRatherThanAtSubscribeTime() public void ShouldReplayBufferedValuesToALateSubscriberThenDeliverLiveValues() { var manager = new FakeTargetManager(); - using var source = Extensions.FromEventBuffered( + using var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h); @@ -52,7 +52,7 @@ public void ShouldReplayBufferedValuesToALateSubscriberThenDeliverLiveValues() public void ShouldDeliverToEveryIndependentSubscriber() { var manager = new FakeTargetManager(); - using var source = Extensions.FromEventBuffered( + using var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h); @@ -71,7 +71,7 @@ public void ShouldDeliverToEveryIndependentSubscriber() public void ShouldOnlyReplayUpToTheRequestedBufferSize() { var manager = new FakeTargetManager(); - using var source = Extensions.FromEventBuffered( + using var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h, bufferSize: 2); @@ -90,7 +90,7 @@ public void ShouldOnlyReplayUpToTheRequestedBufferSize() public void ShouldDetachTheHandlerOnDispose() { var manager = new FakeTargetManager(); - var source = Extensions.FromEventBuffered( + var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h); @@ -106,7 +106,7 @@ public void ShouldDetachTheHandlerOnDispose() public void ShouldBeSafeToDisposeMoreThanOnce() { var manager = new FakeTargetManager(); - var source = Extensions.FromEventBuffered( + var source = RxExtensions.FromEventBuffered( h => manager.TargetCreated += h, h => manager.TargetCreated -= h); diff --git a/test/RxSharp.Tests/Extras/TimeoutExtrasTests.cs b/test/RxSharp.Tests/Extras/TimeoutExtrasTests.cs index 625f46a..28e0edb 100644 --- a/test/RxSharp.Tests/Extras/TimeoutExtrasTests.cs +++ b/test/RxSharp.Tests/Extras/TimeoutExtrasTests.cs @@ -11,7 +11,7 @@ public void ShouldErrorAfterTheDelay() using var signal = new ManualResetEventSlim(); Exception? received = null; - Extensions.Timeout(TimeSpan.FromMilliseconds(20)).Subscribe(onError: err => + RxExtensions.Timeout(TimeSpan.FromMilliseconds(20)).Subscribe(onError: err => { received = err; signal.Set(); @@ -25,7 +25,7 @@ public void ShouldErrorAfterTheDelay() public void ShouldNeverErrorWhenDelayIsZero() { var errored = false; - Extensions.Timeout(TimeSpan.Zero).Subscribe(onError: _ => errored = true); + RxExtensions.Timeout(TimeSpan.Zero).Subscribe(onError: _ => errored = true); Thread.Sleep(50); @@ -39,7 +39,7 @@ public void ShouldUseTheCustomCauseFactory() var cause = new InvalidOperationException("custom timeout cause"); Exception? received = null; - Extensions.Timeout(TimeSpan.FromMilliseconds(10), () => cause).Subscribe(onError: err => + RxExtensions.Timeout(TimeSpan.FromMilliseconds(10), () => cause).Subscribe(onError: err => { received = err; signal.Set();