robotdotnet / NetworkTables

FRC NetworkTables for .NET. This is all old code and should not be used anymore.
3 stars 5 forks source link

NetworkTable.Shutdown throws AggregateException when connecting to an invalid team number #64

Closed jkoritzinsky closed 7 years ago

jkoritzinsky commented 7 years ago

When shutting down NetworkTables, it will throw an AggregateException containing a DNS exception about a host not being found when shutting down network tables after connecting to an invalid host (for example, a team number for a robot that is not present). I expected the Shutdown method to handle all exceptions and make sure everything is shut down before returning.

I have a workaround of calling NetworkTable.Shutdown in a try-catch in a loop until it succeeds, but I'd prefer to not have to do that.

ThadHouse commented 7 years ago

There are 2 Stop functions that get called. One is going to be in the DSClient class and one is going to be in the Dispatcher class. Do you know which one it is crashing in, and do you have a stack trace? I have a feeling its in DSClient since that was recently added.

jkoritzinsky commented 7 years ago

The stack trace I get is minimal and doesn't tell me which method it's crashing in oddly enough. Probably because of the Task class internals or something. I'll try to get you a stack trace next time this bug shows up.

ThadHouse commented 7 years ago

Ok. I have a feeling its in DSClient. What's weird is I am unwrapping all the task exceptions on shutdown, so it shouldn't be throwing that. I'll look into this more too.

jkoritzinsky commented 7 years ago

Its actually not in DSClient. Here's the exception info:

System.AggregateException occurred
  HResult=-2146233088
  Message=One or more errors occurred.
  Source=mscorlib
  StackTrace:
       at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
       at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
       at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout)
       at NetworkTables.TcpSockets.TcpConnector.ResolveHostName(String hostName, IPAddress[]& addr)
       at NetworkTables.TcpSockets.TcpConnector.Connect(String server, Int32 port, Logger logger, Int32 timeout)
       at NetworkTables.Dispatcher.<>c__DisplayClass7_0.<SetServer>b__0()
       at NetworkTables.DispatcherBase.ClientThreadMain()
       at System.Threading.Tasks.Task.InnerInvoke()
       at System.Threading.Tasks.Task.Execute()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
  InnerException: 
       ErrorCode=11001
       HResult=-2147467259
       Message=No such host is known
       NativeErrorCode=11001
       Source=System
       StackTrace:
            at System.Net.Dns.HostResolutionEndHelper(IAsyncResult asyncResult)
            at System.Net.Dns.EndGetHostAddresses(IAsyncResult asyncResult)
            at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
       InnerException: 

Maybe you're trying to catch a SocketException instead of an AggregateException in that function?

jkoritzinsky commented 7 years ago

Also my workaround doesn't actually work, so my need for this to be fixed just went up a lot.

ThadHouse commented 7 years ago

Yup. SocketException is exactly whats getting caught. I'll fix that.

jkoritzinsky commented 7 years ago

Perfect 👌 Can you push an updated beta to nuget once you've fixed this so I can pull it down for DotNetDash?

jkoritzinsky commented 7 years ago

Btw I'd suggest catching both exception types just in case either is thrown.

ThadHouse commented 7 years ago

Fix is pushed. 3.1.4-rc2. It should be on nuget in 5-10 minutes. (Also so happy I can push to nuget now by just taging the repo. Makes things so easy)