Skip to content

Stop intercepting Attach calls within StreamJsonRpc itself#1241

Merged
AArnott merged 1 commit intomainfrom
dev/andarno/fixInterceptor
Aug 8, 2025
Merged

Stop intercepting Attach calls within StreamJsonRpc itself#1241
AArnott merged 1 commit intomainfrom
dev/andarno/fixInterceptor

Conversation

@AArnott
Copy link
Copy Markdown
Member

@AArnott AArnott commented Aug 8, 2025

This was causing runtime regressions in existing callers that were using the older APIs.

The interceptor was designed for external assemblies. Not StreamJsonRpc itself.

This was causing runtime regressions in existing callers that were using the older APIs.
@AArnott AArnott added this to the v2.23 milestone Aug 8, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR disables interceptors within the StreamJsonRpc library itself to prevent runtime regressions in existing callers using older APIs. The change ensures that the library's own unsafe APIs are not automatically rewritten to call safe APIs, which could cause unexpected failures when callers aren't prepared for NativeAOT safety restrictions.

Key changes:

  • Disabled interceptors for the StreamJsonRpc library project itself
  • Added test to verify no interceptors are generated within the library
  • Made interceptor enablement conditional in analyzer targets

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/StreamJsonRpc/StreamJsonRpc.csproj Explicitly disables interceptors for the main library
src/AnalyzerUser.targets Makes interceptor enablement conditional to allow overrides
test/StreamJsonRpc.Tests/ArchitectureTests.cs Adds test to verify no interceptors exist in the assembly
test/StreamJsonRpc.Analyzer.Tests/ProxyGeneratorTests.cs Documents existing interceptor test scenarios

Comment thread test/StreamJsonRpc.Tests/ArchitectureTests.cs
Comment thread test/StreamJsonRpc.Tests/ArchitectureTests.cs
@AArnott AArnott enabled auto-merge August 8, 2025 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants