Add PSJsonSerializerV2 experimental feature for ConvertTo-Json using System.Text.Json #26624
Conversation
…erV2 experimental feature
…izerV2 is enabled
Tests can be skipped or enabled based on an experimental feature. Search by Please add old tests we skip to the PR description since it is breaking changes. We should think will we accept them as is or try to fix. |
|
@yotsuda Please add a reference to previous PR (there are a lot of useful comments). Also I guess there are some related to the cmdlet issues. These references would be useful too. |
2ad540d to
264335e
Compare
| /// Otherwise: default is 2, max is 100. | ||
| /// </summary> | ||
| [Parameter] | ||
| [ValidateRange(0, 100)] |
There was a problem hiding this comment.
Is [ValidateRange(-1, int.MaxValue)] that we need for new cmdlet version? If so I suggest create new ConvertToJsonCommandV2 class and avoid all new validating extra code. In PR description you can point that only the attribute and serializer are changed. Also SystemTextJsonSerializer.cs content will be in the cmdlet file.
This will simplify converting the experimental feature in regular code and removing old code.
There was a problem hiding this comment.
Can you point a discussion where we said about -1 value? I'd expect we need range from 1 to int.MaxValue.
int.MaxValue is unlimit de-facto.
There was a problem hiding this comment.
Done! I created ConvertToJsonCommandV2 class with [Experimental(..., Show)] and kept V1 with [Experimental(..., Hide)].
For ValidateRange:
ValidateRange(0, 1000): The maximum is 1000 due toUtf8JsonWriter's hard limit (documented here). Starts from 0 for V1 compatibility.- The
-1for unlimited was my own idea, but I removed it sinceUtf8JsonWriterhas a hard limit of 1000.
Also merged SystemTextJsonSerializer.cs into the cmdlet file as suggested.
There was a problem hiding this comment.
@yotsuda Thanks!
I think it is better to put new cmdlet in new file. This way it will be easier for us to make sure that we are not dependent on NewtonSoft, and later it will be easier for us to delete the old code. But I don't insist on it strictly.
There was a problem hiding this comment.
Thank you for the guidance. Done in 505fa77.
There was a problem hiding this comment.
1000 due to Utf8JsonWriter's hard limit (documented here).
I think this code says the default max depth is 1000,
rather than the maximum depth being hard limited to 1000 JsonWriterOptions.cs
I think ValidateRange would prevent using the higher limit
Gets or sets the maximum depth allowed when writing JSON, with the default (i.e. 0) indicating a max depth of 1000.
There was a problem hiding this comment.
@ninmonkey You're right! Thanks for the correction. The 1000 is STJ's default maximum depth, not a hard limit. ValidateRange is what actually enforces the limit on the user side.
This PR has been closed in favor of #26637, where I've aligned the maximum depth with V1 (100) for consistency.
Thank you. I've updated the tests to use I verified that all existing |
Is it real problem? Have you taken the measurements? Are you aware of the real problems with the standard serializer? I think we should start with a standard serializer (I guess this halves size of the PR.). Later, if we receive feedback, we can consider adding a custom serializer. |
Changing the default depth beyond 2/3 is unfortunately dangerous, anything with a # Fine
Get-Item env:PSModulePath | ConvertTo-Json
# Also somewhat ok but there is a lot of data
Get-Item env:PSModulePath | ConvertTo-Json -Depth 4
# Will end but you start to see it struggling
Get-Item env:PSModulePath | ConvertTo-Json -Depth 5
# Hits OOM on Linux for me
Get-Item env:PSModulePath | ConvertTo-Json -Depth 6A more practical example would be something like |
|
@jborean93 Thanks for feedback! The purpose of this PR is to migrate to a modern .Net API but not fixing all problems and controversial situations. This is not possible, it will just block the work. @daxian-dbw What do you want to discuss in WG? The purpose of this PR is to study migration to the modern .Net API. It was approved earlier by PowerShell Committee #8393 (comment) and nearly :-) implemented in #11198. |
|
Yea I’ll definitely be down for migrating away from newtonsoft and exposing more APIs to make it more extensible than it is now. I’ve added a few to my yaml library that fan do things like transform values based on the type and other logic so would love to see that here! My comment was more just a warning about changing the default depth and how it is easy to cause an OOM by serializing to json with a depth larger than 4/5. Probably should be done as part of a separate issue/PR to talk through that more and figure out some nice ways to avoid the problem I mentioned. |
|
Thank you @iSazonov and @jborean93 for the detailed feedback! Changes in this commit
Why
|
|
@yotsuda Thanks for great summarization!
So we lost support of custom Json converters? Then we lose all the benefits of the new API. Is there a workaround that preserves these benefits? If we have to rewrite the entire standard serializer, then it's probably too expensive. Maybe it's better to ask in .Net Runtime repository. |
|
@iSazonov Thank you for the feedback. You were right to push back on my earlier analysis. CorrectionIn my previous comment, I stated that
I apologize for the misleading analysis. Experimental ImplementationI've created an alternative implementation using Source code: Note: V1 file is identical between PR and Experimental branches. Implementation Comparison
(*) New Feature:
|
| Aspect | V1 | PR V2 | Exp V2 (Default) | Exp V2 (-JsonSerializerOptions) |
|---|---|---|---|---|
| ETS properties | ✅ Serialized | ✅ Serialized | ✅ Serialized | ❌ Ignored (*) |
| PSCustomObject | ✅ Works | ✅ Works | ✅ Works | ❌ Empty {} |
| Custom JsonConverter | N/A | ❌ | ❌ | ✅ Works |
| Default Depth | 2 | 2 | 2 | 64 (STJ default) |
| Depth exceeded | ❌ Exception | |||
| Circular reference | Depth truncation | Partial (**) | Depth truncation | Exception (***) |
| OOM risk (recursive types) | ✅ Safe |
(*) ETS properties are ignored because -JsonSerializerOptions mode passes the unwrapped BaseObject directly to JsonSerializer.Serialize() to enable custom JsonConverter support. This is a technical constraint, not a design choice.
(**) PR V2 detects circular references for PSCustomObject (→ null) but not for Hashtable/IDictionary (→ Depth truncation). This can be fixed.
(***) Can be avoided by setting ReferenceHandler = ReferenceHandler.IgnoreCycles.
Why -JsonSerializerOptions Can Use Depth 64 Safely
As @jborean93 pointed out, increasing default depth beyond 2-3 is dangerous when ETS properties are serialized:
# Default mode - ETS properties cause exponential growth
Get-Item env:PSModulePath | ConvertTo-Json -Depth 2 # 7 KB
Get-Item env:PSModulePath | ConvertTo-Json -Depth 5 # 33 MB, 6 sec
Get-Item env:PSModulePath | ConvertTo-Json -Depth 6 # OOM
# -JsonSerializerOptions mode - ETS ignored, safe
$opt = [System.Text.Json.JsonSerializerOptions]::new()
Get-Item env:PSModulePath | ConvertTo-Json -JsonSerializerOptions $opt # 552 bytesSince -JsonSerializerOptions mode ignores ETS properties (to enable custom JsonConverter), it avoids the Type object explosion and can safely use Depth 64.
My Assessment
The experimental implementation has advantages:
- Enables custom JsonConverter (which you mentioned as important)
- No global state (fully thread-safe)
- Better leverages STJ's native capabilities
- V1 compatibility maintained in default mode
I'd appreciate your feedback on both approaches. Should we:
- Continue with the current PR (simpler, direct Utf8JsonWriter control)?
- Switch to the experimental approach by merging it into this PR (custom converter support, better STJ integration)?
- Something else?
|
@yotsuda Thanks for great investigations!
Do you say about just PSObject converter? I guess we still haven't control on others? If we take C# type having 5 depth do we still get exception with default 2 depth serialization? |
Default mode (without
|
|
@yotsuda Thanks for clarify! I'm looking at V1 and I see that there's the same problem - the NewtonSoft serializer throws an exception when the depth is exceeded. V1 implementation is forced to circumvent the limitation of the NewtonSoft serializer by performing preprocessing, converting complex objects into a dictionary, limiting the depth, and then calling the serializer for the truncated object. |
|
As for adding -JsonSerializerOptions parameter I suggest to postpone this for next stage. |
|
@iSazonov Thank you for the detailed feedback!
My experimental branch uses exactly what you suggested - a custom JsonConverter for PSObject that handles depth control internally. Serialization is done entirely by STJ (Newtonsoft is only referenced for JObject compatibility and the existing EscapeHandling parameter type). Here's how V1 and experimental V2 differ: V1: Experimental V2: The key difference: V1 creates intermediate Dictionary/List objects, while experimental V2 writes directly without intermediate allocation.
Agreed. I'll remove it from the initial implementation. I'll close this PR and submit a new one from the experimental branch, focused on V1 compatibility. For reference: |
Ok. And since we keep our serializer and lost benefits of standard serializer I believe we need to ask .Net team to enhance the API so that we can use it. |
PR Summary
Adds a new experimental feature
PSJsonSerializerV2that migratesConvertTo-Jsonfrom Newtonsoft.Json to System.Text.Json, with improved defaults and support for non-string dictionary keys.Fixes #5749
PR Context
Problem
When converting objects containing dictionaries with non-string keys (such as
Exception.Datawhich usesIDictionarywithobjectkeys),ConvertTo-JsonthrowsNonStringKeyInDictionaryerror, making it impossible to serialize such objects. Additionally, the default depth of 2 is too shallow for most real-world use cases.Solution
Added a new
PSJsonSerializerV2experimental feature that uses System.Text.Json instead of Newtonsoft.Json. The new implementation converts non-string dictionary keys viaToString(), enabling serialization of previously unsupported types. Default depth is increased to 64 (matching System.Text.Json's default), and there is no upper limit on depth when V2 is enabled.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerPSJsonSerializerV2Changes Made
Commit 1: Add System.Text.Json serializer for ConvertTo-Json via PSJsonSerializerV2 experimental feature
1.
ExperimentalFeature.cs(+5 lines)PSJsonSerializerV2experimental feature constant and registration2.
ConvertToJsonCommand.cs(+50 lines, -4 lines)DefaultDepth,DefaultDepthV2,DepthAllowed)_depthfrominttoint?to detect user-specified valuesDepthproperty to return V2 default (64) or legacy default (2) based on feature stateBeginProcessingto validate depth parameter3.
JsonObject.cs(+6 lines)SystemTextJsonSerializer.ConvertToJsonwhen V2 is enabled4.
SystemTextJsonSerializer.cs(+624 lines, new file)SystemTextJsonSerializerstatic class withConvertToJsonmethodPowerShellJsonWriterclass for manual depth trackingToString()JsonIgnoreAttributeandHiddenAttributeJObject5.
WebCmdletStrings.resx(+3 lines)JsonDepthExceedsLimiterror message resource6.
ConvertTo-Json.PSJsonSerializerV2.Tests.ps1(+199 lines, new file)Commit 2: Refactor PowerShellJsonWriter to use iterative approach instead of recursion
SystemTextJsonSerializer.cs(+180 lines, -83 lines)WriteValuecalls with explicitStack<WriteTask>Commit 3: Remove Depth upper limit and allow -1 for unlimited when PSJsonSerializerV2 is enabled
1.
ConvertToJsonCommand.csDepthAllowedV2constant (no upper limit when V2 enabled)[ValidateRange]attribute fromDepthparameter-Depth -1as unlimited depth-2or less throws an error2.
WebCmdletStrings.resxJsonDepthMustBeNonNegativeerror message resource3.
ConvertTo-Json.PSJsonSerializerV2.Tests.ps1Commit 4: Add backward compatibility tests that run in both V2 enabled and disabled states
ConvertTo-Json.PSJsonSerializerV2.Tests.ps1-Skipfrom backward compatibility tests so they run in both statesCommit 5: Fix description and doc comment to reflect no upper depth limit
1.
ExperimentalFeature.cs2.
ConvertToJsonCommand.csTotal: 6 files changed, +1032 insertions, -4 deletions
Behavior Examples
Before (throws error)
After (with PSJsonSerializerV2 enabled)
Unlimited depth with -1
Default depth comparison
Testing
Test Categories (30 tests total)
V2 Feature Tests (24 tests) - Run when PSJsonSerializerV2 is enabled:
Backward Compatibility Tests (5 tests) - Run in both enabled and disabled states:
Legacy Behavior Test (1 test) - Run when PSJsonSerializerV2 is disabled:
Test Results
Implementation Details
Key Features
-1for unlimited)Design Decisions
Experimental feature - Allows users to opt-in to new behavior without breaking existing scripts
Utf8JsonWriter with iterative approach - Direct writer usage instead of JsonSerializer for full control over depth tracking. The iterative (non-recursive) implementation uses an explicit stack, eliminating call stack exhaustion risk for deeply nested objects.
Default depth 64 - Matches System.Text.Json's default maximum depth, providing a good balance between usability and safety
No upper limit with -1 for unlimited - Since the implementation is iterative (not recursive), there's no risk of stack overflow. Users can specify
-Depth -1for truly unlimited depth, while negative values other than -1 throw an error to catch typos.Graceful depth handling - Converts deep objects to strings instead of throwing (matches legacy behavior)
ToString() for non-string keys - Simple, predictable conversion for non-string dictionary keys
Backward Compatibility
Questions for Reviewers
I would like to verify that the V2 tests run correctly when
PSJsonSerializerV2is enabled. Is there a way to run CI tests with experimental features enabled, or is there a separate CI job for testing experimental features?I've verified locally that:
Related Work
Previous PR
Directly Related Issues
Indirectly Related (ConvertFrom-Json / Depth handling)
.NET Runtime Issues (STJ feedback from previous work)