dougdellolio / coinbasepro-csharp

The unofficial .NET/C# client library for the Coinbase Pro/GDAX API
MIT License
192 stars 90 forks source link

#ODD - Json deserilizer unable to convert string "3e-7" to decimal - BCH-BTC snapshot response #145

Closed BradForsythe closed 6 years ago

BradForsythe commented 6 years ago

@dougdellolio have you seen exponential notation in the json response string from a snapshot rersponse? I am getting an ask size of "3e-7" around the 30th level and the deserializer is trying to convert it to decimal. I am not super familiar with Newtonsoft but I think this would throw an error and does. Could I be using an older verison? Looks like in the config you have it set to parse floats as decimal, I wondering if this needs to be different. Anyway, just curious. You can test it by subscribing to BCH-BTC Of course it will be gone by the time you test :)

BradForsythe commented 6 years ago

@dougdellolio I'll move to the latest release (which I need to do sooner or later) and see if the problem still exists. So hang tight for now....happy 4th!

dougdellolio commented 6 years ago

weird.... yeah I just tried it and don't see it 😛 let me know if it continues to happen I can try and setup a test around it to see how it behaves.... and thx, you too!

Sotam commented 6 years ago

This does happen on rare occasions, have only seen it a few times 😛

BradForsythe commented 6 years ago

At least I am not going crazy. Yes, it's rare. When it happens, it will continue to throw an exception until that price level is removed. I believe it's because newtonsoft is trying the decode the decimal but it encounters the exponential format and chokes. I haven't been able to trap it, but I am sure with a little work I can log a copy of the json string before deserialization.

chrisw000 commented 6 years ago

Assuming the deserialization is occuring via the static helper method:

CoinbasePro.Shared.Utilities.JsonConfig.DeserializeObject

Then the json value before deserialization should already be logged via the Serilog.Log.Error, which is specified in CoinbasePro.Shared.Utilities.JsonConfig.SerializerSettings

with this bit of code:

Log.Error("Json serialization error {@OriginalObject} {@Member} {@ErrorMessage}"
                                                            , args.ErrorContext.OriginalObject
                                                            , args.ErrorContext.Member
                                                            , args.ErrorContext.Error.Message);

Either that, or this bit of code need tweaking!

BradForsythe commented 6 years ago

Hey guys, I am going to close this for now. I'll use Chris's logging suggestion to see if I can catch it again. Volume has been sloth like and I haven't been hitting it lately. Jumping back in soon...

guanm326 commented 5 years ago

I am getting this quite often now. Its with newtonsoft deserializer i think. As of this moment i am getting it when i subscribe to ethusd snapshot. its on the bid side.

guanm326 commented 5 years ago

i think to replicate this you will need to enter to the sand box market and place an order with size 3e-7. subscribe to it and then the error should appear. I think its more to do with NewtonSoft and their de-serializer....

Here is a link to the problem https://github.com/JamesNK/Newtonsoft.Json/issues/1711

dougdellolio commented 5 years ago

i have just added a fix for this here: https://github.com/dougdellolio/coinbasepro-csharp/pull/176. I want to do some more testing and then will merge. Newtonsoft has fixed this too but it hasn't been released yet. Looks like they haven't released in some time