Rename Extensions to RxExtensions, clarify FromEventBuffered's bufferSize risk - #2
Merged
Conversation
…Size 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s
kblok
added a commit
to hardkoded/puppeteer-sharp
that referenced
this pull request
Jul 27, 2026
Code review caught it: FromEventBuffered's default bufferSize of 1 means that if a matching event and a later non-matching event both land in the narrow gap between attaching the handler and the Rx pipeline actually subscribing, the size-1 buffer keeps only the non-matching one - silently dropping the match. The old TrySetResult-based implementation didn't have this risk (idempotent, first match always wins regardless of how many events fire before anyone awaits it). Fixed by passing an explicit EventBufferSize (16) instead of relying on the default - enough headroom to safely absorb a realistic burst in that gap without buffering unboundedly for the whole wait (once subscribed, live delivery is unaffected by buffer size regardless). Also bumps to RxSharp 0.1.3 (pending publish - see hardkoded/ReactiveExtensions-Sharp#2), which renames the consolidated Extras class to RxExtensions and documents this exact bufferSize risk for future callers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent changes, bundled since both landed the same day.
Rename:
Extensions→RxExtensions. Same class, same purpose (the single-name consolidation of all the Extras static classes from #1), just a less generic name.bufferSize clarification: a puppeteer-sharp code review caught that
FromEventBuffered's defaultbufferSize: 1is a real footgun — if a caller is 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 from the size-1 buffer by a later non-matching one before anyone subscribes to see it. Not a bug inFromEventBuffereditself (1 is a perfectly reasonable default for "I only care about the most recent value"), but worth spelling out loudly in the doc comment so the next caller doesn't hit it blind. puppeteer-sharp's own two call sites (WaitForTargetAsync/WaitForFrameAsync) are being fixed in a follow-up there to pass a largerbufferSize.All 839 tests still pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s