@ktoso and I would like to propose changes to the Task API to adopt typed throws and make it more difficult to ignore thrown errors accidentially.
The motivation for adopting typed throws can be clearly illustrated with a code example:
let task = Task {
throw MyError.somethingBadHappened
}
do {
_ = try await task.value
} catch {
// type information has been lost and error is now `any Error`
}
The ability to accidentially ignore errors for a Task with a throwing body has been common problem, as supported by this thread. The proposal removes the @discardableResult that makes this possible. We believe the benefit of eliminating these potentially serious and hard-to-debug problems far outweighs any convenience the current API might offer.
We very much look forward to your feedback!
The full proposal draft is available here:
(For reference, there was also an earlier, less-formal, pitch discussion thread).
We should clarify that the proposal is two changes and not just the discarding change:
adopt typed throws in Task.init and Task.detached
We are able to do this sooner than some other places of the concurrency library. Doing the same for TaskGroup is a goal but more difficult and will be a separate proposal in the future (the difficulty lies in making TaskGroup adopt typed throws while keeping its âgood nameâ).
removing the @discardableResult when the thrown error is not Never
So thatâs the two things weâre proposing here, they go hand in hand somewhat, so itâs in the same proposal
Can you elaborate a bit on why the @discardableResult overloads are available to Task<*, Never> as opposed to just Task<Void, Never> (which truly provides nothing of value out of its value property)? Is it that you expect people to have one-liner tasks that have an implicit return value and the overload resolution canât figure out that the user wants to discard the result of the expression in the closure rather than passing it through the Task machinery? I feel like if you put an explicit return foo in your task we should help you make sure that youâre properly using that result.
Double thumbs up and rainbows all the way across the sky for the @discardableResult portion of this proposal from me.
One of the most fundamental aspects of the Swift design ethos from its inception is providing compile-time guarantees of program correctness that help teams discover issues that other languages quietly hide. It should not be possible to compile unhandled thrown errors.
Huge +1 on the change, errors is valuable information and shouldnât be disregarded implicitly.
IMO it would be better to leave ability to implicitly discard only Task<Void, Never>. Iâm in favor of more explicit stating here, then ignoring value will communicate that this is intentional.
I fully agree that losing any errors thrown in the Task initializers can easily lead to ignoring errors by accident. It's just way too easy to add a throw somewhere inside a Task without much thought, not realizing that the error won't be handled.
However, I wish there was a better way to express the "fire-and-forget" case for tasks in which the desired behavior is to ignore the thrown errors. I feel like understanding why the _ = is needed here:
_ = Task {
try first()
try second()
try third()
}
...requires a somewhat deep understanding of how Tasks work in Swift which may not be all that easy to understand to people new to the language. You need to understand, at a glance:
That Task is a struct (some people may believe that it's a keyword at first).
That Task { ... } is the initializer for said struct, using a trailing closure as an argument.
That Task has two generic parameters, and is actually Task<Success, Failure>.
That a struct with generic parameters like Task may have different initializers based on the type of the generic parameters.
That, by using a throwing function in the closure that initializes the class, the compiler is infering the result to be of type Task<*, Error>.
How @discardableResult works.
And, finally, understand (guess?) that the initializer for Task may be using @discardableResult when the type is infered to be Task<*, Never> and not when the type is infered to be Task<*, Error>.
To me, that sounds like a tall order for someone relatively new to the language. And, even at a high level, it may be difficult to understand why explicitly discarding a reference to the Task object itself is the way to silence a warning intended to warn about errors thrown not being handled in the execution (which happens after initialization).
I really feel like there's a nontrivial gap between what the compiler is trying to tell me (that errors in the task body are not handled) and the "solution" (to explicitly discard the result of the Task initializer). I don't see the _ = Task { ... } as a clear way to convey the intention of ignoring errors thrown inside the task either.
Maybe all the above can be handled with smart diagnostic messages, just pointing out that I think it can be hard to understand what's happening with the new behavior.
This makes sense to me. We have ways to ignore return values in the Task init if thatâs what we really want and this would make it more consistent with the language itself.
With this change Iâm 100% in favour of the proposal.
I feel like removing discardableResult entirely might be a more significant source breaking change. But⊠in reality, how often do you return a value from a task, but then not consume it? I canât remember ever doing so. So it does feel like the compiler should warn in this case as well.
The "underscore-to-ignore" pattern is not unique to Task. It applies to all initializers. It is also used in many other places in the language, like closure arguments and variables defined by a for loop. But, I don't think those are at issue.
I agree that there's potentially a fair amount to understand here. And I also agree that the fix for an intentionally-ignored error is tricky to understand. But I have a pretty hard time imagining that both this is where a beginner would first run into this problem and also that their intention would also be to actually ignore the error.
That said, I believe there are a few other examples of similar APIs where a fix-it exists to add the "_ =" and I believe that could be possible here too.
+1 from me, Ive hit this issue myself, and the default discardable result, while convenient in some places, made it easy to mistake where I should have been capturing and dealing with errors.
The issue is not so much the _ = pattern (as you say, it's widely used already) but rather understanding what issue the assignment is supposed to be fixing. In other places where the _ appears, the developer intent is obvious. For example, in a closure:
{ _, secondParameter in
// ...
}
It's obvious that the developer's intent is to ignore the first closure parameter. However, with the non-discardable Task initializers:
_ = Task {
try foo()
}
Would you say it's clear that the developer's intent here is to ignore errors thrown when the task is executed?
I didn't at first, although I've been looking at it for so long now that it kinda feels natural . Maybe it's just one of those things that requires getting used to. TBH I've seen Task { ... } used so many times as a replacement for dispatchQueue.async { ... } that I have to remind myself that not all tasks use Void as the success type.
Other than the fix-it, I think it would be nice to have a targeted warning that clearly states that the problem is that errors in the Task { ... } closure are not handled (and not just that the result of the initializer is unused).
I agree that general behavior can be confusing (including the fact that we just instantiate object and immediately ignore its value). Yet Task API has already been with us for a while, and changing this will be major, with the _ = Task {} is only one point (relatively insignificant) in the complexities you state in previous post. I believe that while this deserves discussion, it shouldnât be part of this change.
. Maybe it's just one of those things that requires getting used to. TBH I've seen