fix(csharp): throw JsonException from generated converters on invalid JSON

[vivekkumarq]

Fixes #24345.

The post-loop validation in generated JsonConverter<T>.Read() threw ArgumentException when a required property was missing, and ArgumentNullException when a non-nullable property was null:

if (!name.IsSet)
    throw new ArgumentException("Property is required for class Pet.", nameof(name));

if (name.IsSet && name.Value == null)
    throw new ArgumentNullException(nameof(name), "Property is not nullable for class Pet.");

Both violate the System.Text.Json converter contract, with two practical consequences:

• System.Text.Json only enriches a JsonException with the JSON path of the failure, so for these two errors the path was lost and the caller got no indication of which element failed.
• Callers wrapping deserialization in catch (JsonException) did not catch them, so a malformed payload surfaced as an unhandled ArgumentException rather than a deserialization failure.

Change

Both now throw JsonException, naming the property in the message since JsonException carries no parameter name:

if (!name.IsSet)
    throw new JsonException("Property is required for class Pet: name.");

if (name.IsSet && name.Value == null)
    throw new JsonException("Property is not nullable for class Pet: name.");

Read() already documented <exception cref="JsonException">, so this also makes the generated documentation accurate.

What is deliberately unchanged

WriteProperties still throws ArgumentNullException. That check validates an object handed in by the caller during serialization, not JSON being parsed, so an argument exception is the correct choice there and the issue explicitly scopes the change to the Read() post-loop blocks. Generated models still carry those calls.

Samples

Regenerated with ./bin/generate-samples.sh ./bin/configs/csharp-generichost-*.yaml — all 50 generichost configs. 1688 files, 5888 insertions and 5888 deletions: every changed line is one of the two throws being replaced, with no other churn.

I do not have a .NET toolchain on this machine, so I have not compiled the regenerated samples locally; the change is confined to the two throw statements described above.

---

Summary by cubic

Generated C# converters now throw JsonException from JsonConverter<T>.Read() when a required property is missing or a non-nullable property is null, replacing the previous ArgumentException and ArgumentNullException. Fixes #24345.

Because System.Text.Json only adds the JSON path to JsonException, the old exceptions lost the failing element's location and escaped callers' catch (JsonException) blocks.

Deliberately unchanged

• WriteProperties still throws ArgumentNullException because it validates caller-supplied objects during serialization, not parsed JSON.

^{Written for commit f2b3e86. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 1688 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/ModelClient.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/ModelClient.cs:148">
P1: The template change (JsonConverter.mustache Read() post-loop) now emits `throw new JsonException("...")` instead of `throw new ArgumentException(...)/ArgumentNullException(...)`, but the corresponding codegen unit tests still assert the old text and will fail on CI. `testGenericHostInnerStringEnumUnknownHandlingPreservesNullBehavior` (CSharpClientCodegenTest.java lines 123-125) asserts `throw new ArgumentException("Property is required for class RequiredClass.", nameof(requiredNullableEnumString))`, and `testNumericEnumJsonConverterUsesNumericOperations` (lines 443-445) asserts the old ArgumentException/ArgumentNullException forms. The regenerated `RequiredClass.cs` (e.g. lines 898-961) now produces `throw new JsonException("Property is required for class RequiredClass: ...")`, so both `contains` assertions no longer match. Update these test expectations to the new JsonException format (with the `: <baseName>.` suffix).</violation>
</file>

<file name="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache">

<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache:299">
P3: When a schema property name contains `&` or another HTML-significant character, this `JsonException` reports a name different from the JSON payload. Render a C#-escaped raw `baseName` value instead of using default HTML escaping.</violation>
</file>

<sub>Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The template change (JsonConverter.mustache Read() post-loop) now emits throw new JsonException("...") instead of throw new ArgumentException(...)/ArgumentNullException(...), but the corresponding codegen unit tests still assert the old text and will fail on CI. testGenericHostInnerStringEnumUnknownHandlingPreservesNullBehavior (CSharpClientCodegenTest.java lines 123-125) asserts throw new ArgumentException("Property is required for class RequiredClass.", nameof(requiredNullableEnumString)), and testNumericEnumJsonConverterUsesNumericOperations (lines 443-445) asserts the old ArgumentException/ArgumentNullException forms. The regenerated RequiredClass.cs (e.g. lines 898-961) now produces throw new JsonException("Property is required for class RequiredClass: ..."), so both contains assertions no longer match. Update these test expectations to the new JsonException format (with the : <baseName>. suffix).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net10/FormModels/src/Org.OpenAPITools/Model/ModelClient.cs, line 148:

<comment>The template change (JsonConverter.mustache Read() post-loop) now emits `throw new JsonException("...")` instead of `throw new ArgumentException(...)/ArgumentNullException(...)`, but the corresponding codegen unit tests still assert the old text and will fail on CI. `testGenericHostInnerStringEnumUnknownHandlingPreservesNullBehavior` (CSharpClientCodegenTest.java lines 123-125) asserts `throw new ArgumentException("Property is required for class RequiredClass.", nameof(requiredNullableEnumString))`, and `testNumericEnumJsonConverterUsesNumericOperations` (lines 443-445) asserts the old ArgumentException/ArgumentNullException forms. The regenerated `RequiredClass.cs` (e.g. lines 898-961) now produces `throw new JsonException("Property is required for class RequiredClass: ...")`, so both `contains` assertions no longer match. Update these test expectations to the new JsonException format (with the `: <baseName>.` suffix).</comment>

<file context>
@@ -145,7 +145,7 @@ public override ModelClient Read(ref Utf8JsonReader utf8JsonReader, Type typeToC
 
             if (varClient.IsSet && varClient.Value == null)
-                throw new ArgumentNullException(nameof(varClient), "Property is not nullable for class ModelClient.");
+                throw new JsonException("Property is not nullable for class ModelClient: client.");
 
             return new ModelClient(varClient);
</file context>

[cubic-dev-ai]

P3: When a schema property name contains & or another HTML-significant character, this JsonException reports a name different from the JSON payload. Render a C#-escaped raw baseName value instead of using default HTML escaping.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache, line 299:

<comment>When a schema property name contains `&` or another HTML-significant character, this `JsonException` reports a name different from the JSON payload. Render a C#-escaped raw `baseName` value instead of using default HTML escaping.</comment>

<file context>
@@ -296,14 +296,14 @@
             {{#required}}
             if (!{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.IsSet)
-                throw new ArgumentException("Property is required for class {{classname}}.", nameof({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}));
+                throw new JsonException("Property is required for class {{classname}}: {{baseName}}.");
 
             {{/required}}
</file context>

[vivekkumarq]

CI caught two stale assertions in CSharpClientCodegenTest — fixed and pushed.

testGenericHostInnerStringEnumUnknownHandlingPreservesNullBehavior and testNumericEnumJsonConverterUsesNumericOperations both pinned the old exception text from the generated Read() validation:

"throw new ArgumentException(\"Property is required for class RequiredClass.\", nameof(requiredNullableEnumString));"

They now expect the JsonException form. Worth noting both tests already expected throw new JsonException(); from the in-loop null checks a few lines above, so these two assertions were the inconsistent ones rather than a deliberate contract.

Verified locally: CSharpClientCodegenTest is 12 run, 0 failures, 0 errors.

The Build .Net N projects jobs passed on the previous run, so the regenerated samples compile; only the Java-side expectations needed updating.

[wing328]

thanks for the PR

cc @devhl-labs @mandrean (2017/08) @shibayan (2020/02) @Blackclaws (2021/03) @lucamazzanti (2021/05)