Exception inside SocketClient.ProcessReceive is not handled

[gribunin]

Hello, yesterday it seems IqFeed sent wrong data which caused the following unhandled exception to be generated:

Application: IqFeedManager.exe
CoreCLR Version: 5.0.20.51904
.NET Version: 5.0.0
Description: The process was terminated due to an unhandled exception.
Exception Info: System.OverflowException: Value was either too large or too small for an Int32.
   at System.Number.ThrowOverflowOrFormatException(ParsingStatus status, TypeCode type)
   at System.Number.ParseInt32(ReadOnlySpan`1 value, NumberStyles styles, NumberFormatInfo info)
   at IQFeed.CSharpApiClient.Lookup.Historical.Messages.TickMessage.Parse(String message)
   at IQFeed.CSharpApiClient.Lookup.Common.BaseLookupMessageHandler.ProcessMessages[T](Func`2 parserFunc, Func`2 errorParserFunc, Byte[] message, Int32 count)
   at IQFeed.CSharpApiClient.Lookup.Historical.HistoricalMessageHandler.GetTickMessages(Byte[] message, Int32 count)
   at IQFeed.CSharpApiClient.Lookup.Common.BaseLookupFacade.<>c__DisplayClass4_0`1.<GetMessagesAsync>g__SocketClientOnMessageReceived|1(Object sender, SocketMessageEventArgs args)
   at IQFeed.CSharpApiClient.Socket.SocketClient.ProcessReceive(SocketAsyncEventArgs e)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading._IOCompletionCallback.PerformIOCompletionCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* pNativeOverlapped)

The exception has been generated in the _IOCompletionCallback thread and wasn't handled in that thread, so it became unhandled exception and crashed the service.

Wrapping the API call which generated the IQFeed call above doesn't catch this exception:

try {
 ... await lookupClient.Historical.GetHistoryTickDatapointsAsync(...
} catch (Exception ex) {
...
 // wasn't called
}

obviously because there is no any code which marshals this exception from the _IOCompletionCallback thread to the context of calling function.

[mathpaquette]

@gribunin do you remember the ticker? Are you able to reproduce?

[gribunin]

As I said it was probably IqFeed who sent the bad data. I do have requested tickers in my logs, but they receive good replies when being requested at the moment. I think it was something going on IqFeed side exactly at that moment (Jan 27, 5:35 PM EST), because the service continue to crash after automatic restart several times.

Nevertheless we must be somehow prepared for such situation (bad data from the vendor) and have an ability to handle it gracefuly. I am going to make a fix in a day or so, can file it as a PR.

[mathpaquette]

@gribunin absolutely this is bad data from the vendor. I dont want to change the types as already mention in another discussion. What are you suggesting ? Using TryParse instead like we do for streaming data? If it doesnt parse properly we need to be aware in other to drop the bad data.

[gribunin]

I am suggesting to have a global try/catch in SocketClient.ProcessReceive, also add an event handler OnException to SocketClient, pass the caught exception object to this handler. Then subscribe to this event handler (OnException) in GetMessagesAsync and rethrow the exception passed to this handler there.

This way, the calls to lookupClient.Historical.GetHistoryTickDatapointsAsync and other API methods wrapped to try/catch will catch these exceptions.

[mathpaquette]

I am suggesting to have a global try/catch in SocketClient.ProcessReceive, also add an event handler OnException to SocketClient, pass the caught exception object to this handler. Then subscribe to this event handler (OnException) in GetMessagesAsync and rethrow the exception passed to this handler there.

This way, the calls to lookupClient.Historical.GetHistoryTickDatapointsAsync and other API methods wrapped to try/catch will catch these exceptions.

not sure I like this solution. Maybe converting all Parse to TryParse makes more sense to me.

[gribunin]

If convert Parse to TryParse what is going to be done when TryParse fails? The calling method will throw an exception?

[mathpaquette]

If convert Parse to TryParse what is going to be done when TryParse fails? The calling method will throw an exception?

not at all.... TryParse doesnt throw.

[mathpaquette]

@gribunin please., lets start with this first. create a PR converting double.Parse DateTime.ParseExact int.Parse like we do in UpdateSummaryMessage

[gribunin]

I see what you mean. Do you think its a correct approach to swallow bad data from iqfeed and just return zeros in the fields which couldnt be parsed? How will a client know that its not a correct TickMessage and that actually bad data has been received?

[gribunin]

Added a pull request (https://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/97) with my idea to catch exceptions and marshal them to the caller context. This way the caller can catch the situation when IqFeed returned bad data, handle this situation as the caller needs. The thrown exception also provides additional information about what exactly failed to be parsed as long as context information (request, received message -- same as in other IQFeedExceptions).

[mathpaquette]

@gribunin I'm about to push a slightly enhanced version as well. I'll add you as a reviewer.

[mathpaquette]

@gribunin please check my PR.