Skip to content

[http-client-csharp] Keep MRW builders for system types referenced by generated serialization - #11398

Closed
Wei Hu (live1206) wants to merge 1 commit into
microsoft:mainfrom
live1206:live1206/fix-mrw-system-object-model-buildable
Closed

[http-client-csharp] Keep MRW builders for system types referenced by generated serialization#11398
Wei Hu (live1206) wants to merge 1 commit into
microsoft:mainfrom
live1206:live1206/fix-mrw-system-object-model-buildable

Conversation

@live1206

Copy link
Copy Markdown
Contributor

Summary

ProviderReferenceMapAnalyzer.IsResolvableBuildableProvider() rejected every SystemObjectModelProvider, so ModelReaderWriterContextDefinition pruned [ModelReaderWriterBuildable(...)] entries for ARM framework types (ManagedServiceIdentity, ResponseError, SystemData, ...) while generated serialization still deserializes them via ModelReaderWriter.Read<T>(data, options, Context.Default). That path calls context.GetTypeBuilder(returnType) and threw InvalidOperationException: No ModelReaderWriterTypeBuilder found ... at runtime.

Regression from #11288 (89f388c40).

Change

A SystemObjectModelProvider is now considered resolvable for MRW context generation when its wrapped SystemType implements IPersistableModel<T> or IJsonModel<T>. External ModelProviders and providers removed by the reference map remain excluded.

Tests

  • SystemObjectModelProvidersSupportingMrwRemainBuildable — regression coverage; fails on main with the buildable attribute missing, passes with the fix.
  • SystemObjectModelProvidersWithoutMrwSupportAreNotBuildable — negative coverage so non-MRW system types stay excluded.

Validation

  • Microsoft.TypeSpec.Generator.ClientModel.Tests: 1515/1515 passed
  • Microsoft.TypeSpec.Generator.Tests: 1720/1720 passed

Fixes #11396.

SystemObjectModelProvider instances were unconditionally rejected by
ProviderReferenceMapAnalyzer.IsResolvableBuildableProvider, so the MRW
context dropped ModelReaderWriterBuildable entries for framework types
(e.g. ManagedServiceIdentity, ResponseError, SystemData) that generated
serialization code still reads through ModelReaderWriter.Read<T>(...,
Context.Default), causing runtime failures.

Keep such a provider resolvable when its wrapped SystemType implements
IPersistableModel<T> or IJsonModel<T>.

Fixes microsoft#11396.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e996d84-23b1-4051-b2ba-d0a08d41ca93
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Jul 27, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jul 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@11398

commit: 762bfd9

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@live1206

Copy link
Copy Markdown
Contributor Author

Closing this — further investigation shows the premise of #11396 does not hold, and this change would actually regress generated output.

Why this fix is wrong

Making SystemObjectModelProvider unconditionally resolvable would re-add ~154 [ModelReaderWriterBuildable(typeof(ResponseError))] attributes that the reference map removes correctly, because nothing in those libraries references the type.

Evidence

I classified every buildable-attribute removal across the 156 context files in the Azure SDK regeneration PR (Azure/azure-sdk-for-net#61331). Removals: 154x ResponseError plus 11 others. All fall into three benign buckets:

  1. Type genuinely unreferenced -> correct pruning. ResponseError in 153 libraries (Advisor, Avs, Billing, ...) appears only in the context file; ApiProfile, ProviderExtendedLocation, ProviderResourceType, ZoneMapping likewise in Azure.ResourceManager.Resources.Deployments.
  2. Already declared in a hand-written custom partial -> GetCustomizedBuildableTypes() dedup. MySql (ManagedServiceIdentity, ResponseError, SystemData), MongoCluster (SubResource), StandbyPool (WritableSubResource), OracleDatabase (ArmPlan). Attributes merge across partial declarations, so Context.Default.GetTypeBuilder still resolves.
  3. Referenced only through non-MRW paths. OperationStatusResult in HybridConnectivity and ManagedNetworkFabric is consumed via DeserializeXxx in customized code; ModelReaderWriter.Read<OperationStatusResult> count is 0 in both.

Decisive check: of the 154 libraries that lost ResponseError, only MySql actually calls ModelReaderWriter.Read<ResponseError> — and MySql's src/Custom/MySql/Models/AzureResourceManagerMySqlContext.cs explicitly declares all three removed types.

Counter-evidence that the pruning is precise: ManagedNetworkFabric keeps ResponseError (it is read via MRW) while losing OperationStatusResult (it is not). Its custom partial declares neither, so retention was decided purely on usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[http-client-csharp] MRW context pruning removes required ARM framework type builders

1 participant