Real-Serious-Games / C-Sharp-Promise

Promises library for C# for management of asynchronous operations.
MIT License
1.19k stars 149 forks source link

Uncaught exceptions are lost #88

Closed alonsohki closed 5 years ago

alonsohki commented 5 years ago

The promise library is right now catching the unhandled exceptions that happen during the execution to mark the promise as rejected. This is good because we can .Catch() the exceptions and handle them, but in case we don't do any .Catch() those exceptions are lost forever and sometimes bugs go unnoticed. Whenever an exception is caught from the Promise library, if there are no Catch() handlers for it it should be rethrown.

RoryDungan commented 5 years ago

This is a known issue with the promise system. The solution we recommend is using the .Done() function to terminate promise chains, in conjunction with adding a handler to the unhandled exception event. You should use the .Done() function even after a .Catch() because it will handle if your callback inside the .Catch() function threw an exception.

alonsohki commented 5 years ago

The main problem are those cases where the programmer misses handling all the promise states, such as:

public IPromise DoSomething() { ... }

...
DoSomething();
...

In this case the programmer didn't even notice that the return value of the function is a promise. They might not even care about the return value because it's not necessary to wait for the function to be resolved. And if something raises an unhandled exception in DoSomething() it will be lost and never logged. What I wonder the most is if it's possible with the current code design to fix this problem, as I don't have yet enough knowledge of the internals of the library to judge this.

ashleydavis commented 5 years ago

It's not a limitation of this library, it's a limitation of the promises design pattern. If you want to do asynchronous programming this is something that you need to accept.

Of course if you can think of a solution, you are welcome to submit a PR.

RoryDungan commented 5 years ago

As an aside, native ES6 promises in Node.js and most major browsers do have a solution for this where unhandled rejections are logged, but I believe it only works because promises are built into the runtime.