Never let one property getter lose a whole OptionsDescription (#590) - #591
Merged
Conversation
This was referenced Jul 30, 2026
jeremydmiller
added a commit
that referenced
this pull request
Jul 31, 2026
…wn (#595) WaitForCompletionAsync read and posted the trailing partial batch WITHOUT taking _syncLock, and never cleared _current afterward -- while the flush timer armed by that batch's first item was still live. A timer firing at the same moment ran TriggerBatch, which posts AND clears, so both paths shipped the same items and the downstream block saw the batch twice. Found as the intermittent CI failure that has been reddening unrelated PRs (most recently #591): BatchingChannelTests.steady_trickle_faster_than_the_timeout_ still_flushes_within_the_max_age on net9.0, asserting [0..19] and getting [0..10, 10, 11, 11, ... 19, 19]. The duplicate values -- rather than missing or late ones -- are what identify this as a real double-delivery rather than a timing-sensitive test. The drain now happens under _syncLock, disarms the timer first, and clears _current, so exactly one of the two paths can ship any given item. A timer callback already parked on the lock is harmless: whichever side wins clears the buffer and the other sees it empty. The downstream post stays outside the lock -- _syncLock guards the buffer, never an await. Regression test drives completion into the timer's window 50 times over; it fails within ~25 attempts against the old code and passes against the new. This is a product bug on the sender-batching path, not only a test fix. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
) An OptionsDescription is a diagnostic view built by calling arbitrary property getters on somebody else's configuration object, so any one of them can throw -- and losing the entire description over one bad property is a terrible trade. Reported from the field as JasperFx/wolverine#3740, where AzureServiceBusTransport.HostName threw a NullReferenceException for credential-based connections and a monitored service could consequently never build its ServiceCapabilities snapshot at all. - Skip set-only properties and indexers, neither of which can be read via PropertyInfo.GetValue(subject) (ArgumentException / TargetParameterCountException). Wolverine.Pulsar's PulsarTransport has a this[Uri] indexer, which was enough to make it undescribable. - Catch whatever a getter throws -- including from the [ChildDescription], [DescribeAsStringArray] and [DescribeAsConfigurationState] branches -- and record OptionsValue.Unreadable(...) in its place. - Report the exception TYPE only, never the message: descriptions get shipped to monitoring consoles and work hard to keep secrets out, and exception messages habitually quote the offending configuration value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RainvDHdde5JUAiGEPEgWw
jeremydmiller
force-pushed
the
gh/harden-options-description
branch
from
July 31, 2026 15:56
7daa4ff to
984c5c2
Compare
This was referenced Aug 3, 2026
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.
Closes #590.
OptionsDescription(object subject)builds a diagnostic view by calling arbitrary property getters on somebody else's configuration object. Any one of those getters can throw, and until now that lost the entire description — along with everything built on top of it.Reported from the field as JasperFx/wolverine#3740:
AzureServiceBusTransport.HostNamethrew aNullReferenceExceptionfor credential-based (managed identity) connections, so a monitored Wolverine service could never build itsServiceCapabilitiessnapshot, so CritterWatch never completed a handshake for it — permanently, since the same doomed read is retried on every batch.What changed
ArgumentException) and indexers (GetValuewith no index arguments →TargetParameterCountException). Both are legal on a described type and neither is configuration data.Wolverine.Pulsar'sPulsarTransporthas athis[Uri]indexer, which alone was enough to make it undescribable.[ChildDescription],[DescribeAsStringArray]and[DescribeAsConfigurationState]branches — and recordOptionsValue.Unreadable(...)in its place, withValue="Could not be read -- <ExceptionTypeName>".The per-property body moved into a
readProperty(...)helper so the try/catch wraps every branch, with no behavior change for properties that read cleanly.Tests
CoreTests/Descriptors/OptionsDescriptionTests.csgains adescribing_awkward_propertiesfixture: a throwing getter, a throwing[ChildDescription], an indexer, a set-only property, secret-in-the-exception-message, and still-serializable. FullCoreTests(485) andCommandLineTests(295) green locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01RainvDHdde5JUAiGEPEgWw