Skip to content

fix: fixed date time deserializer #827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbansla merged 2 commits into from
Apr 30, 2026
Merged

fix: fixed date time deserializer #827

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37413df
fixed date time deserializer
sbansla Apr 27, 2026
0924857
added tests
sbansla Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Twilio/Converters/CustomConverters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,16 @@
}
}

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)

Check failure on line 30 in src/Twilio/Converters/CustomConverters.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=twilio_twilio-csharp&issues=AZ3OGqhvDEWyCvtato42&open=AZ3OGqhvDEWyCvtato42&pullRequest=827
{
if (reader.TokenType == JsonToken.Null)
{
return null;
}
if (reader.TokenType == JsonToken.Date)
{
return (DateTime)reader.Value;
}
Comment on lines +32 to +44

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadJson returns a DateTime for JsonToken.Date regardless of objectType. If this converter is used for List<DateTime>, a non-array JSON value (e.g., a single date) will cause the converter to return the wrong runtime type and Newtonsoft will throw during assignment. Consider branching on objectType and either (a) wrapping a single date into a singleton list for List<DateTime> or (b) throwing a clear exception when the JSON shape doesn't match the target type (and similarly avoid returning a list when objectType is DateTime?).

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +44

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting reader.Value directly to DateTime can throw if the serializer is configured with DateParseHandling.DateTimeOffset (in which case JsonToken.Date values are typically DateTimeOffset). Consider handling both DateTime and DateTimeOffset (and converting consistently) to avoid an InvalidCastException.

Copilot uses AI. Check for mistakes.
if (reader.TokenType == JsonToken.StartArray)
{
var dateTimes = new List<DateTime>();
Expand All @@ -42,6 +50,10 @@
dateTimes.Add(dateTime);
}
}
else if (reader.TokenType == JsonToken.Date)
{
dateTimes.Add((DateTime)reader.Value);
}
Comment on lines 32 to +61

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior adds support for JsonToken.Null and JsonToken.Date (including date values inside arrays), but there are no unit tests covering these cases. There are existing NUnit tests for other converters under test/Twilio.Test/Converters; adding a focused test for DateTimeConverter would help prevent regressions across different token shapes (null, string, date token, array).

Copilot uses AI. Check for mistakes.
else if (reader.TokenType == JsonToken.EndArray)
{
return dateTimes;
Expand Down
Loading