`TimeOnlyConverter` should tolerate more formats in line with `TimeSpanConverter`.

[ghosttie]

In #53539 support for DateOnly and TimeOnly was added to JsonSerializer but nullable versions of them is not supported

[elgonzo]

but nullable versions of them is not supported

What exactly led you to making this statement?

Nullable DateOnly and TimeOnly are just some concrete Nullable<T>'s and JsonSerializer is being able to handle Nullable\<T>. Proof: https://dotnetfiddle.net/ackJio

[ghosttie]

I got an exception, I'll work on reproducing it on Monday

[ghosttie]

I misread the exception, it's not that Nullable TimeOnly isn't supported, it's the specific value that it can't convert to a Nullable TimeOnly.

The value it's trying to convert to Nullable TimeOnly is "16:55" which seems like it should be valid (given that TimeOnly has a constructor that only needs hour and minute) but the serializer will only accept "16:55:00".

The exception I get is

System.Text.Json.JsonException
  HResult=0x80131500
  Message=The JSON value could not be converted to System.Nullable`1[System.TimeOnly]. Path: $.transitions[0].scheduleTime | LineNumber: 0 | BytePositionInLine: 275.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at ETR.Tools.JsonHelper.Deserialize[T](String json) in C:\Users\ghosttie\source\repos\ETR\ETR\Tools\JsonHelper.cs:line 20

  This exception was originally thrown at this call stack:
    [External Code]

Inner Exception 1:
FormatException: The JSON value is not in a supported TimeOnly format.

test.json

[martincostello]

The converter for TimeOnly appears to require seconds:

https://github.com/dotnet/runtime/blob/6931b3b729ec050f935f958122d3cc66bb46fdcf/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeOnlyConverter.cs#L13

[ghosttie]

OK, well if it's intended behavior then we can close this

[elgonzo]

Don't close, yet. This seems to be a bug, an oversight, or still a pending work item as far as STJ 9.0.0 is concerned.

I just tested with STJ 9.0.0-preview.6.24327.7. It can deserialize "16:55" as a TimeSpan (which STJ 8.x.x doesn't seem to be able to do), however, it still can't deserialize it into a TimeOnly, which doesn't look intentional to me.

[martincostello]

The comments in the code for TimeSpan for JSON deserialization also appear to require seconds:

https://github.com/dotnet/runtime/blob/6931b3b729ec050f935f958122d3cc66bb46fdcf/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.TimeSpan.cs#L22-L25

[elgonzo]

There is something funky going on with the PR https://github.com/dotnet/runtime/pull/102091.

It enables deserializing of TimeSpan's without seconds when it appears as a property/field of a model class (or nested in an json object and/or array, perhaps). However, deserializing of TimeSpan's directly from a json string still doesn't work...

PEBCAK.

PR https://github.com/dotnet/runtime/pull/102091 enables TimeSpan deserialization without seconds without problems. It's just that TimeOnly deserialization isn't benefiting from this improvement, yet...

[martincostello]

In that case, this issue is a feature request to support hh:mm for TimeOnly just like #102091 did for TimeSpan.

AFAIK, it's too late for such a change to be included in .NET 9 if approved, it would be part of .NET 10.

[elgonzo]

@martincostello FYI, the code comment you referred to is inaccurate when the c format is passed (as the JsonConverter for TimeSpan does). If you look at the code of the method the comment is from, you'll notice it calls TryParseTimeSpanC for the c format, which doesn't require seconds to be present to successfully parse a TimeSpan...

[jeffhandley]

@eiriktsarpalis Can you weigh in on if you think we should address this, and if so, in .NET 9 during RC1/RC2?

[eiriktsarpalis]

Extending https://github.com/dotnet/runtime/pull/102091 to also apply to the built-in TimeOnly converter would be desirable, and I would support a fix that targets RC1 (we have one week left until the RC1 window closes). I don't necessarily believe that it meets the bar for an RC2 fix (the current behavior got shipped with .NET 8) but perhaps we could make a case for consistency with TimeSpan.

@ghosttie you mentioned in your initial post that DateOnly is also affected. Can you confirm if that is still the case?

[ghosttie]

No, sorry, I haven't tried this with DateOnly, I modeled the title off of #53539 because of my incorrect assumption about what the problem was

[ghosttie]

Actually I've just had an exception with Nullable DateOnly

System.Text.Json.JsonException
  HResult=0x80131500
  Message=The JSON value could not be converted to System.Nullable`1[System.DateOnly]. Path: $.transitions[0].startDate | LineNumber: 0 | BytePositionInLine: 301.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo`1 jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo`1 jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at ETR.Tools.JsonHelper.Deserialize[T](String json) in C:\Users\ghosttie\source\repos\ETR\ETR\Tools\JsonHelper.cs:line 21

  This exception was originally thrown at this call stack:
    [External Code]

Inner Exception 1:
FormatException: The JSON value is not in a supported DateOnly format.

when trying to deserialize the value '' as a Nullable DateOnly

I guess this is a separate issue from TimeOnly not working without seconds

[martincostello]

I'm not sure that's valid - an empty string isn't null and it isn't a date, so I'd argue that's the correct behaviour to fail to deserialize.