Skip to content
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.

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

ArgumentOfRangeException when inserting objects with DateTime.MinValue #66

Closed
ThomasHoest opened this issue May 29, 2016 · 8 comments
Closed

Comments

@ThomasHoest
Copy link

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.

@ThomasHoest ThomasHoest changed the title ArgumentOfOfRangeException when inserting objects with DateTime.MinValue ArgumentOfangeException when inserting objects with DateTime.MinValue May 29, 2016
@ThomasHoest ThomasHoest changed the title ArgumentOfangeException when inserting objects with DateTime.MinValue ArgumentOfRangeException when inserting objects with DateTime.MinValue May 29, 2016
@bchavez
Copy link
Owner

bchavez commented May 29, 2016

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

🍂 🍃 _"I get a sense of weightlessness…"_

@bchavez bchavez added the gotcha label May 29, 2016
@bchavez
Copy link
Owner

bchavez commented May 29, 2016

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.

🌙 🌠 _"Nothing good happens past 2am..."_

@ThomasHoest
Copy link
Author

ThomasHoest commented May 29, 2016

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
Copy link
Owner

bchavez commented May 30, 2016

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. 😿 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

🙌 👴 _"A pair of hard working hands. Everything that I needed. I got it from the old man..."_

bchavez added a commit that referenced this issue Jun 1, 2016
bchavez added a commit that referenced this issue Jun 1, 2016
@bchavez
Copy link
Owner

bchavez commented Jun 1, 2016

Hi Thomas,

I've changed up the ReqlDateTimeConverter a little bit upon a deeper investigation 🔍 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. 😸

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

Thanks,
Brian

💥 💫 _"Crashing, hit a wall. Right now I need a miracle..."_

@ThomasHoest
Copy link
Author

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 bchavez added the bug label Jun 1, 2016
@bchavez
Copy link
Owner

bchavez commented Jun 1, 2016

@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

🚗 🚙 _"Let the good times roll..."_

@bchavez
Copy link
Owner

bchavez commented Jun 3, 2016

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

🍫 🍪 🍭 _Ronald Jenkees - Stay Crunchy_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants