ArgumentOfRangeException when inserting objects with DateTime.MinValue

ThomasHoest commented 8 years ago

It seems that the ReqlDateTimeConverter throws an exception when converting DateTime.MinValue. Seems converting non UTC results can result in this. Converting like this

dto = DateTime.SpecifyKind((DateTime)value,DateTimeKind.Utc);

prevents it, but wouldn't work for dates saved in local time. Digging deeper and perhaps i can come up with some more detail.

bchavez commented 8 years ago

Hi, thanks for the report; however, I cannot help without more context information. Preferably,

Driver Version
Stacktrace
JSON Protocol Traces
Problematic ReQL Queires
Any associated classes or POCOs used in queries
Possible Unit Test to reproduce the issue

Maybe related to #49

Thanks, Brian

:fallenleaf: :leaves: [**"I get a sense of weightlessness…"**_](https://www.youtube.com/watch?v=lPdNTBwRR-4)

bchavez commented 8 years ago

Also, labeled this as kind of gotcha since RethinkDB doesn't support a datetimes with DateTime.MinValue. The minimum supported date in RethinkDB is I think is something like year 1400 minimum.

:crescentmoon: :stars: [**"Nothing good happens past 2am..."**_](https://www.youtube.com/watch?v=VOlcDBXKhSU)

ThomasHoest commented 8 years ago

Hi Brian

Sorry for the lack of detail. Here is a test and a stacktrace. Time is Unix time or Epoch and DateTime.MinValue works fine with this. It simply becomes a rather large negative number. More precisely this -62135596800 :)

Unit test

[Fact]
public void MinValueThrowsException()
{
    var connection = R.Connection()
            .Hostname("127.0.0.1")
            .Port(28016)
            .Timeout(30)
            .Connect();
    try
    {
        R.Db("db").TableCreate("test").Run(connection);

        var t = new PocoWithDateTime()
        {
            id = "42",
            Time = DateTime.MinValue
        };

        R.Db("db").Table("test").Insert(t).Run(connection);

        var p = R.Db("db").Table("test").Get("42").Run(connection);
        Assert.NotNull(p);
    }
    finally
    {
        R.Db("db").TableDrop("test").Run(connection);
    }
}

public class PocoWithDateTime
{
    public string id { get; set; }
    public DateTime Time { get; set; }
}

Stacktrace

mscorlib.dll!System.DateTimeOffset.ValidateDate(System.DateTime dateTime, System.TimeSpan offset)   Unknown
mscorlib.dll!System.DateTimeOffset.DateTimeOffset(System.DateTime dateTime) Unknown
mscorlib.dll!System.DateTimeOffset.implicit operator System.DateTimeOffset(System.DateTime dateTime)    Unknown
RethinkDb.Driver.dll!RethinkDb.Driver.Net.JsonConverters.ReqlDateTimeConverter.WriteJson(Newtonsoft.Json.JsonWriter writer, object value, Newtonsoft.Json.JsonSerializer serializer) Line 25    C#
Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeConvertable(Newtonsoft.Json.JsonWriter writer, Newtonsoft.Json.JsonConverter converter, object value, Newtonsoft.Json.Serialization.JsonContract contract, Newtonsoft.Json.Serialization.JsonContainerContract collectionContract, Newtonsoft.Json.Serialization.JsonProperty containerProperty)    Unknown
Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(Newtonsoft.Json.JsonWriter writer, object value, Newtonsoft.Json.Serialization.JsonObjectContract contract, Newtonsoft.Json.Serialization.JsonProperty member, Newtonsoft.Json.Serialization.JsonContainerContract collectionContract, Newtonsoft.Json.Serialization.JsonProperty containerProperty) Unknown
Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(Newtonsoft.Json.JsonWriter jsonWriter, object value, System.Type objectType)   Unknown
Newtonsoft.Json.dll!Newtonsoft.Json.JsonSerializer.SerializeInternal(Newtonsoft.Json.JsonWriter jsonWriter, object value, System.Type objectType)   Unknown
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.Poco.Build() Line 39  C#
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.ReqlAst.Build.AnonymousMethod__10_0(RethinkDb.Driver.Ast.ReqlAst a) Line 37   C#
System.Core.dll!System.Linq.Enumerable.WhereSelectListIterator<RethinkDb.Driver.Ast.ReqlAst, object>.MoveNext() Unknown
Newtonsoft.Json.dll!Newtonsoft.Json.Linq.JContainer.AddInternal(int index, object content, bool skipParentCheck)    Unknown
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.ReqlAst.Build() Line 38   C#
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.Query.Serialize() Line 70 C#
RethinkDb.Driver.dll!RethinkDb.Driver.Net.Connection.SendQuery(RethinkDb.Driver.Ast.Query query, System.Threading.CancellationToken cancelToken, bool awaitResponse) Line 420   C#
RethinkDb.Driver.dll!RethinkDb.Driver.Net.Connection.RunQueryAsync<object>(RethinkDb.Driver.Ast.Query query, System.Threading.CancellationToken cancelToken) Line 381   C#
RethinkDb.Driver.dll!RethinkDb.Driver.Net.Connection.RunAsync<object>(RethinkDb.Driver.Ast.ReqlAst term, object globalOpts, System.Threading.CancellationToken cancelToken) Line 462    C#
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.ReqlAst.RunAsync<object>(RethinkDb.Driver.Net.IConnection conn, object runOpts, System.Threading.CancellationToken cancelToken) Line 71   C#
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.ReqlAst.Run<object>(RethinkDb.Driver.Net.IConnection conn, object runOpts) Line 130   C#
RethinkDb.Driver.dll!RethinkDb.Driver.Ast.ReqlAst.Run(RethinkDb.Driver.Net.IConnection conn, object runOpts) Line 143   C#
Contentai.Test.DbMT.dll!Contentai.Test.DbMT.TableAndIndexCreation.MinValueThrowsException() Line 58 C#

bchavez commented 8 years ago

Hi @ThomasHoest ,

Unfortunately, I _cannot_ reproduce the error with your unit test.

[Test]
public void issue_66_something_weird_about_datetime_again()
{
    var connection = R.Connection()
        .Hostname("127.0.0.1")
        .Port(28015)
        .Timeout(30)
        .Connect();

    ClearDefaultTable();

    var t = new PocoWithDateTime()
        {
            id = "42",
            Time = DateTime.MinValue
        };

    table.Insert(t).Run(connection);

    var p = table.Get("42").Run(connection);
    Assert.NotNull(p);
    if( !ReferenceEquals(p, null) )
    {
        Console.WriteLine("Works on my box. =/");
        ExtensionsForTesting.Dump(p);
    }
}

public class PocoWithDateTime
{
    public string id { get; set; }
    public DateTime Time { get; set; }
}

TRACE JSON Send: Token: 2, JSON: [1,[60,[[14,["query"]],"test"]],{}]
TRACE JSON Recv: Token: 2, JSON: {"t":18,"e":4100000,"r":["Table `query.test` already exists."],"b":[]}
TRACE JSON Send: Token: 3, JSON: [1,[54,[[15,[[14,["query"]],"test"]]]],{}]
TRACE JSON Recv: Token: 3, JSON: {"t":1,"r":[{"deleted":1,"errors":0,"inserted":0,"replaced":0,"skipped":0,"unchanged":0}]}
TRACE JSON Send: Token: 1, JSON: [1,[56,[[15,[[14,["query"]],"test"]],{"id":"42","Time":{"$reql_type$":"TIME","epoch_time":-62135568000.0,"timezone":"-08:00"}}]],{}]
TRACE JSON Recv: Token: 1, JSON: {"t":1,"r":[{"deleted":0,"errors":0,"inserted":1,"replaced":0,"skipped":0,"unchanged":0}]}
TRACE JSON Send: Token: 2, JSON: [1,[16,[[15,[[14,["query"]],"test"]],"42"]],{}]
TRACE JSON Recv: Token: 2, JSON: {"t":1,"r":[{"Time":{"$reql_type$":"TIME","epoch_time":-62135568000,"timezone":"-08:00"},"id":"42"}]}
Works on my box. =/
{
  "Time": {
    "$reql_type$": "TIME",
    "epoch_time": -62135568000,
    "timezone": "-08:00"
  },
  "id": "42"
}

Still need more information:

[ ] DO include JSON protocol traces in your issue.
[ ] DO include the driver version you are using.
[ ] DO describe the operating system for the Server and Client (Windows / Linux). Also, the run-time platform and versions (.NET Framework/.NET Core/Mono).

Also, FWIW, as guidance, I would not recommend using DateTime.MinValue with RethinkDB. ReQL datetime functions will break with dates less than year 1400. I'd highly recommend using DateTime? nullable to indicate some kind of sentinel value. _The spirits technical debt rattle the chains of fire breathing dragons, Dragons, DRAGONS!!!_

Lastly, it's a holiday weekend here in the USA so I won't be available much. I'd prefer to keep issues related to source code changes, confirmed bugs, and features requests. Every time an issue is created 12 people get slammed on the repo watch list. :crying_cat_face: If you need additional help, you can PM me on slack via @bchavez. Hopefully we can work this out on slack until we have both confirmed the issue. I suspect the issue is related to some esoteric platform/CLR run-time specific issue.

BBQ time.

Thanks, Brian

:raised_hands: :olderman: [**"A pair of hard working hands. Everything that I needed. I got it from the old man..."**_](https://www.youtube.com/watch?v=lGt54Ozo8LQ)

bchavez commented 8 years ago

Hi Thomas,

I've changed up the ReqlDateTimeConverter a little bit upon a deeper investigation :mag: of a unrelated CoreCLR cross-platform Windows vs Linux issue.

Basically, I've shunted the DateTime.MinValue -> DateTimeOffset.MinValue without an intermediate conversion though new DateTimeOffset() constructor. The DateTimeOffset() constructor uses OS-timezone information which is subtlety different between operating systems especially for date-times before 1845. Now MinValue is consistent between platforms. I don't know if this will solve your issue, but let me if it does. :smile_cat:

v2.3.4-beta-4 is now available with these changes.

Thanks, Brian

:collision: :dizzy: _"Crashing, hit a wall. Right now I need a miracle..."_

ThomasHoest commented 8 years ago

Hi Brian

Thanks for letting me know I will give the new version a test.

A note on the previous test. It fails when have set my timezone to UTC+2 the test succeeds when i have it set to UTC-7 (which I assume is yours).

Thanks, Thomas

bchavez commented 8 years ago

@ThomasHoest Ah, that's probably it then. Maybe DateTimeOffset conversion was trying to push MinValue down further out of range of MinValue based on timezone info and resulted in ArgumentOfRangeException.

Let me know how it goes. Don't forget to hit the close button with vengeance if your unit test passes!

Thanks, Brian

:car: :bluecar: [**"Let the good times roll..."**_](https://www.youtube.com/watch?v=7BDBzgHXf64)

bchavez commented 8 years ago

Hola Thomas,

Hope you are doing well. It's been a few days, so I'm going to close the issue but if you still encounter any problems, please feel free to reach out on slack / @bchavez or re-open the issue if the problem persists.

Tak, Brian

:chocolate_bar: :cookie: :lollipop: _Ronald Jenkees - Stay Crunchy_

bchavez / RethinkDb.Driver

ArgumentOfRangeException when inserting objects with DateTime.MinValue #66