dougdellolio / coinbasepro-csharp

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

Certain methods does not work in Windows 7 #285

Closed prozerran closed 3 years ago

prozerran commented 3 years ago

I have my project working good in Win10, but once i test it on my "production" server, a Win7 machine, it crashes and certain methods dont work. I made sure to update all the .NET framework requirements, fully update windows, etc. But it still crash... I haven't fully tested everything, but at least this method doesnt work....

        private async Task CoinbaseGetOrderBook(string prodId)
        {
            ProductsOrderBookResponse res = null;

            try
            {
                Log.Information($"GetProductOrderBookAsync [{prodId}]");
                res = await coinbaseProClient.ProductsService.GetProductOrderBookAsync(prodId, ProductLevel.Two);
            }
            catch (Exception e)
            {
                Log.Error($"CoinbaseGetOrderBook.Exception {e.Message}");
                Log.Error($"CoinbaseGetOrderBook.Exception {e.StackTrace}");
            }
           ...
        }`


The exception is caught and this is what i get from the log....

2021-04-21 19:30:49.103 +08:00 [INF] GetProductOrderBookAsync [BTC-USD]
2021-04-21 19:30:49.103 +08:00 [ERR] CoinbaseGetOrderBook.Exception An error occurred while sending the request.
2021-04-21 19:30:49.103 +08:00 [ERR] CoinbaseGetOrderBook.Exception    at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbasePro.Network.HttpClient.HttpClient.<SendAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbasePro.Network.HttpClient.HttpClient.<SendAsync>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbasePro.Services.AbstractService.<SendHttpRequestMessageAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbasePro.Services.AbstractService.<SendServiceCall>d__6`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbasePro.Services.Products.ProductsService.<GetProductOrderBookAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at CoinbaseEngine.CoinbasePrice.<CoinbaseGetOrderBook>d__14.MoveNext()

As said, works in Win10, but not Win7 (probably also doesnt work in other older versions of windows)
The code that throws the exception is in ProductsService.GetProductOrderBookAsync()
(maybe alot more other methods....)

One way to easily reproduce this is setup a Win7 VM in virtualbox, etc....
dougdellolio commented 3 years ago

hey @prozerran it's hard to say what the problem is here. I don't have access to a windows 7 machine to test this out. But I would recommend probably not using Windows 7 to run your production version as Microsoft stopped support back in January of 2020. This could be a significant security risk. Any chance to upgrade the OS?

I have searched around a bit for any explanations but haven't seen much. Will let you know if I come across anything else.

prozerran commented 3 years ago

Cant you just build a Win7 machine on VirtualBox, just download a trial Win7 OS. I compiled my code in Win10, but once i deploy to Win7, it doesnt work. I tried on 4 Win7 OS, and all doesnt work (2 production Win7 and 2 VMs, all dont work)

I think this is a serious enough issue for you to look at, since you cant expect your library to only work in Win10.

prozerran commented 3 years ago

Ok, after some investigation, this can be fixed by adding... ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12;

This should be in the init or the very start of the program. Other coinbase libs also mention this is needed, so you can verify and test yourself, then add this to your readme file. Aparently, its needed for all other version of windows, but not Win10.

dougdellolio commented 3 years ago

I'm not convinced that setting the security protocol to a specific version is the best solution here as this is not really a problem with the library code rather a problem with what your OS is defaulted to. .NET uses the OS to choose the latest and best security protocol and changing the code here will always keep it at TLS1.2 (what happens when next version comes?)

According to the docs, TLS 1.2 is supported with Windows 7 but it's not set as the default by your machine so I think the real fix here if you want to use Windows 7 is to enable TLS 1.2 on your OS. Your code would then choose to use that as the protocol.

There are some solutions in this thread here if you want to read more about it. Let me know if this helps your issue: https://github.com/dotnet/docs/issues/13787

more here too: https://docs.microsoft.com/en-us/dotnet/framework/network-programming/tls#support-for-tls-12

prozerran commented 3 years ago

ok. i'll close it and adjust things on my end. But you should probably mention this issue in the readme so other people dont fall into this trap like i did. closing it.