Skip to content

Conversation

@JimDaly
Copy link
Contributor

@JimDaly JimDaly commented Apr 8, 2022

Please see #7777

Summary

Changed the Remarks for the GetResult method.

Fixes #Issue_Number (if available)

@JimDaly JimDaly requested a review from a team as a code owner April 8, 2022 23:03
@opbld34
Copy link

opbld34 commented Apr 8, 2022

Docs Build status updates of commit a51ab92:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Runtime.CompilerServices/ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@BillWagner
Copy link
Member

ping @stephentoub for comments.

@stephentoub
Copy link
Member

No one should ever be calling .ConfigureAwait(anything).GetAwaiter().GetResult(); there's literally zero benefit to using that over .GetAwaiter().GetResult(). The purpose of ConfigureAwait is to influence how the {Unsafe}OnCompleted method that the compiler binds to with await behaves, and it has no impact on GetResult. So we shouldn't be updating the docs for the configured awaiters here; they are accurate, they are intended only for compiler use.

Now, there's the related question of "what about GetAwaiter().GetResult() on a Task rather than on a configured awaiter, as those docs also say the same thing". When all of this support was introduced, the intent was in fact that no one should ever be using these directly, that they were purely for compiler consumption. However, there's a behavioral difference between task.Wait() and task.GetAwaiter().GetResult(), which is that, while functionally identical in terms of how they wait, the latter propagates the first exception that faulted/canceled the task whereas the former wraps the one or more exceptions into an AggregateException and throws that wrapper. As such, developers that want the original exception propagated often use .GetAwaiter().GetResult() instead of Wait. At this point, the right thing for TaskAwaiter and TaskAwaiter<T> is probably to just strike those sentence from the docs, though some extra verbiage around the distinction from Task.Wait would be welcome. No one should be using GetAwaiter().GetResult() on any of the ValueTask variants for all the reasons discussed in https://devblogs.microsoft.com/dotnet/understanding-the-whys-whats-and-whens-of-valuetask/.

JimDaly added a commit to JimDaly/dotnet-api-docs that referenced this pull request Apr 18, 2022
@stephentoub 

Per comments in dotnet#7941 , removing remarks that say 'This method is intended for compiler use rather than for use in application code.'

> At this point, the right thing for TaskAwaiter and TaskAwaiter<T> is probably to just strike those sentence from the docs, though some extra verbiage around the distinction from Task.Wait would be welcome
JimDaly added a commit to JimDaly/dotnet-api-docs that referenced this pull request Apr 18, 2022
@stephentoub 

Per comments in dotnet#7941 , removing remarks that say 'This type and its' members are intended for compiler use...

> At this point, the right thing for TaskAwaiter and TaskAwaiter<T> is probably to just strike those sentence from the docs, though some extra verbiage around the distinction from Task.Wait would be welcome
This was referenced Apr 18, 2022
@JimDaly
Copy link
Contributor Author

JimDaly commented Apr 18, 2022

Closing this as @stephentoub mentioned it shouldn't be used.

@JimDaly JimDaly closed this Apr 18, 2022
This was referenced Apr 19, 2022
gewarren pushed a commit to JimDaly/dotnet-api-docs that referenced this pull request Apr 19, 2022
@stephentoub 

Per comments in dotnet#7941 , removing remarks that say 'This type and its' members are intended for compiler use...

> At this point, the right thing for TaskAwaiter and TaskAwaiter<T> is probably to just strike those sentence from the docs, though some extra verbiage around the distinction from Task.Wait would be welcome
@JimDaly JimDaly deleted the patch-1 branch May 7, 2022 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants