Skip to content

Replace JSON.NET with System.Text.Json 10.x#359

Open
niemyjski wants to merge 4 commits into
mainfrom
niemyjski/drop-json-net-use-stj
Open

Replace JSON.NET with System.Text.Json 10.x#359
niemyjski wants to merge 4 commits into
mainfrom
niemyjski/drop-json-net-use-stj

Conversation

@niemyjski
Copy link
Copy Markdown
Member

@niemyjski niemyjski commented May 31, 2026

Summary

Drops Newtonsoft.Json (JSON.NET) entirely from the Exceptionless .NET client and replaces it with System.Text.Json 10.x.

Key Changes

Serializer Infrastructure

  • Removed vendored src/Exceptionless/Newtonsoft.Json/ directory (hundreds of files)
  • Added DefaultJsonSerializer using STJ with JsonNamingPolicy.SnakeCaseLower (matches server approach from Replace JSON.NET with System.Text.Json across the codebase Exceptionless#2135)
  • Added DataDictionaryConverter for the special DataDictionary type (stores complex values as raw JSON strings)
  • Added SettingsDictionaryConverter for the SettingsDictionary type
  • Added PostDataConverter for RequestInfo.PostData (object → indented string on deserialization)

Bug Fixes

  1. DataDictionary roundtrip bug (CRITICAL): When events go through storage (serialize → deserialize → re-serialize for API), complex objects in Event.Data became escaped strings instead of JSON objects. Fixed by using WriteRawValue for string values that start with { or [.

  2. Exclusion logic bug: GetTypeInfo() silently threw without an explicit TypeInfoResolver, causing the WriteValue exclusion/depth logic to fall through to direct serialization. Fixed by adding TypeInfoResolver = new DefaultJsonTypeInfoResolver().

Wire Format Alignment

  • Uses JsonNamingPolicy.SnakeCaseLower globally (same as server)
  • [JsonPropertyName] attributes on EnvironmentInfo.OSName/OSVersion for legacy o_s_name/o_s_version format
  • PropertyNameCaseInsensitive = true for flexible deserialization
  • Dictionary keys are preserved as-is (they're data, not property names)

Dependencies

  • Removed: Vendored Newtonsoft.Json source
  • Added: System.Text.Json 10.0.0-preview.4.25258.110 (supports netstandard2.0)

Testing

  • All 318 tests pass (301 core + 10 MessagePack + 7 skipped platform-specific)
  • Added regression test for DataDictionary roundtrip proving the bug fix
  • All existing serialization tests updated for snake_case output format

- Remove vendored Newtonsoft.Json (entire src/Exceptionless/Newtonsoft.Json/ directory)
- Remove update-json.ps1 script
- Add System.Text.Json 10.0.0 NuGet package reference
- Rewrite DefaultJsonSerializer using System.Text.Json with:
  - SnakeCaseNamingPolicy matching legacy Newtonsoft behavior (e.g. OSName -> o_s_name)
  - Per-type snake_case applied only to Exceptionless.Models namespace
  - DataDictionaryConverter for storing complex values as JSON strings
  - SettingsDictionaryConverter for ObservableDictionary-based type
  - ObjectToInferredTypesConverter for proper type inference
  - PostDataConverter to convert object/array PostData to indented strings
  - Custom WriteValue with depth limiting and property exclusion support
- Update all model classes to remove [JsonObject] attributes
- Add [JsonPropertyName] for EnvironmentInfo OS properties
- Update DefaultSubmissionClient to use JsonDocument instead of JObject
- All 300 tests pass

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 31, 2026 02:59
Copy link
Copy Markdown
Contributor

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

…sion bug

- Replace custom SnakeCaseNamingPolicy with built-in JsonNamingPolicy.SnakeCaseLower
  (aligns with server approach in exceptionless/Exceptionless#2135)
- Delete unused SnakeCaseNamingPolicy.cs
- Fix DataDictionaryConverter.Write: use WriteRawValue for JSON strings that
  were previously objects (fixes double-escaping on storage roundtrip)
- Fix exclusion logic: add TypeInfoResolver = new DefaultJsonTypeInfoResolver()
  so GetTypeInfo() works and WriteValue can filter properties by name
- Update all test assertions to expect snake_case property names

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@niemyjski niemyjski changed the title Replace Newtonsoft.Json with System.Text.Json 10.x Replace JSON.NET with System.Text.Json 10.x May 31, 2026
WriteValue for IDictionary entries wrote the property name before checking
whether the value could actually be serialized at the current depth. When a
complex value exceeded maxDepth, WriteValue returned without writing anything,
leaving the JSON writer in an invalid state. The error was silently swallowed
by continueOnSerializationError, causing a fallback to full serialization
(effectively ignoring the depth limit entirely).

Fix: check depth before writing the property name. Skip complex dictionary
entries that would exceed maxDepth, consistent with the object property path.

Added regression test that fails before the fix (depth limit violated) and
passes after.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@niemyjski niemyjski requested a review from Copilot May 31, 2026 03:38
Copy link
Copy Markdown
Contributor

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@niemyjski niemyjski self-assigned this May 31, 2026
- preserve literal JSON-looking strings in DataDictionary as strings
- restore raw JSON emission only for values produced from structured data
- preserve raw JSON markers through MessagePack storage roundtrips
- coerce primitive SettingsDictionary JSON values to strings like main
- keep dictionary depth-limit regression coverage

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants