Skip to content

Commit 5dcc52b

Browse files
Sergio0694Sergio0694
authored
Merge pull request #113 from CommunityToolkit/dev/task-await-propagate-unobserved
Propagate task exceptions to TaskScheduler.UnobservedTaskException
2 parents 90cf344 + 2ca1ac7 commit 5dcc52b

File tree

10 files changed

+351
-36
lines changed

10 files changed

+351
-36
lines changed

‎CommunityToolkit.Mvvm.SourceGenerators/EmbeddedResources/INotifyPropertyChanged.cs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -389,13 +389,7 @@ private bool SetPropertyAndNotifyOnCompletion<TTask>(ITaskNotifier<TTask> taskNo
389389

390390
async void MonitorTask()
391391
{
392-
try
393-
{
394-
await newValue!;
395-
}
396-
catch
397-
{
398-
}
392+
await global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__TaskExtensions.GetAwaitableWithoutEndValidation(newValue!);
399393

400394
if (ReferenceEquals(taskNotifier.Task, newValue))
401395
{

‎CommunityToolkit.Mvvm.SourceGenerators/EmbeddedResources/ObservableObject.cs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -437,13 +437,7 @@ private bool SetPropertyAndNotifyOnCompletion<TTask>(ITaskNotifier<TTask> taskNo
437437

438438
async void MonitorTask()
439439
{
440-
try
441-
{
442-
await newValue!;
443-
}
444-
catch
445-
{
446-
}
440+
await global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__TaskExtensions.GetAwaitableWithoutEndValidation(newValue!);
447441

448442
if (ReferenceEquals(taskNotifier.Task, newValue))
449443
{

‎CommunityToolkit.Mvvm/ComponentModel/ObservableObject.cs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
using System.Diagnostics.CodeAnalysis;
2020
using System.Runtime.CompilerServices;
2121
using System.Threading.Tasks;
22+
using CommunityToolkit.Mvvm.ComponentModel.__Internals;
23+
24+
#pragma warning disable CS0618
2225

2326
namespace CommunityToolkit.Mvvm.ComponentModel;
2427

@@ -525,14 +528,8 @@ private bool SetPropertyAndNotifyOnCompletion<TTask>(ITaskNotifier<TTask> taskNo
525528
// which would result in a confusing behavior for users.
526529
async void MonitorTask()
527530
{
528-
try
529-
{
530-
// Await the task and ignore any exceptions
531-
await newValue!;
532-
}
533-
catch
534-
{
535-
}
531+
// Await the task and ignore any exceptions
532+
await newValue!.GetAwaitableWithoutEndValidation();
536533

537534
// Only notify if the property hasn't changed
538535
if (ReferenceEquals(taskNotifier.Task, newValue))

‎CommunityToolkit.Mvvm/ComponentModel/__Internals/__TaskExtensions.cs

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.ComponentModel;
7+
using System.Runtime.CompilerServices;
8+
using System.Threading.Tasks;
9+
10+
namespace CommunityToolkit.Mvvm.ComponentModel.__Internals;
11+
12+
/// <summary>
13+
/// An internal helper used to support <see cref="ObservableObject"/> and generated code from its template.
14+
/// This type is not intended to be used directly by user code.
15+
/// </summary>
16+
[EditorBrowsable(EditorBrowsableState.Never)]
17+
[Obsolete("This type is not intended to be used directly by user code")]
18+
public static class __TaskExtensions
19+
{
20+
/// <summary>
21+
/// Gets an awaitable object that skips end validation.
22+
/// </summary>
23+
/// <param name="task">The input <see cref="Task"/> to get the awaitable for.</param>
24+
/// <returns>A <see cref="TaskAwaitableWithoutEndValidation"/> object wrapping <paramref name="task"/>.</returns>
25+
[EditorBrowsable(EditorBrowsableState.Never)]
26+
[Obsolete("This method is not intended to be called directly by user code")]
27+
public static TaskAwaitableWithoutEndValidation GetAwaitableWithoutEndValidation(this Task task)
28+
{
29+
return new(task);
30+
}
31+
32+
/// <summary>
33+
/// A custom task awaitable object that skips end validation.
34+
/// </summary>
35+
[EditorBrowsable(EditorBrowsableState.Never)]
36+
[Obsolete("This type is not intended to be called directly by user code")]
37+
public readonly struct TaskAwaitableWithoutEndValidation
38+
{
39+
/// <summary>
40+
/// The wrapped <see cref="Task"/> instance to create an awaiter for.
41+
/// </summary>
42+
private readonly Task task;
43+
44+
/// <summary>
45+
/// Creates a new <see cref="TaskAwaitableWithoutEndValidation"/> instance with the specified parameters.
46+
/// </summary>
47+
/// <param name="task">The wrapped <see cref="Task"/> instance to create an awaiter for.</param>
48+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
49+
public TaskAwaitableWithoutEndValidation(Task task)
50+
{
51+
this.task = task;
52+
}
53+
54+
/// <summary>
55+
/// Gets an <see cref="Awaiter"/> instance for the current underlying task.
56+
/// </summary>
57+
/// <returns>An <see cref="Awaiter"/> instance for the current underlying task.</returns>
58+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
59+
public Awaiter GetAwaiter()
60+
{
61+
return new(this.task);
62+
}
63+
64+
/// <summary>
65+
/// An awaiter object for <see cref="TaskAwaitableWithoutEndValidation"/>.
66+
/// </summary>
67+
public readonly struct Awaiter : ICriticalNotifyCompletion
68+
{
69+
/// <summary>
70+
/// The underlying <see cref="TaskAwaiter"/> inistance.
71+
/// </summary>
72+
private readonly TaskAwaiter taskAwaiter;
73+
74+
/// <summary>
75+
/// Creates a new <see cref="Awaiter"/> instance with the specified parameters.
76+
/// </summary>
77+
/// <param name="task">The wrapped <see cref="Task"/> instance to create an awaiter for.</param>
78+
public Awaiter(Task task)
79+
{
80+
this.taskAwaiter = task.GetAwaiter();
81+
}
82+
83+
/// <summary>
84+
/// Gets whether the operation has completed or not.
85+
/// </summary>
86+
/// <remarks>This property is intended for compiler user rather than use directly in code.</remarks>
87+
public bool IsCompleted
88+
{
89+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
90+
get => taskAwaiter.IsCompleted;
91+
}
92+
93+
/// <summary>
94+
/// Ends the await operation.
95+
/// </summary>
96+
/// <remarks>This method is intended for compiler user rather than use directly in code.</remarks>
97+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
98+
public void GetResult()
99+
{
100+
}
101+
102+
/// <inheritdoc/>
103+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
104+
public void OnCompleted(Action continuation)
105+
{
106+
this.taskAwaiter.OnCompleted(continuation);
107+
}
108+
109+
/// <inheritdoc/>
110+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
111+
public void UnsafeOnCompleted(Action continuation)
112+
{
113+
this.taskAwaiter.UnsafeOnCompleted(continuation);
114+
}
115+
}
116+
}
117+
}

‎CommunityToolkit.Mvvm/Input/AsyncRelayCommand.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
using System.Runtime.CompilerServices;
88
using System.Threading;
99
using System.Threading.Tasks;
10+
using CommunityToolkit.Mvvm.ComponentModel.__Internals;
11+
12+
#pragma warning disable CS0618
1013

1114
namespace CommunityToolkit.Mvvm.Input;
1215

@@ -213,13 +216,7 @@ private set
213216

214217
static async void MonitorTask(AsyncRelayCommand @this, Task task)
215218
{
216-
try
217-
{
218-
await task;
219-
}
220-
catch
221-
{
222-
}
219+
await task.GetAwaitableWithoutEndValidation();
223220

224221
if (ReferenceEquals(@this.executionTask, task))
225222
{

‎CommunityToolkit.Mvvm/Input/AsyncRelayCommand{T}.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
using System.Runtime.CompilerServices;
88
using System.Threading;
99
using System.Threading.Tasks;
10+
using CommunityToolkit.Mvvm.ComponentModel.__Internals;
11+
12+
#pragma warning disable CS0618
1013

1114
namespace CommunityToolkit.Mvvm.Input;
1215

@@ -197,13 +200,7 @@ private set
197200

198201
static async void MonitorTask(AsyncRelayCommand<T> @this, Task task)
199202
{
200-
try
201-
{
202-
await task;
203-
}
204-
catch
205-
{
206-
}
203+
await task.GetAwaitableWithoutEndValidation();
207204

208205
if (ReferenceEquals(@this.executionTask, task))
209206
{

‎tests/CommunityToolkit.Mvvm.UnitTests/Helpers/TaskSchedulerTestHelper.cs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Threading.Tasks;
7+
8+
namespace CommunityToolkit.Mvvm.UnitTests.Helpers;
9+
10+
/// <summary>
11+
/// A helper class to validate scenarios related to <see cref="TaskScheduler"/>.
12+
/// </summary>
13+
internal static class TaskSchedulerTestHelper
14+
{
15+
/// <summary>
16+
/// A custom <see cref="Delegate"/> for callbacks to <see cref="IsExceptionBubbledUpToUnobservedTaskExceptionAsync(TestCallback)"/>.
17+
/// </summary>
18+
/// <param name="throwAction">An <see cref="Action"/> instance that throws a test exception to track.</param>
19+
/// <param name="completeAction">An <see cref="Action"/> that signals whenever the test has completed.</param>
20+
public delegate void TestCallback(Action throwAction, Action completeAction);
21+
22+
/// <summary>
23+
/// Checks whether a given test exception is correctly bubbled up to <see cref="TaskScheduler.UnobservedTaskException"/>.
24+
/// </summary>
25+
/// <param name="callback">The <see cref="TestCallback"/> instance to use to run the test.</param>
26+
/// <returns>Whether or not the test exception was correctly bubbled up to <see cref="TaskScheduler.UnobservedTaskException"/>.</returns>
27+
public static async Task<bool> IsExceptionBubbledUpToUnobservedTaskExceptionAsync(TestCallback callback)
28+
{
29+
TaskCompletionSource<object?> tcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
30+
string guid = Guid.NewGuid().ToString();
31+
bool exceptionFound = false;
32+
33+
void TaskScheduler_UnobservedTaskException(object? sender, UnobservedTaskExceptionEventArgs e)
34+
{
35+
e.SetObserved();
36+
37+
foreach (Exception exception in e.Exception!.InnerExceptions)
38+
{
39+
if (exception is TestException testException &&
40+
testException.Message == guid)
41+
{
42+
exceptionFound = true;
43+
44+
return;
45+
}
46+
}
47+
}
48+
49+
EventHandler<UnobservedTaskExceptionEventArgs> handler = TaskScheduler_UnobservedTaskException;
50+
51+
TaskScheduler.UnobservedTaskException += handler;
52+
53+
try
54+
{
55+
// Enqueue a continuation that will throw and ignore the returned task. This has
56+
// to be a separate method to ensure the returned task isn't kept alive for longer.
57+
callback(
58+
() => throw new TestException(guid),
59+
() => tcs.SetResult(null));
60+
61+
// Await for the continuation to actually run
62+
_ = await tcs.Task;
63+
64+
// Wait for some additional time to ensure the exception is propagated. This is a bit counterintuitive, but the delay is
65+
// not actually for the event to be raised, but to ensure the task that is throwing has had time to be scheduled and fail.
66+
// The event is raised only when the exception wrapper inside that task is collected and its finalizer has run (that's where
67+
// the logic to raise the event is executed), which is why we're then calling GC.Collect() and GC.WaitForPendingFinalizers().
68+
// That is, we can't use a task completion source from that event, because that event is only guaranteed to actually be raised
69+
// when the finalizer for that task run, which is why we're calling the GC after the delay here.
70+
await Task.Delay(200);
71+
72+
GC.Collect();
73+
GC.WaitForPendingFinalizers();
74+
}
75+
finally
76+
{
77+
TaskScheduler.UnobservedTaskException -= handler;
78+
}
79+
80+
return exceptionFound;
81+
}
82+
83+
/// <summary>
84+
/// A custom exception to support <see cref="IsExceptionBubbledUpToUnobservedTaskExceptionAsync(TestCallback)"/>.
85+
/// </summary>
86+
private sealed class TestException : Exception
87+
{
88+
/// <summary>
89+
/// Creates a new <see cref="TestException"/> instance with the specified parameters.
90+
/// </summary>
91+
/// <param name="message">The exception message.</param>
92+
public TestException(string message)
93+
: base(message)
94+
{
95+
}
96+
}
97+
}

‎tests/CommunityToolkit.Mvvm.UnitTests/Test_AsyncRelayCommand.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.ComponentModel;
88
using System.Threading.Tasks;
99
using CommunityToolkit.Mvvm.Input;
10+
using CommunityToolkit.Mvvm.UnitTests.Helpers;
1011
using Microsoft.VisualStudio.TestTools.UnitTesting;
1112

1213
namespace CommunityToolkit.Mvvm.UnitTests;
@@ -298,4 +299,30 @@ private static async Task Test_AsyncRelayCommand_AllowConcurrentExecutions_TestL
298299

299300
Assert.IsFalse(command.IsRunning);
300301
}
302+
303+
[TestMethod]
304+
public async Task Test_AsyncRelayCommand_ThrowingTaskBubblesToUnobservedTaskException()
305+
{
306+
static async Task TestMethodAsync(Action action)
307+
{
308+
await Task.Delay(100);
309+
310+
action();
311+
}
312+
313+
async void TestCallback(Action throwAction, Action completeAction)
314+
{
315+
AsyncRelayCommand command = new(() => TestMethodAsync(throwAction));
316+
317+
command.Execute(null);
318+
319+
await Task.Delay(200);
320+
321+
completeAction();
322+
}
323+
324+
bool success = await TaskSchedulerTestHelper.IsExceptionBubbledUpToUnobservedTaskExceptionAsync(TestCallback);
325+
326+
Assert.IsTrue(success);
327+
}
301328
}

0 commit comments

Comments
 (0)