When should we use Done? about c-sharp-promise

[ashleydavis]

I actually think it is always worthwhile to add a Done even if you already have a Catch.

LIke you said a Catch can throw an exception and you won't know about it unless you have a Done. So that's a spot where your code can go wrong silently - never a good situation.

An alternative is to make sure you always setup a global error handler.

You can easily do this in Unity like this:

Promise.UnhandledException += (s, ev) => 
{
    Debug.LogError("Unhandled exception ocurred in promise chain.\r\n" + ev.Exception.ToString());
};

It's a good idea to have this global protection against silent errors. The only problem is that you don't know which promise chain it came from! So better protection, but still not an ideal solution.

So it seems always useful to always include a call to Done.

This is also a good way of expressing your intent which makes for clearer communication between coders (or between you and your future self). In this way using Done is communicating that you have finished with a promise chain.

[RoryDungan]

.Done ensures that any exceptions are handled and terminates the promise chain, so is used for the end of a chain that you don't plan on chaining anything else after. It's not usually necessary if you also have a .Catch, since its only purpose there would be to catch an exception thrown within the .Catch.

Make sure that you put a .Done or a .Catch on the end of your promise chains though, because if neither is present your exception can be swallowed and disappear.

[RoryDungan]

+1 yeah, those are definitely good ideas

[nah0y]

So a "correct" workflow, would be something like that, right?

myPromise
.Then()
.Catch()
.Done()

Also yes thanks for the global error handler, I have to put that, even if it's for logs only, so I can see that something is missed somewhere :)

And sorry but you said ".Done ensures that any exceptions are handled", but Done does not return an exception, so... what happens when I have an exception and I only use Then and Done. Would the exception trigger the Done callback with a null parameter or something like that?

[RoryDungan]

Actually the .Done is what sends the exception to the unhanded exception handler. Here's a simple example of where we could reject a promise, and have the exception disappear:

var promise = new Promise();

Promise.UnhandledException += (sender, args) => Console.WriteLine(args.Exception.Message);

Exception expectedException = new Exception("test");
promise.Reject(expectedException);
// Even though we rejected the promise, our call to Console.WriteLine is never reached

However, if we added promise.Done(); after the promise.Reject then the exception will be sent to our unhanded exception handler.

As a broad rule of thumb, always use a .Done() on the end of a promise chain except in a function where you're returning the promise and may want to chain something else after it.

Typically I only ever use .Done() without any arguments, but there are also versions that take either an onResolved action or an onResolved and onRejected action. Essentially these are just the same as a Then before a Done:

// These two examples do exactly the same thing
promise
    .Then(() => DoSomething())
    .Catch(ex => LogError(ex))
    .Done();

promise
    .Done(() => DoSomething(), ex => LogError(ex));

[nah0y]

Okay, thanks for explaining!
But... :)

I mean, in your first example where you don't use Done, I think it should by design always call the UnhandledException, even if you don't use Done. Why is it like that by design, is there a particular reason? If I throw an exception, it should be handled, even if I didn't added a Done.

Also you said that if we add promise.Done after promise.Reject the exception will be sent. But how is this possible since we're calling Done after sending the Exception with Reject. Shouldn't we add Done before Reject?

I'm just talking in a "logical" sense, what I think is logic to do/use at first glance for me.

Okay, so if I understand everything correctly, I should use Then/Catch and also add a Done so it can trigger the UnhandledException if there's a new exception inside my Catch. Is that correct?

[ashleydavis]

Thanks @RoryDungan I'd forgotten that you actually have to call Done in order to sequence the unhandled exception handler (it's basically just setting up a final Catch for you). This means that it is definitely essential that you terminate your promise chains with Done.

To @nah0y, we would have loved to automatically call the unhandled exception handler with or without Done. Unfortunately there is no reliable way to do this. Done explicitly passes the error to the unhandled exception handler. How else would the error be passed on if not for Done? If you can come with some magic solution that would be awesome ;)

@nah0y In answer to your second query. Promises are generally (although not necessarily always) resolved or rejected after all calls to Then, Catch and Done. That's because we are talking about asynchronous coding here. The work that the promise is doing continues after the promise chain has been defined. If it wasn't asynchronous in this way there would not be a need for Then, Catch and Done, because we'd be doing normal synchronous coding. So Done, as with Then and Catch, can be called before or after resolving or rejecting a promise. That's simply the nature of the way this works and asynchronous coding is far from logical, but this is how we manage it.

Always add a call to Done, let's just say that's best practice :)

[nah0y]

"we would have loved to automatically call the unhandled exception handler with or without Done. Unfortunately there is no reliable way to do this. Done explicitly passes the error to the unhandled exception handler. How else would the error be passed on if not for Done? If you can come with some magic solution that would be awesome ;)"
Well, you could simple do that in the Catch or Then callback?
When you detect a Catch or Then or anything, you register for the unhandled exception?
You also register in the done, but only if it wasn't first registered in the Catch/Then/...

But maybe I'm missing something :D

[ashleydavis]

I think you are missing something.

Until you call Done there is simply no way to know if the user has finished with the promise. You can't detect an unhandled exception unless you know that they are done with it: and therefore not going to handle the exception at some time in the future.

There is nothing to stop a user from adding more Catch handlers to a promise chain at some later point. So at any point (unless there is a Done which terminates further chaining) you can't assume that an exception will never be handled. There is no general way of knowing if a user has finished with a promise.

We did think of maybe adding a timer that waits X seconds after initiation of the asynchronous operation and then interpreting any outstanding exception as unhandled and reporting it via the unhandled error mechanism. But how do you choose a value for X? That's really arbitrary and whatever value we chose is bound to be wrong for someone else. Not to mention that this would make the promises API unnecessarily complex.

So the question you need to answer is how does the promises API know when a user is finished with a promise chain. They aren't finished with it after a call to Then or Catch, because that would mean you can't chain additional callbacks.

Incidentally the official JavaScript API doesn't solve this problem either, although some of the implementations do allow use of the done pattern.

Asynchronous coding is tricky and even trickier to explain to someone :) Hope this helps clear it up a bit.

[ashleydavis]

I should also add this problem kind of goes way with the async / await feature that is now in C# and hopefully we'll get one day in Unity.

This is implemented with the help of the compiler.... which does the magic for you to route any occuring exception back to your code so you can handle it in the same way way that you'd handle any synchronous exception in C#.

Under the hood in C# this uses tasks which are very similar to promises.

The same feature is also coming in JavaScript. Under the hood in JavaScript this feature is implemented by promises and so use of async / await and promises are interchangeable which will be very nice for porting over old code.

[bendangelo]

This info should be added somewhere in the readme. I didn't know every promise should always have a Done() at the end. This would have saved me so much time debugging disappearing errors.

[RoryDungan]

This is mentioned in the unhandled errors section of the readme, as well as in the examples.

The reason it isn't mentioned right at the start of the readme is that the concept of the .Done() function and the unhandled exception callback doesn't make any sense unless you understand how promises work in general first. I'm happy to accept pull requests for the readme if you think it isn't obvious enough though.