Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Task Extensions .Await completed callback can trigger error callback #2804

Open
Binderbound opened this issue Feb 1, 2023 · 3 comments
Labels

Comments

@Binderbound
Copy link

Binderbound commented Feb 1, 2023

Description

When Await() is used with a callback and errorCallback, an exception in the callback can trigger an errorCallback.
I think this is kind of counterintuitive - the errorCallback should be about the task you're awaiting on only, and you should either
A) get some kind of guarantee that only the completed callback or the error callback is going to be triggered or
B) completed callback will always be run, and error will only be run if there's an error in the Task

Steps to Reproduce

Console.WriteLine("Hello, World!");
Task.Delay(1000).Await(()=> {
    Console.WriteLine("completed");
    throw new Exception("Unrelated exception");
}, ex=>Console.WriteLine(ex.Message), true);
Console.ReadKey();

Platform with bug

Prism Core

Affected platforms

Windows

Did you find any workaround?

No workaround, but it should be easily resolved by moving completedCallback?Invoke() line outsideof the try/catch and adding a return after errorCallback invoke OR moving the completed callback to a finally block, depending on what behaviour is desired.

Relevant log output

Hello, World!
completed
Unrelated exception
@brianlagunas
Copy link
Member

In case of an exception the completed callback should not fire at all, only the error call back.

Would you like to submit a PR?

@Binderbound
Copy link
Author

Can do
I think this could be done as a shorthand wrapper around Task.ContinueWith as well. What approach would you prefer?

@brianlagunas
Copy link
Member

Please avoid using ContinueWith.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants