Fix GetValue<object>() and TryGetValue<JsonElement?> on JsonValueOfJsonPrimitive types by Copilot · Pull Request #125139 · dotnet/runtime

Copilot · 2026-03-03T18:00:32Z

Description

JsonValueOfJsonString, JsonValueOfJsonBool, and JsonValueOfJsonNumber (introduced in .NET 10) throw InvalidOperationException on GetValue<object>() because their TryGetValue<T> methods don't handle typeof(T) == typeof(object). The older JsonValueOfElement handled this implicitly via if (Value is TypeToConvert element).

Additionally, TryGetValue<JsonElement?>() was broken on all three types because they use explicit typeof(T) == typeof(JsonElement) checks which don't match typeof(JsonElement?). The sibling JsonValueOfElement handles this naturally via Value is TypeToConvert pattern matching, but the new types needed explicit typeof(T) == typeof(JsonElement?) checks.

var reader = new Utf8JsonReader("\"Hello World!\""u8);
reader.Read();
var node = converter.Read(ref reader, typeof(JsonNode), options);

// .NET 10: throws InvalidOperationException
// .NET 9:  works fine
var o = node.GetValue<object>();

Changes

All three types (JsonValueOfJsonString, JsonValueOfJsonBool, JsonValueOfJsonNumber): Added typeof(T) == typeof(object) and typeof(T) == typeof(JsonElement?) to the JsonElement branch of TryGetValue<T>. This matches JsonValueOfElement's behavior where Value is TypeToConvert (line 54) naturally handles JsonElement, JsonElement?, and object by boxing the JsonElement value. GetValue<object>() now returns JsonElement uniformly across all types for backward compatibility.
Added TryGetValue<object> assertions to existing TryGetValue_From* tests and a new GetValue_Object theory exercising the converter code path — asserts JsonElement return type for all primitive kinds
Added TryGetValue_NullableTypes_Deserialized test covering JsonElement? on all three types, bool? on JsonValueOfJsonBool, and all numeric nullable types on JsonValueOfJsonNumber

Original prompt

This section details on the original issue you should resolve

<issue_title>Breaking change in JsonNodeConverter</issue_title>
<issue_description>### Description

Using a JsonConverter to read a string node, .NET 9 and .NET 10 show different behavior.

The new System.Text.Json.Nodes.JsonValueOfJsonString introduced in .NET 10 cannot be converted to an object using GetValue<object>(), resulting in an exception.

Reproduction Steps

See Repo at https://github.com/NiceWaffel/jsonnodeconverter-repro
var reader = new Utf8JsonReader("\"Hello World!\""u8);
reader.Read();
var node = (SerializationContext.Default.JsonNode.Converter as JsonConverter<JsonNode>)
  ?.Read(ref reader, typeof(JsonNode), SerializationContext.Default.Options);

Debug.Assert(node != null, "We should get a valid JsonNode.");

// Different type in .NET 8, .NET 9 and .NET 10
// .NET 8: System.Text.Json.Nodes.JsonValuePrimitive`1[System.Text.Json.JsonElement]
// .NET 9: System.Text.Json.Nodes.JsonValueOfElement
// .NET 10: System.Text.Json.Nodes.JsonValueOfJsonString
Console.WriteLine(node.GetType());

// Exception in .NET 10 (no exception in .NET 8)
// System.InvalidOperationException: An element of type 'String' cannot be converted to a 'System.Object'.
var o = node.GetValue<object>();
Console.WriteLine(o);
Expected behavior

The behavior in .NET 8, .NET 9 and .Net 10 should be the same. No exception should be thrown.

Actual behavior

In .NET 10, a string JsonNode (read as JsonValueOfJsonString) cannot be converted to an object, resulting in an exception.
Unhandled exception. System.InvalidOperationException: An element of type 'String' cannot be converted to a 'System.Object'.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_NodeUnableToConvertElement(JsonValueKind valueKind, Type destinationType)
   at System.Text.Json.Nodes.JsonValueOfJsonString.GetValue[T]()
Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

JsonValueOfJsonString was introduced with #116798</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Breaking change in JsonNodeConverter #123372

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

dotnet-policy-service · 2026-03-03T18:08:26Z

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Handle typeof(T) == typeof(object) in TryGetValue for JsonValueOfJsonString, JsonValueOfJsonBool, and JsonValueOfJsonNumber. This fixes InvalidOperationException when calling GetValue<object>() on these types that were introduced in .NET 10. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>

eiriktsarpalis

@copilot @PranavSenthilnathan Based on the changes made in #116798 are there any further variants we should be considering?

Copilot · 2026-03-04T14:42:58Z

@copilot @PranavSenthilnathan Based on the changes made in #116798 are there any further variants we should be considering?

The three types fixed here — JsonValueOfJsonString, JsonValueOfJsonBool, JsonValueOfJsonNumber — are the only ones affected. They're the only JsonValue subclasses that directly extend JsonValue (not JsonValue<TValue>) and override TryGetValue<T> with manual type checks.

The other subclasses are not affected:

JsonValuePrimitive<TValue> and JsonValueCustomized<TValue> don't override TryGetValue; they inherit JsonValue<TValue>.TryGetValue which uses if (Value is T returnValue) — this naturally handles object via boxing.
JsonValueOfElement also uses if (Value is TypeToConvert element) as its first check, so JsonElement boxes to object successfully.

No further variants to consider.

Copilot

Pull request overview

Fixes a regression in System.Text.Json.Nodes where JsonValueOfJsonString/Bool/Number (used by JsonValueConverter/JsonNodeConverter for primitives) throw on GetValue<object>() by adding explicit typeof(T) == typeof(object) handling in their TryGetValue<T> implementations.

Changes:

Add object handling in TryGetValue<T> for JsonValueOfJsonString, JsonValueOfJsonBool, and JsonValueOfJsonNumber.
Extend existing TryGetValue_From* tests with TryGetValue<object> assertions.
Add a new GetValue_Object theory validating GetValue<object>() for deserialized primitive JsonValues.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs	Adds explicit `object` support in `TryGetValue<T>` for optimized primitive-backed `JsonValue` implementations.
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs	Adds regression tests covering `TryGetValue<object>` and `GetValue<object>` for primitives.

Comments suppressed due to low confidence (2)

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs:82

TryGetValue() for string values now returns a decoded string rather than a boxed JsonElement. In .NET 9 (JsonValueOfElement) and in JsonNode.Parse(...) today, GetValue() yields a JsonElement, so this changes the observable runtime type and may be a compatibility break. Please confirm the intended contract for GetValue/TryGetValue() on deserialized primitives; if compatibility is desired, consider returning JsonElement for object here (and similarly for bool) or update the public docs accordingly.
```
            if (typeof(T) == typeof(string) || typeof(T) == typeof(object))
            {
                string? result = JsonReaderHelper.TranscodeHelper(_value.Span);

                Debug.Assert(result != null);
                value = (T)(object)result;
                return true;
            }
```
src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs:163
- TryGetValue() for booleans returns boxed bool, which differs from the historical JsonValueOfElement behavior (boxed JsonElement) and from JsonNode.Parse(...) today. If the goal is to preserve .NET 9 behavior for GetValue(), consider returning JsonElement for object here as well (or document that object returns CLR primitives for these optimized deserialized nodes).
```
            if (typeof(T) == typeof(bool) || typeof(T) == typeof(bool?) || typeof(T) == typeof(object))
            {
                value = (T)(object)_value;
                return true;
            }
```
  You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T14:43:57Z

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs

+        [Theory]
+        [InlineData("\"Hello World!\"", typeof(string))]
+        [InlineData("42", typeof(JsonElement))]
+        [InlineData("true", typeof(bool))]
+        [InlineData("false", typeof(bool))]
+        public static void GetValue_Object(string json, Type expectedType)
+        {
+            JsonValue jValue = JsonSerializer.Deserialize<JsonValue>(json);
+
+            object result = jValue.GetValue<object>();
+            Assert.NotNull(result);
+            Assert.IsType(expectedType, result);
+        }


The new GetValue_Object theory asserts that GetValue() returns different runtime types per primitive (string/bool return CLR primitives, number returns JsonElement). This bakes in a potentially breaking behavioral change vs .NET 9’s JsonValueOfElement (where object yielded JsonElement for all primitives) and vs JsonNode.Parse(...) today. Please confirm the intended API behavior and adjust expectations/docs to avoid inconsistent results depending on how the node was created.

stephentoub · 2026-03-04T14:46:01Z

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs

Copilot · 2026-03-04T14:54:51Z

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

🤖 Copilot Code Review — PR #125476

Holistic Assessment

Motivation: The regression is real and well-documented — GetValue<object>() throws InvalidOperationException on the three JsonValueOfJsonPrimitive types introduced in .NET 10 (PR #116798). The documented contract for TryGetValue (see JsonValue.cs line 28) explicitly states "Specifying the object type for {T} will always succeed."

Approach: Adding typeof(T) == typeof(object) checks to each TryGetValue<T> override is the correct minimal fix. The only question is what value to return for the object case, which has implications for backward compatibility.

Summary: ⚠️ Needs Human Review. The fix is correct for the stated regression (no more exception), but there is a behavioral inconsistency worth a maintainer decision: GetValue<object>() now returns different types depending on how the node was created (Parse path vs Deserialize path). Details below.

Detailed Findings

⚠️ Behavioral inconsistency — `GetValue<object>()` returns different types depending on creation path

In .NET 9, both JsonNode.Parse("\"hello\"") and JsonSerializer.Deserialize<JsonValue>("\"hello\"") produced JsonValueOfElement, so GetValue<object>() returned a boxed JsonElement in all cases.

With this fix:

JsonNode.Parse("\"hello\"").GetValue<object>() → boxed JsonElement (via JsonValueOfElement, unchanged)
JsonSerializer.Deserialize<JsonValue>("\"hello\"").GetValue<object>() → string (via JsonValueOfJsonString, new behavior)

Similarly for booleans (bool vs JsonElement). Numbers are consistent (both return JsonElement).

This asymmetry is a deliberate choice by the PR (stated in the description), and arguably more useful. But it means code migrating from .NET 9 that does node.GetValue<object>() is JsonElement would get true with Parse-created nodes but false with Deserialize-created nodes.

Alternative: For maximum backward compatibility, all three types could return JsonElement for typeof(T) == typeof(object) by placing the object check in the JsonElement branch. This would match JsonValueOfElement's behavior. The trade-off is returning a less natural type.

This is a design decision for the maintainers.

⚠️ Stale documentation on `TryGetValue`

JsonValue.cs line 29 says: "The underlying value of a JsonValue after deserialization is an instance of JsonElement." This was accurate before PR #116798 but is now stale — deserialized primitives are backed by JsonValueOfJsonString/Bool/Number, not JsonElement. With this PR's fix, GetValue<object>() returns string/bool (not JsonElement) for string/bool values created via deserialization. The doc should be updated or marked as a follow-up.

💡 Test coverage — `TryGetValue_From*` assertions test `JsonValueOfElement`, not the fix

The three new Assert.True(jValue.TryGetValue(out object _)) lines added to TryGetValue_FromString, TryGetValue_FromNumber, and TryGetValue_FromBoolean use JsonNode.Parse(...), which creates JsonValueOfElement nodes. They do not exercise the JsonValueOfJsonPrimitive types this PR fixes. These assertions test pre-existing working behavior.

Only the GetValue_Object theory (which uses JsonSerializer.Deserialize<JsonValue>) actually tests the fix. The assertions aren't wrong — they're useful documentation of expected behavior — but they don't provide coverage for the regression.

Consider adding a parallel TryGetValue_From*_Deserialized variant or extending GetValue_Object to also assert TryGetValue<object> to make the coverage more explicit.

✅ Correctness — fix is sound for the three affected types

The three types (JsonValueOfJsonString, JsonValueOfJsonBool, JsonValueOfJsonNumber) are the only JsonValue subclasses that directly extend JsonValue (not JsonValue<TValue>) and override TryGetValue<T> with manual type checks. The other subclasses (JsonValuePrimitive<T>, JsonValueCustomized<T>, JsonValueOfElement) handle object correctly through Value is T pattern matching or the JsonValue<TValue> base class. No further variants are needed.

✅ No public API surface changes

No changes to ref/ assembly files. The fix is purely internal behavioral correction.

… and add test coverage Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>

PranavSenthilnathan · 2026-03-04T22:33:48Z

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs

            where T : default
        {
-            if (typeof(T) == typeof(JsonElement))
+            if (typeof(T) == typeof(JsonElement) || typeof(T) == typeof(JsonElement?))


I think the object type check should also be part of this branch and return a JsonElement instead of a string. This keeps backwards compatibility and it allows the users a uniform way across numbers/strings/bools to check the JSON type (using JsonElement.ValueKind) before using it.

For reference, this check was meant to behave the same as

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfElement.cs

Line 54 in 14c4360

if (Value is TypeToConvert element)

In that case Value is a JsonElement, so the expression would be true when TypeToConvert was JsonElement, JsonElement? or object. However, when I initially wrote the code here for JsonValueOfJsonPrimitive, I missed JsonElement? and object. So if we want to keep the behavior, we should just add the checks here and return JsonElement in those cases too.

@copilot please address the above.

Done in 429b4b0 — moved the object check into the JsonElement branch for all three types, so GetValue<object>() now returns JsonElement uniformly, matching JsonValueOfElement's behavior via Value is TypeToConvert. The GetValue_Object test now asserts Assert.IsType<JsonElement>(result) for all cases (string, number, bool).

@PranavSenthilnathan

Per reviewer feedback from @PranavSenthilnathan: GetValue<object>() should return JsonElement (not string/bool) for all three JsonValueOfJsonPrimitive types, matching the behavior of JsonValueOfElement where `Value is TypeToConvert` returned JsonElement for object. Updated tests accordingly. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>

Copilot · 2026-03-05T15:51:14Z

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

🤖 Copilot Code Review — PR #125476 (updated after latest changes)

Holistic Assessment

Motivation: Real regression — GetValue<object>() throws InvalidOperationException on JsonValueOfJsonString/Bool/Number types introduced in .NET 10 (PR #116798). The TryGetValue contract (line 28 of JsonValue.cs) explicitly states "Specifying the object type for {T} will always succeed."

Approach: Adding typeof(T) == typeof(object) and typeof(T) == typeof(JsonElement?) checks to the JsonElement branch in each TryGetValue<T> override is the correct minimal fix. Returning JsonElement for object matches JsonValueOfElement's behavior where Value is TypeToConvert (line 54) boxes JsonElement to object.

Summary: ✅ LGTM. The fix is correct, backward-compatible, and well-tested. All three affected types now handle object, JsonElement, and JsonElement? identically to how JsonValueOfElement handles them. No other JsonValue subclasses are affected.

Detailed Findings

✅ Correctness — fix matches `JsonValueOfElement` behavior exactly

The three JsonElement branches now include typeof(T) == typeof(JsonElement?) and typeof(T) == typeof(object), which mirrors exactly what JsonValueOfElement.TryGetValue does at line 54 via if (Value is TypeToConvert element) — that pattern matches JsonElement, JsonElement?, and object (since JsonElement is a struct that boxes to object).

✅ No other variants affected

Verified: JsonValuePrimitive<TValue> and JsonValueCustomized<TValue> inherit JsonValue<TValue>.TryGetValue which uses if (Value is T returnValue) — handles object naturally. JsonValueOfElement uses the same pattern. Only these three types needed fixing.

✅ Test coverage

GetValue_Object theory covers GetValue<object>() returning JsonElement for all four primitive JSON types (string, number, true, false) via the deserialization path.
TryGetValue_NullableTypes_Deserialized covers JsonElement? and relevant nullable value types on deserialized primitives.
Existing TryGetValue_From* tests also assert TryGetValue<object> on JsonValueOfElement (Parse path).

💡 Stale doc comment on `TryGetValue` (follow-up)

JsonValue.cs line 29 says: "The underlying value of a JsonValue after deserialization is an instance of JsonElement." This was accurate before PR #116798 but is now stale since deserialized primitives are JsonValueOfJsonString/Bool/Number. With GetValue<object>() returning JsonElement for all types, the behavioral contract still holds, but the implementation detail described is incorrect. Could be updated as a follow-up.

✅ No public API surface changes

No changes to ref/ assembly files. Purely internal behavioral correction.

Initial plan

19e720f

Copilot AI assigned Copilot and eiriktsarpalis Mar 3, 2026

Copilot started work on behalf of eiriktsarpalis March 3, 2026 18:00 View session

jkotas added the area-System.Text.Json label Mar 3, 2026

Copilot AI changed the title ~~[WIP] Fix breaking change in JsonNodeConverter for .NET 10~~ Fix GetValue<object>() on JsonValueOfJsonPrimitive types Mar 3, 2026

Copilot finished work on behalf of eiriktsarpalis March 3, 2026 18:36

eiriktsarpalis marked this pull request as ready for review March 4, 2026 14:36

Copilot AI review requested due to automatic review settings March 4, 2026 14:36

eiriktsarpalis reviewed Mar 4, 2026

View reviewed changes

Copilot started reviewing on behalf of eiriktsarpalis March 4, 2026 14:38 View session

Copilot started work on behalf of eiriktsarpalis March 4, 2026 14:38 View session

eiriktsarpalis requested a review from PranavSenthilnathan March 4, 2026 14:38

Copilot finished work on behalf of eiriktsarpalis March 4, 2026 14:43

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Copilot started work on behalf of stephentoub March 4, 2026 14:46 View session

stephentoub reviewed Mar 4, 2026

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfJsonPrimitive.cs Outdated Show resolved Hide resolved

Copilot finished work on behalf of stephentoub March 4, 2026 14:55

Copilot started work on behalf of eiriktsarpalis March 4, 2026 15:38 View session

Add JsonElement? handling to TryGetValue in all three primitive types…

d20d0cf

… and add test coverage Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>

Copilot AI changed the title ~~Fix GetValue<object>() on JsonValueOfJsonPrimitive types~~ Fix GetValue<object>() and TryGetValue<JsonElement?> on JsonValueOfJsonPrimitive types Mar 4, 2026

Copilot finished work on behalf of eiriktsarpalis March 4, 2026 16:08

PranavSenthilnathan reviewed Mar 4, 2026

View reviewed changes

build-analysis bot mentioned this pull request Mar 4, 2026

Test failure: System.IO.Tests.File_Move_Tests.File_Move_SynchronizingObject #103940

Open

Copilot started work on behalf of eiriktsarpalis March 5, 2026 15:24 View session

Copilot finished work on behalf of eiriktsarpalis March 5, 2026 15:53

build-analysis bot mentioned this pull request Mar 5, 2026

Android WebSocket failure #121518

Open

PranavSenthilnathan approved these changes Mar 10, 2026

View reviewed changes

Merge branch 'main' into copilot/fix-breaking-change-jsonnodeconverter

c0fffe2

eiriktsarpalis enabled auto-merge (squash) March 10, 2026 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GetValue<object>() and TryGetValue<JsonElement?> on JsonValueOfJsonPrimitive types#125139

Fix GetValue<object>() and TryGetValue<JsonElement?> on JsonValueOfJsonPrimitive types#125139
Copilot wants to merge 5 commits intomainfrom
copilot/fix-breaking-change-jsonnodeconverter

Copilot AI commented Mar 3, 2026 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented Mar 3, 2026

Uh oh!

eiriktsarpalis left a comment

Uh oh!

Copilot AI commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

stephentoub commented Mar 4, 2026

Uh oh!

Uh oh!

Copilot AI commented Mar 4, 2026

Uh oh!

PranavSenthilnathan Mar 4, 2026

Uh oh!

eiriktsarpalis Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

Copilot AI commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Reproduction Steps

Expected behavior

Actual behavior

Regression?

Known Workarounds

Configuration

Other information

Comments on the Issue (you are @copilot in this section)

Uh oh!

dotnet-policy-service bot commented Mar 3, 2026

Uh oh!

eiriktsarpalis left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

stephentoub commented Mar 4, 2026

Uh oh!

Uh oh!

Copilot AI commented Mar 4, 2026

🤖 Copilot Code Review — PR #125476

Holistic Assessment

Detailed Findings

⚠️ Behavioral inconsistency — GetValue<object>() returns different types depending on creation path

⚠️ Stale documentation on TryGetValue

💡 Test coverage — TryGetValue_From* assertions test JsonValueOfElement, not the fix

✅ Correctness — fix is sound for the three affected types

✅ No public API surface changes

Uh oh!

PranavSenthilnathan Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

eiriktsarpalis Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 5, 2026

🤖 Copilot Code Review — PR #125476 (updated after latest changes)

Holistic Assessment

Detailed Findings

✅ Correctness — fix matches JsonValueOfElement behavior exactly

✅ No other variants affected

✅ Test coverage

💡 Stale doc comment on TryGetValue (follow-up)

✅ No public API surface changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Copilot AI commented Mar 3, 2026 •

edited

Loading

⚠️ Behavioral inconsistency — `GetValue<object>()` returns different types depending on creation path

⚠️ Stale documentation on `TryGetValue`

💡 Test coverage — `TryGetValue_From*` assertions test `JsonValueOfElement`, not the fix

✅ Correctness — fix matches `JsonValueOfElement` behavior exactly

💡 Stale doc comment on `TryGetValue` (follow-up)