RickStrahl / wwDotnetBridge

.NET Interop for Visual FoxPro made easy
http://west-wind.com/wwDotnetBridge.aspx
MIT License
74 stars 35 forks source link

Async callback failure crashes FoxPro #13

Closed breyed closed 6 years ago

breyed commented 6 years ago

If the callback object passed to InvokeMethodAsync doesn't have a proper OnComplete and OnError method, FoxPro crashes. This happens when _InvokeMethodAsync calls InvokeMethod_Internal(cb, "onError", ex.Message, ex.GetBaseException(), method).

There is no final exception handling around the attempt to call OnError. Since the call is no unhandled exception handler for the thread, .NET terminates the process.

RickStrahl commented 6 years ago

This is kind of by design. You're basically implementing a contract so the contract requires that the methods be there. If it fails it should fail - but a hard failure that crashes VFP is probably not quite what I was looking for here either...

breyed commented 6 years ago

I agree. When I was debugging the crash, I saw where I could put a try/catch around the invoke, but I was scratching my head about what to do in the catch block.

How about this? Add an optional parameter to GetwwDotnetBridge, which takes an object on which a OnDotnetBridgeError function will be called for each error. If the parameter isn't passed or OnDotnetBridgeError can't be called, the bridge will output the error to the console as an alternative. Such an approach would allow production apps to capture the error for diagnostics, with reasonable fallback notification for scenarios where setting up an event handler is overkill.

RickStrahl commented 6 years ago

I think if we go down that path then having a oException property on the event handler object would be a better choice. That way the exception is captured and available on the caller. If you don't pass the object or you are not providing the required methods or member- well then you're opting out essentially.

I have to look at the code (been a while) to see exactly what should happen there. I think given that the thread will crash VFP if unhandled the error needs to be handled no matter what.

breyed commented 6 years ago

An application may have many async calls. Considering also event support, there's the question of how to handle an event that is raised for which there is no handler (but was expected to have been) and how to handle an exception that propagates out of an event handler.

In all cases, it's generally an application bug. It would be nice to opt into diagnostics in one place, rather in every async/event handler. Often, the error handling logic is not handler-specific , but rather just logging the error and exiting to avoid data corruption.

RickStrahl commented 6 years ago

So I've added exception handling around the async callback operations so at the very least VFP won't be crashing if an error occurs during callbacks (like invalid signature or missing method or simply a failure in the code).

Maybe start a new issue for the tracking of the exceptions since that's a slightly different and more complicated issue that needs perhaps separate attention.