Replace JSON.NET with System.Text.Json across the codebase#2135
Open
Replace JSON.NET with System.Text.Json across the codebase#2135
Conversation
Remove all Newtonsoft.Json serialization from the application layer, replacing it with System.Text.Json (STJ). NEST still brings in Newtonsoft transitively, but all application-level serialization now uses STJ exclusively. Key changes: - Add ElasticSystemTextJsonSerializer as custom IElasticsearchSerializer for NEST - Add EmptyCollectionModifier to omit empty collections during serialization - Add ObjectToInferredTypesConverter to handle JObject/JToken from NEST reads - Add JsonNodeExtensions as STJ equivalents of JObject helpers for event upgraders - Add IJsonOnDeserialized to Event model to merge [JsonExtensionData] into Data dict - Add [JsonPropertyName] attributes to V1 webhook models for PascalCase compat - Migrate all event upgraders from JObject to JsonObject (System.Text.Json.Nodes) - Migrate all plugins from ISerializer/JsonSerializerOptions DI injection - Use case-insensitive deserialization for DataDictionary.GetValue<T>() from JsonElement - Use semantic comparison (JsonNode.DeepEquals) in tests for fixture validation - Remove DataObjectConverter, ElasticJsonNetSerializer, and related Newtonsoft classes - Remove Foundatio.JsonNet, NEST.JsonNetSerializer, FluentRest.NewtonsoftJson packages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| var connectionPool = CreateConnectionPool(); | ||
| var settings = new ConnectionSettings(connectionPool, (serializer, values) => new ElasticJsonNetSerializer(serializer, values, _serializerSettings)); | ||
| var serializer = new ElasticSystemTextJsonSerializer(_jsonSerializerOptions); | ||
| var settings = new ConnectionSettings(connectionPool, (_, _) => serializer); |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates application-layer JSON usage from Newtonsoft.Json to System.Text.Json (STJ), aligning core models/plugins/tests and adding STJ-based infrastructure for Elasticsearch/NEST while keeping Newtonsoft only as a transitive dependency for the NEST wire protocol.
Changes:
- Introduces STJ-based
IElasticsearchSerializerimplementation and STJ type metadata modifiers to match prior Newtonsoft serialization behavior (e.g., omit empty collections). - Updates core pipeline/plugins/controllers/jobs to use
ITextSerializer/JsonSerializerOptionsinstead of Newtonsoft abstractions. - Refactors tests and fixtures to validate STJ semantics (including semantic JSON comparison).
Reviewed changes
Copilot reviewed 87 out of 87 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Exceptionless.Tests/Utility/DataBuilder.cs | Updates test builder to use ITextSerializer for manual stacking info. |
| tests/Exceptionless.Tests/Serializer/SerializerTests.cs | Reworks serializer tests away from Newtonsoft-specific behavior toward STJ round-trips. |
| tests/Exceptionless.Tests/Serializer/Models/PersistentEventSerializerTests.cs | Uses ITextSerializer for typed data retrieval in PersistentEvent tests. |
| tests/Exceptionless.Tests/Serializer/Models/DataDictionaryTests.cs | Updates GetValue<T> tests to use ITextSerializer pathway. |
| tests/Exceptionless.Tests/Repositories/StackRepositoryTests.cs | Compares serialized JSON via ITextSerializer instead of ToJson(). |
| tests/Exceptionless.Tests/Repositories/ProjectRepositoryTests.cs | Updates Slack token access to use serializer-aware accessor. |
| tests/Exceptionless.Tests/Repositories/EventRepositoryTests.cs | Uses serializer-based typed accessors in repository tests. |
| tests/Exceptionless.Tests/Plugins/WebHookDataTests.cs | Switches expected/actual comparisons to semantic JSON equivalence using STJ. |
| tests/Exceptionless.Tests/Plugins/SummaryDataTests.cs | Moves summary comparisons to semantic JSON using STJ. |
| tests/Exceptionless.Tests/Plugins/GeoTests.cs | Updates Geo plugin/tests to pass ITextSerializer. |
| tests/Exceptionless.Tests/Plugins/EventUpgraderTests.cs | Uses JsonNode formatting + semantic compare for upgrader outputs. |
| tests/Exceptionless.Tests/Plugins/EventParserTests.cs | Uses ITextSerializer for round-trip verification; removes Newtonsoft formatting asserts. |
| tests/Exceptionless.Tests/Pipeline/EventPipelineTests.cs | Updates pipeline tests to use serializer-based typed accessors. |
| tests/Exceptionless.Tests/Mail/MailerTests.cs | Updates Mailer construction to accept ITextSerializer. |
| tests/Exceptionless.Tests/IntegrationTestsBase.cs | Replaces FluentRest Newtonsoft serializer with STJ JsonContentSerializer. |
| tests/Exceptionless.Tests/Exceptionless.Tests.csproj | Removes FluentRest.NewtonsoftJson, adds STJ-based FluentRest. |
| tests/Exceptionless.Tests/Controllers/EventControllerTests.cs | Uses ITextSerializer for typed accessors in controller tests. |
| src/Exceptionless.Web/Exceptionless.Web.csproj | Removes NEST.JsonNetSerializer package reference. |
| src/Exceptionless.Web/Controllers/ProjectController.cs | Injects ITextSerializer for Slack token access. |
| src/Exceptionless.Web/Controllers/EventController.cs | Injects ITextSerializer and uses it for event payload byte generation. |
| src/Exceptionless.Core/Utility/TypeHelper.cs | Updates equality handling for STJ JsonElement. |
| src/Exceptionless.Core/Utility/ExtensibleObject.cs | Adds JsonElement handling to generic property retrieval. |
| src/Exceptionless.Core/Utility/ErrorSignature.cs | Switches error signature extraction to serializer-based GetValue<T>. |
| src/Exceptionless.Core/Services/SlackService.cs | Uses serializer-aware Slack token access. |
| src/Exceptionless.Core/Serialization/ObjectToInferredTypesConverter.cs | Refines number inference logic for STJ dynamic object conversion. |
| src/Exceptionless.Core/Serialization/LowerCaseUnderscorePropertyNamesContractResolver.cs | Deletes Newtonsoft contract resolver (obsolete after STJ migration). |
| src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs | Defines Exceptionless STJ defaults (snake_case, safe encoder, converters, empty-collection skipping). |
| src/Exceptionless.Core/Serialization/ExceptionlessNamingStrategy.cs | Deletes Newtonsoft naming strategy (replaced by STJ naming policy). |
| src/Exceptionless.Core/Serialization/EmptyCollectionModifier.cs | Adds STJ type info modifier to omit empty collections. |
| src/Exceptionless.Core/Serialization/ElasticSystemTextJsonSerializer.cs | Adds STJ IElasticsearchSerializer for NEST with custom converters. |
| src/Exceptionless.Core/Serialization/ElasticJsonNetSerializer.cs | Deletes Newtonsoft-based NEST serializer. |
| src/Exceptionless.Core/Serialization/ElasticConnectionSettingsAwareContractResolver.cs | Deletes Newtonsoft resolver used by old NEST serializer. |
| src/Exceptionless.Core/Serialization/DynamicTypeContractResolver.cs | Deletes Newtonsoft dynamic contract resolver. |
| src/Exceptionless.Core/Serialization/DataObjectConverter.cs | Deletes Newtonsoft-based model/data converter. |
| src/Exceptionless.Core/Repositories/Configuration/ExceptionlessElasticConfiguration.cs | Switches Elasticsearch client wiring to STJ serializer. |
| src/Exceptionless.Core/Plugins/WebHook/Default/010_VersionOnePlugin.cs | Uses serializer-based typed accessors; adds [JsonPropertyName] for v1 webhook compatibility. |
| src/Exceptionless.Core/Plugins/WebHook/Default/005_SlackPlugin.cs | Uses serializer-based typed accessors for Slack webhook creation. |
| src/Exceptionless.Core/Plugins/Formatting/FormattingPluginBase.cs | Converts formatting plugins to depend on ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/99_DefaultFormattingPlugin.cs | Updates default formatting/slack attachment creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/60_LogFormattingPlugin.cs | Updates typed accessors + slack attachment creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/50_SessionFormattingPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/40_UsageFormattingPlugin.cs | Updates typed accessors + slack attachment creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/30_NotFoundFormattingPlugin.cs | Updates typed accessors + IP retrieval to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/20_ErrorFormattingPlugin.cs | Updates typed accessors + slack attachment creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/10_SimpleErrorFormattingPlugin.cs | Updates typed accessors + slack attachment creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/Formatting/Default/05_ManualStackingFormattingPlugin.cs | Updates manual stacking info access to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventUpgrader/EventUpgraderContext.cs | Migrates upgrader DOM from JObject/JArray to JsonObject/JsonArray. |
| src/Exceptionless.Core/Plugins/EventUpgrader/Default/V2_EventUpgrade.cs | Rewrites upgrader logic from Newtonsoft DOM to JsonNode DOM. |
| src/Exceptionless.Core/Plugins/EventUpgrader/Default/V1R850_EventUpgrade.cs | Rewrites upgrader logic to JsonObject. |
| src/Exceptionless.Core/Plugins/EventUpgrader/Default/V1R844_EventUpgrade.cs | Rewrites upgrader logic to JsonObject. |
| src/Exceptionless.Core/Plugins/EventUpgrader/Default/V1R500_EventUpgrade.cs | Rewrites upgrader logic to JsonObject and normalizes date string formatting. |
| src/Exceptionless.Core/Plugins/EventUpgrader/Default/GetVersion.cs | Rewrites version detection to JsonNode APIs. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/90_RemovePrivateInformationPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/80_AngularPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/70_SessionPlugin.cs | Updates typed accessors + session start creation to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/50_GeoPlugin.cs | Updates IP extraction to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/45_EnvironmentInfoPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/40_RequestInfoPlugin.cs | Updates typed accessors and request exclusion processing to accept ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/30_SimpleErrorPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/20_ErrorPlugin.cs | Updates typed accessors + ErrorSignature to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/10_NotFoundPlugin.cs | Updates typed accessors to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/0_ThrottleBotsPlugin.cs | Updates request-info access to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventProcessor/Default/03_ManualStackingPlugin.cs | Updates manual stacking info access to use ITextSerializer. |
| src/Exceptionless.Core/Plugins/EventParser/Default/LegacyErrorParserPlugin.cs | Migrates legacy parsing to JsonNode + STJ options. |
| src/Exceptionless.Core/Plugins/EventParser/Default/JsonEventParserPlugin.cs | Uses ITextSerializer for parsing event payloads instead of Newtonsoft. |
| src/Exceptionless.Core/Models/SummaryData.cs | Adjusts nullability/required-ness to align with STJ behaviors. |
| src/Exceptionless.Core/Models/StackSummaryModel.cs | Adjusts nullability/required-ness to align with STJ behaviors. |
| src/Exceptionless.Core/Models/Stack.cs | Removes Newtonsoft enum converter attribute; keeps STJ converter. |
| src/Exceptionless.Core/Models/SlackToken.cs | Replaces Newtonsoft [JsonProperty] with STJ [JsonPropertyName]; updates SlackAttachment ctor to use ITextSerializer. |
| src/Exceptionless.Core/Models/Messaging/ReleaseNotification.cs | Removes required modifiers (likely to avoid STJ required-member enforcement). |
| src/Exceptionless.Core/Models/Event.cs | Adds [JsonExtensionData] + IJsonOnDeserialized merge into Data. |
| src/Exceptionless.Core/Mail/Mailer.cs | Switches user-info extraction to serializer-based accessors. |
| src/Exceptionless.Core/Jobs/WebHooksJob.cs | Switches webhook POST payload handling to STJ PostAsJsonAsync + options. |
| src/Exceptionless.Core/Jobs/EventPostsJob.cs | Uses serializer-based event bytes for retries. |
| src/Exceptionless.Core/Jobs/EventNotificationsJob.cs | Updates request-info access to use ITextSerializer. |
| src/Exceptionless.Core/Jobs/CloseInactiveSessionsJob.cs | Updates identity extraction to use ITextSerializer. |
| src/Exceptionless.Core/Extensions/RequestInfoExtensions.cs | Requires serializer for post-data exclusion parsing (STJ). |
| src/Exceptionless.Core/Extensions/ProjectExtensions.cs | Makes Slack token extraction serializer-aware via GetValue<T>. |
| src/Exceptionless.Core/Extensions/PersistentEventExtensions.cs | Updates helpers to accept ITextSerializer for typed accessors. |
| src/Exceptionless.Core/Extensions/JsonNodeExtensions.cs | Adds STJ JsonNode helpers used by upgraders/tests (formatting, mutation helpers). |
| src/Exceptionless.Core/Extensions/JsonExtensions.cs | Removes Newtonsoft helpers, retains JSON-type detection helpers. |
| src/Exceptionless.Core/Extensions/EventExtensions.cs | Updates typed accessor APIs to accept ITextSerializer; switches GetBytes to serializer bytes. |
| src/Exceptionless.Core/Extensions/ErrorExtensions.cs | Updates stacking target helper to use serializer-based error access. |
| src/Exceptionless.Core/Extensions/DataDictionaryExtensions.cs | Reworks GetValue<T> around STJ/ITextSerializer, adds case-insensitive JsonElement handling. |
| src/Exceptionless.Core/Exceptionless.Core.csproj | Removes Newtonsoft/JsonNet packages. |
| src/Exceptionless.Core/Bootstrapper.cs | Removes Newtonsoft setup; registers STJ options + SystemTextJsonSerializer in DI. |
| AGENTS.md | Documents the new STJ-only serialization architecture and security posture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
165
to
169
| // Fast path: if already a MemoryStream with accessible buffer, use it directly | ||
| if (stream is MemoryStream ms && ms.TryGetBuffer(out var segment)) | ||
| { | ||
| return segment.AsSpan(); | ||
| } |
There was a problem hiding this comment.
ReadStreamToSpan's MemoryStream fast-path ignores ms.Position and returns the entire underlying buffer segment. If the input stream isn't positioned at 0 (common when reusing streams), deserialization will include leading bytes and can fail or produce incorrect results. Consider slicing the span to [ms.Position..ms.Length] (or just use JsonSerializer.Deserialize(stream, ...) for the sync path).
Comment on lines
97
to
101
| public void Serialize<T>(T data, Stream stream, SerializationFormatting formatting = SerializationFormatting.None) | ||
| { | ||
| using var writer = new Utf8JsonWriter(stream); | ||
| var options = GetOptions(formatting); | ||
|
|
There was a problem hiding this comment.
Serialize<T> creates Utf8JsonWriter without enabling indentation, so SerializationFormatting.Indented will still produce compact JSON. JsonSerializerOptions.WriteIndented isn't applied when you pass an existing writer; you need to construct the writer with new JsonWriterOptions { Indented = true } (and optionally a consistent IndentSize).
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 87 out of 87 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
171
to
178
| // Slow path: copy to new buffer | ||
| using var buffer = stream.CanSeek | ||
| ? new MemoryStream((int)stream.Length) | ||
| : new MemoryStream(); | ||
|
|
||
| stream.CopyTo(buffer); | ||
| return buffer.TryGetBuffer(out var seg) ? seg.AsSpan() : buffer.ToArray(); | ||
| } |
There was a problem hiding this comment.
ReadStreamToSpan returns a ReadOnlySpan backed by a MemoryStream that is disposed at the end of the method (the using var buffer scope). That makes the returned span potentially point to freed/invalid memory, leading to undefined behavior during deserialization. Consider removing this helper and using JsonSerializer.Deserialize(stream, ...) for the sync APIs, or return a byte[]/Memory whose lifetime outlives the call (and also respect the stream's current Position).
Comment on lines
114
to
126
| // If the raw text contains a decimal point, treat as floating-point | ||
| if (rawValue.Contains((byte)'.')) | ||
| { | ||
| // Try decimal for precise values (e.g., financial data) before double | ||
| if (reader.TryGetDecimal(out decimal d)) | ||
| return d; | ||
|
|
||
| // Fall back to double for floating-point | ||
| return reader.GetDouble(); | ||
| } | ||
|
|
||
| // No decimal point - this is an integer | ||
| // Try int32 first for smaller values, then long for larger integers |
There was a problem hiding this comment.
ReadNumber only checks for a '.' to decide whether a JSON number was written as floating-point. JSON numbers can also use exponent notation (e.g., 1e-3, 1E5) without a decimal point; those will currently fall into the "integer" path and may be parsed as int/long/decimal, which changes the original representation and can alter round-tripping behavior. Treat numbers containing 'e'/'E' as floating-point as well (in addition to '.').
niemyjski
force-pushed
the
feature/system-text-json-v2
branch
from
e6a3b35 to
c292e13
Compare
| } | ||
| } | ||
|
|
||
| foreach (var (parent, name) in toRemove) |
| } | ||
| } | ||
| catch (Exception) { } | ||
| catch (JsonException) { } |
Comment on lines
+90
to
+96
| foreach (var name in names) | ||
| { | ||
| if (descendant.TryGetPropertyValue(name, out var value) && value.IsNullOrEmpty()) | ||
| { | ||
| toRemove.Add((descendant, name)); | ||
| } | ||
| } |
niemyjski
force-pushed
the
feature/system-text-json-v2
branch
from
c292e13 to
4d880ad
Compare
- Fix ConvertJsonElement ternary type coercion: (object)l cast prevents implicit long→double widening in switch expression - Make ObjectToInferredTypesConverter configurable with preferInt64 flag for ES serializer to match JSON.NET DataObjectConverter behavior - Fix ElasticSystemTextJsonSerializer: remove ReadStreamToSpan lifetime bug (span backed by disposed MemoryStream), deserialize from stream directly with MemoryStream fast-path - Fix Serialize<T> indentation: pass JsonWriterOptions to Utf8JsonWriter so SerializationFormatting.Indented actually produces indented output - Handle exponent notation (1e5) as floating-point in ReadNumber - Use double consistently (not decimal) for floating-point to match JSON.NET behavior - Fix RenameAll return value: return whether any renames occurred - Add using var to MemoryStream in EventController and EventPostsJob - Handle empty response bodies in SendRequestAsAsync (STJ throws on empty input, Newtonsoft returned default) - Fix SerializerTests: put unknown properties at root level to test JsonExtensionData→ConvertJsonElement path correctly - Revert AGENTS.md to main Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
niemyjski
force-pushed
the
feature/system-text-json-v2
branch
from
4d880ad to
09134ce
Compare
Comment on lines
+35
to
+38
| catch | ||
| { | ||
| // Invalid JSON - ignore | ||
| } |
Comment on lines
+49
to
+52
| catch | ||
| { | ||
| // Invalid JSON - ignore | ||
| } |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes all Newtonsoft.Json serialization from the application layer, replacing it with System.Text.Json (STJ) exclusively. NEST still brings in Newtonsoft transitively for Elasticsearch wire protocol, but all application-level serialization now uses STJ.
Key Changes
New Files
ElasticSystemTextJsonSerializer— CustomIElasticsearchSerializerfor NEST using STJEmptyCollectionModifier— STJ type modifier that omits empty collections during serializationJsonNodeExtensions— STJ equivalents of JObject helpers for event upgradersObjectToInferredTypesConverter— Handles JObject/JToken from NEST during STJ serializationDeleted Files (Newtonsoft-specific)
DataObjectConverter,ElasticJsonNetSerializer,DynamicTypeContractResolverElasticConnectionSettingsAwareContractResolver,ExceptionlessNamingStrategyLowerCaseUnderscorePropertyNamesContractResolverCore Changes
IJsonOnDeserializedmerges[JsonExtensionData]overflow intoDatadictionaryGetValue<T>()fromJsonElementJObjecttoJsonObject(System.Text.Json.Nodes)JsonSerializerOptionsfrom DI instead ofISerializer[JsonPropertyName]attributes for PascalCase backward compatibilityPackage Changes
Foundatio.JsonNet,NEST.JsonNetSerializer(x2),FluentRest.NewtonsoftJsonFluentRest(STJ-based)Testing
JsonNode.DeepEquals)