Nikey646 / VndbSharp

A C# Vndb API Library. #OriginalNamingScheme
MIT License
18 stars 4 forks source link

DateTimeOffset uses improper Date/Time #38

Closed micah686 closed 7 years ago

micah686 commented 7 years ago

I've noticed while testing out my throttling checking code that sometimes the throttled.Minimum/MaximumWait reverts to 1/1/0001 12:00:00 AM +00:00, instead of the proper time. This causes errors when running something like throttled.MinimumWait - DateTime.Now).TotalSeconds, as it creates a large neagtive number that causes problems.

I've encountered the error on https://github.com/Nikey646/VndbSharp/blob/master/VndbConsoleCore/Program.cs#L705.

I've also noticed it on DurationToDateTimeOffsetConverter's ReadJson(..., Object existingValue,...){}, where existingValue is the date I mentioned earlier.

I'm not sure where the value is originating, but it has been causing some issues.

Nikey646 commented 7 years ago

The value is the default when you new up a DateTimeOffset. To be precise, running new DateTimeOffset().ToString() returns 0001-01-01T00:00:00+00:00 (With the last bit, +00:00 being the timezone it was created from)

I'll need the response from when it screws up to figure out why it's screwing up, without that i cannot really test anything since all i know is that the value(s) are never set for some reason.

micah686 commented 7 years ago

Get Response | error {"minwait":1,"type":"cmd","id":"throttled","fullwait":601,"msg":"Throttle limit reached."} {"minwait":1,"type":"cmd","id":"throttled","fullwait":601,"msg":"Throttle limit reached."}

Json of throttled Error: {\"type\":1,\"minwait\":\"0001-01-01T00:00:00+00:00\",\"FullWait\":\"0001-01-01T00:00:00+00:00\",\"id\":5,\"msg\":\"Throttle limit reached.\"}

Here are some snippets from Program.cs that I used to get this error:

public class RequestOptions : IRequestOptions
    {
        public int? Page { get; set; }
        public int? Count { get; set; }
        public string Sort { get; set; }
        public bool? Reverse { get; set; }
    }
public async Task GetVisualNovelListAsync()
        {           
            bool hasMore = true;
            RequestOptions ro = new RequestOptions();
            int page = 1;
            VndbResponse<VisualNovelList> vnList = null;

            while (hasMore)
            {
                ro.Page = page;
                vnList = await this._client.GetVisualNovelListAsync(VndbFilters.UserId.Equals(7887), VndbFlags.FullVisualNovelList, ro);
                if (vnList == null)
                {
                    this.HandleError(this._client.GetLastError());                  
                }
                if (vnList != null)
                {
                    hasMore = vnList.HasMore;
                    page++;
                }
            }
        }
private void HandleError(IVndbError error)
{
...
 if (error is ThrottledError throttled)
            {
                var minSeconds = (throttled.MinimumWait - DateTime.Now).TotalSeconds; 
                var fullSeconds = (throttled.FullWait - DateTime.Now).TotalSeconds; 
                Console.WriteLine(
                    $"A Throttled Error occured, you need to wait at minimum \"{minSeconds}\" seconds, " +$"and preferably \"{fullSeconds}\" before issuing commands.");
                if (throttled.MinimumWait.Year < 2000)
                {
                    Console.WriteLine("error message is: "+error.Message);
                }
            }
}

I hope that's enough for you to help find where the error occurs. The user has hundreds of items on their list, so it should trigger a throttled error each time you try.

Nikey646 commented 7 years ago

The primary issue for this is because the API returned an integer, while the documentation (imo) implied that the result would always be a decimal. So in the converter only checked for Decimals/Floats/Doubles, and not Integers, so when the API returned an Integer the converter ignored it as if it was invalid input. For now, i've created a fix via allowing Integers as well and converting it to a string to parse it (There is probs a better way, tbh) and asked if this is intentional.

micah686 commented 7 years ago

I still/Now get a VndbSharp.UnexpectedResponseException when looping through like before.

Here is a gist of my program.cs, so you can replicate the issue that I am encountering: https://gist.github.com/micah686/836655f337bccf7be9449f343377e8b6

Nikey646 commented 7 years ago

I didn't want to have to needlessly send requests against the Vndb Server, but it appears i have no choice.

It also appears that they were needless requests, since the new converter works, as evidenced:

That was done with a freshly compiled version of VndbSharp, in the VndbConsole(/Core) project, using the HandleError and GetVisualNovelListAsync methods of yours (Although, renamed).

Please ensure that you're using the latest version of the library, and try again :)

micah686 commented 7 years ago

I tried redownloading VndbSharp, compiled it again, while including the loop, and the error still occurs. The throttle error repeats successfully many times before this VndbSharp.UnexpectedResponseException occurs.

Here is a log file from the console. https://gist.github.com/micah686/f61ed55b2d760f2f2f01a188520c619c#file-vndbsharpthrottledresponse-log-L354

Around Line 354, the Get Response seems to throw a VndbSharp.UnexpectedResponseException.

Nikey646 commented 7 years ago

This is unrelated to the throttling error, as shown on line 354 no response is actually returned, so the library throws an UnexpectedResponseException

Because this doesn't occur to me (Since it takes upwards of 200 pages for my internet to finally hit the command rate limit), i cannot test to figure out why it does this, but as far as i can tell it appears to be from Vndbs side, since literally no response is returned.