[Pitch] Improved error handling in unstructured Task initializers

To clarify, I think the proposal is great, I just thought it was pointing out that the _ = bit for fire-and-forget throwing tasks where you do want to ignore the errors may be hard to grasp for some.

Maybe removing the @discardableResult for all tasks other than <Void, Never> (as others have suggested in this thread) would make the behavior more consistent and thus easier to understand.

2 Likes

And I think it is great discussion!

It really is a good observation that just creating a Task struct has this very significant side-effect and is confusing. But I still don't believe that "_ =" is any more confusing, given that it is the way you ignore return values in the language.

It also got me thinking about using try?, which I think could also be a useful way to express this. The lack of optionality is, maybe, also weird. But again, this is an existing language feature.

Task {
  try? await doThing()
}

Ignoring errors can have utility, and it should absolutely be possible. Fundamentally, though, I remain very ok with forcing some head-scratching on developers here. Starting work that could fail without any consequence is strange and requires thought.

1 Like

Wouldn’t using try? change the inferred type of Success to be an Optional instead.

So now you’re not actually discarding the result?

1 Like

It's true, if a value is being returned, you are also making it optional. But since we are talking about ignoring the instance altogether, I'm not sure this matters does it?

Plus, this pattern could give you a much finer-grained way to control which errrors you do and do not care about.

Task {
  try step1()
  try? step2()
  try step3()
}

The side effect of creating the task, while significant, is well-understood by now. It's just that in other instances where you see _ used is more immediately clear what it does if you simply imagine the non-underscored version. But if you write the non-underscored version of this:

let doThingTask = Task {
    try await doThing()
}

The issue is still not fixed. Errors thrown in the task are still ignored. So one might wonder, if the issue is that I'm ignoring errors thrown in the task, why doesn't the compiler complain now? I've done nothing to fix the issue!

Granted, I know the answer, and why the API is the way it is, I even agree that this change is for the best. It's just that taking a step back and looking at Swift Concurrency as a whole I wonder if there could have been a better way to ensure propagation of errors thrown inside unstructured tasks.

On the bright side, a very good thing happens here: since there's not that many things one can do with a reference to a Task, if you write the code above and forget to call value or result on that reference later, the compiler will at least warn you that the doThingTask variable is never used (unless you do use the reference for task cancellation, the only other thing you can do with a reference to a Task, but I'm having trouble imagining a realistic example).

I think another popular alternative to _ = Task { ... } will be do/catch + logging any failures. The try? option gets very messy if the Task contains more than a couple try statements.

2 Likes

One thing that I have in mind could be distinguishing at the initializers naming like Task(throwing: {}) or even Task.throwing {}, and plain Task {} then will allow only non-throwing code, but the issue is that this would be a source breaking change currently, and still questionable in terms of resolving initial case of discarding errors — in overall I don’t see this option as significantly better then current one.

I don't think we'd be interested in introducing new names for throwing or not versions of APIs. If anything, we're going in the opposite direction with the language since we adopt typed throws (as this proposal does), and thus the "does not throw" becomes Failure = Never, this is going to be common in many APIs so we should not be introducing "throwing" in method or type names anymore -- ThrowingTaskGroup is something we'd like to remove as well, it should become TaskGroup<T, Failure = Never or SomeError> eventually.

8 Likes

That's a good analysis.

Yes, the let ignored = Task { throw ... } still leaves the error "ignored", however the un-used variable warning triggering here does another push in the right direction. As you rightfully observe, most uses of a task handle are really going to be about awaiting on it, there's not much else to do with it other than storing it off somewhere, in which case the waiting will be non-local and we can't easily diagnose if it ever will be awaited on anyway.

I think that this is an overall improvement, and since this is unstructured concurrency, we can only do so much before it becomes hard or impossible to diagnose if something was handled or not. At least we're nudging in the right direction with the proposed changes here hm.

4 Likes

+1 from me overall. I think both changes are going to be a good improvement, even though some of the fire-and-forget convenience has to be given up at the same time.

Can you elaborate on what this means?

Big +1

I assumed this was part of the separated Typed throws in Concurrency, much needed.

+1 Now that we have typed throws, it would be nice if they could be used wherever we want. +1 also for removing @discardableResult to spot issues more easily. I hope this gets into future version of Swift. (not directly related but I would also remove @_implicitSelfCapture that tends to introduce leaks in concurrent code :p)

1 Like

Any news if/when this is going to happen? Very much +1 from me!

I guess this would still (silently) ignore the error?

func foo() {
    let task = Task {
      throw MyError.somethingBadHappened
    }
    print(task) // or task.cancel() after a small delay
}
1 Like

This is now replaced by another take on the proposal over here: [Pitch] Unstructured task errors

Please comment in the new thread instead of this one.

1 Like