Real-Serious-Games / C-Sharp-Promise

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

Attempt to resolve already rejected promise when combining All and Sequence #64

Closed atraver closed 6 years ago

atraver commented 6 years ago

I've got a weird situation where I'm getting an exception when my code rejects an asynchronous promise (All) inside a synchronous block (Sequence). The code looks like this:

return GameController.Instance.ApplicationHandshake()
    .Then(() => GameController.Instance.GameHandshake())
    .Then(response => {
        return Promise.Sequence(
            () => Promise.All(
                ContentManager.Instance.LoadAssets(),
                GameController.Instance.Init(response)
                    .Then(() => ContinueLoading())
            ),
            StartGameInternal);
    });

The ContinueLoading method, which is the cause of the issue (although it seems as though it could also occur if LoadAssets throws an exception) looks like this:

private IPromise ContinueLoading()
{
    var storyUi = UIManager.Instance.SpawnPopup<StorySequenceUI>("ui/story/intro_sequence");
    return storyUi.Init()
        .Then(() => this.splashScreenGO.SetActive(true));
}

To test the issue, I'm explicitly Rejecting in an OnClick handler (we're using Unity) in StorySequenceUI. When I do this, the exception bubbles up properly, but I get a second exception as well:

System.ApplicationException: Attempt to resolve a promise that is already in state: Rejected, a promise can only be resolved when it is still in state: Pending`

Is this because Init in the first Sequence function has already rejected, but LoadAssets is trying to resolve? To be clear, in my case, I'm explicitly Rejecting inside a function called by the GameController.Instance.Init(response).Then(() => ContinueLoading()) block, but ContentManager.Instance.LoadAssets finishes after and resolves properly.

Is this a case where something like ThenRace would be better suited? Maybe I'm totally misunderstanding what is/should be happening here when one of the promises in an All block rejects but the others resolve.

RoryDungan commented 6 years ago

Thanks, yep I tested it and that is a bug. If one item of a Promise.All rejects, the whole thing should reject without triggering the unhandled exception handler or attempting to resolve any rejected promises. Same with Promise.Sequence.

I'm working on fixing it now so the fix will be in the next release.

atraver commented 6 years ago

Dude, you guys rock. I've converted so much of my old Coroutine/Update loop stuff to Promises since I found your project -- it mirrors most of what we do on the server side with node.js so it just makes a lot more sense. Looking forward to the next release!

ashleydavis commented 6 years ago

Please spread the word and if anyone wants an introduction please point them at my web site.

http://www.what-could-possibly-go-wrong.com/promises-for-game-development/

RoryDungan commented 6 years ago

I've fixed the issue and pushed a new release now. Let me know if it still happens with the latest version of the library.