Implement the six maintainer decisions for 2.0 - #151
Merged
Merged
Conversation
- Exception disclosure: with IncludeExceptionDetails off, an unhandled exception (or a result that cannot be written) is -32603 with data:null; no type name, no message. Redaction happens at write time, after the error handler, which still sees the exception. ExceptionInfo.ForResponse returns null when details are off. - Sessions: the request path uses a non-creating TryGetSessionHandler that falls through to the master registry on a snapshot miss; unknown ids dispatch through one shared empty handler and answer -32601. Registration adds to the master before publishing the version. GetSessionHandler still creates for binders and Config. - Config: SetPreProcessHandler(sessionId, ...) and SetPostProcessHandler(sessionId, ...); SetBeforeProcessHandler stays as an obsolete alias; doc comments say the session-less setters are default session only. - JsonRpcService(bool autoBind); the AspNetCore binder always binds the resolved instance to its effective session. - Written commitments: CHANGELOG.md as the per-version record (NuGet release notes link to it), SECURITY.md, README versioning section. - Core description says no JSON library dependency; the session registry uses System.Collections.Concurrent.ConcurrentDictionary and the NonBlocking reference is dropped, after SessionRegistryBenchmarks showed unknown-id lookups and register/destroy cycles faster and dispatch unchanged. Tests cover redaction on all three serializers, the error handler seeing the exception, unknown sessions on both processors, registration racing requests, the per-session setters, autoBind and the host binding.
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
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.
Implements the six maintainer decisions that came out of the documentation review (Linear AUS-974 to AUS-979). All of them land before 2.0.0 is pushed to NuGet, so nothing here changes a published package.
What changes
Exception disclosure (AUS-974). With
Config.IncludeExceptionDetailsoff, an unhandled exception, or a result that cannot be written, is answered as-32603withdata: null. The type name and message are no longer sent. Redaction happens when the response is written, after the error handler ran, so a handler still sees theExceptionindataand can author what the client gets.ExceptionInfo.ForResponsereturns null when details are off. TheConvert.ToStringfallback in the envelope writer is covered too.Sessions on the request path (AUS-975).
Process/ProcessAsyncuse a new internalTryGetSessionHandlerthat never creates a session: last hit, then the thread's snapshot, then the master registry (so a registration that landed after the snapshot is found). An unknown id dispatches through one shared empty handler:-32601for calls, parse errors, batches and notifications unchanged, no allocation per request.GetSessionHandlerstill creates, and now adds to the master before bumping the version, so no thread can hold a current-looking snapshot that lacks the entry.Handler setters (AUS-976).
Config.SetPreProcessHandler(sessionId, …)andConfig.SetPostProcessHandler(sessionId, …);SetBeforeProcessHandler(sessionId, …)stays as an[Obsolete]alias. The session-less setters say "default session only" in their doc comments.JsonRpcService(bool autoBind)(AUS-977).base(false)binds nowhere. The AspNetCore binder always binds the resolved instance to its effective session; the skip for subclasses on the default session is gone.Written commitments (AUS-978).
CHANGELOG.md(2.0.0 marked unreleased, 1.3.0 recorded),SECURITY.md, the README "Versioning and support" section, and every csproj'sPackageReleaseNotesis a link to the changelog. Private vulnerability reporting is still disabled in the repository settings; SECURITY.md points at it, so please enable it before merging.Package description and dependency (AUS-979). The core description says "no JSON library dependency". The session registry uses
System.Collections.Concurrent.ConcurrentDictionaryand theNonBlockingreference is dropped, so the core has no dependencies on net8.0 and net10.0. Measured with the newSessionRegistryBenchmarks(quiet and with a background thread registering and destroying sessions),--job short:Tests
1156 tests pass on net8.0 and net10.0. New: redaction on all three serializers including result-serialization failures and the handler still seeing the exception; unknown sessions on both processors (calls, parse errors, batches, notifications) with the registry unchanged; registration after a miss and registration racing requests on four threads; the per-session setters and the alias;
autoBindwith explicit binding; abase(false)subclass bound only to the configured session by the host. Three existing tests that asserted the old disclosure now assertdata: nulland read the exception through an error handler or with details on.benchmarks/charts/render.py --checkandsite/build.pypass.