Task initializer with throwing closure swallows error

This is likely well-known at this point, but I wanted to ask about the rationale for this? Specifically, if this was discussed anywhere else on the forums or documented elsewhere, because I couldn't find it when searching.

func someFunction() {
  Task {
     try await someThrowingFunction()
  }
}

This code swallows the error of someThrowingFunction. You can get the error if you call the value property on the Task, but I could see folks forgetting to do that.

1 Like

What would you rather have it do instead? Given the fact that the error production from someThrowingFunction could itself be asynchronous, what other behavior could it have?

Personally, I’d like the Task initializers to be @discardableResult only if Success == Void, Failure == Never. I’m not sure if this is technically possible to implement, though (without special-casing in the compiler).

2 Likes

That doesn't seem like something unique to Task. And I would disagree anyway, as the point of Task to allow you to create a async context you don't have to care about unless you want. Forcing users to always handle the result, even some of the time, just requires some alternative API to ignore the results anyway.

2 Likes

I don't disagree that Task's init itself shouldn't be made throwing. That said, in all other contexts in Swift I can think of, an error has to either be handled or thrown. It seems odd to me that Tasks are an exception. Maybe the compiler should warn if an error goes unhandled inside a Task?

2 Likes

Swift allows you to circumvent implementing a catch block at call site, but you have to create the function for it yourself.

func `do`<Success>(_ operation: () throws -> Success) {
  _ = try? operation()
}
`do` {
  try throwingFunction1()
  try throwingFunction2()
  try throwingFunction3()
}

Task ends up being an asynchronous version of that in many cases. It's understandable why, but not consistent.

Exactly.

The ability to encapsulate and defer the handling of an Error has been in the language since Swift 5.

let result = Result {
  try toDoSomething()
}

Task is simply the async version of this feature (and we can Results elsewhere in the concurrency features to do the same thing).

Sure, but you have to explicitly ignore the Result.

I agree that constructing a non-Void Task shouldn’t be ignorable. It wouldn’t completely prevent you from ignoring the result, but it would catch the the easy case, and that’s important.

7 Likes

Which makes sense in the Result cases since there's not really a use case where you'd want to ignore it anyway. Task has additional uses.

Wasn't half the point of Task to be roughly analogous to DispatchQueue.async, in that it's a way throw off async work we don't otherwise care about? It's at least been positioned (and used) as that solution, so adding such a requirement would seem to need an alternative for that capability. Which brings us back to the same shape API, so...

Swift already has idiomatic facilities for explicitly ignoring results and errors, all of which would work just fine here. You could do Task { _ = try? foo() } or _ = Task { try foo() }.

Sure, but that's not really the same feature at that point. Like I said, I thought this part of Task was intentional and fit a specific need. If that's not the case, wouldn't it be an obvious additional feature?

The primary purpose of the Task initializer is to spawn work that runs asynchronously, not to be an implicit way of ignoring results. When we made it discardable, we were thinking of fire-and-forget tasks which should naturally return Void and not throw. It would be consistent with Swift’s overall approach to discarded values to somehow make the result ignorable only under these conditions.

9 Likes

It's important to realize that while dealing with this as a @discardableResult problem would effectively be a mnemonic solution after you've learned and ingrained it, it is also a hack, not clear for newcomers or people who forgot about why the hack is there.

Discarding a result/value makes sense:

_ = await Task {
  try await throwingFunction1()
  try await throwingFunction2()
}.result

But…

…discarding a Task itself, as a syntactical hack to say, "I will forgo dealing with values or errors, that I otherwise would be able to access later using .value", does not. It's a confusing concept.

I would prefer if the compiler would offer something that points people to local error handling, rather than "Result is unused", for Task<Void, Error>.init. And for other Success types, "Result is unused" is closer to an accurate assessment of the problem, but not precise.

Yeah, if anything I'm wondering (and forgive me if this has already been expressed) if something like this would help people be aware that they're dropping thrown errors.

Task {
  try await funcThatThrows() // warn: call can throw, but errors thrown inside a Task expression will be discarded.
}

This would guide the user to either try? the statement, wrap in a do-catch block, or await the result on the Task directly. I'm not picky, I'm just concerned that folks will lose track of these errors if they aren't careful.

2 Likes

Except they're not. The error is right where they put it: in the Task. It's just that the Task itself also vends the result, which leads to the conflict in priorities we see.

Half (or more) of the point of Task (at least right now) is that it's a way to throw off async work from sync contexts. Even if we did require the user to do something with the value, what are they supposed to do with it? They can't bring it back into the sync context, so they have no choice but to discard it. Seems to me that makes discarding the result a rather good default.

I don't see how that approach would be compatible with one of its two major use cases: creating an async context from a sync one. If the original feature(s) intended for that purpose had been kept, it might be different, but Task has multiple roles here, and one essentially requires you discard the result, as the user can't receive it in the first place.

There's nothing formally in the design guidelines around the use of @discardableResult, but I seem to recall the original pitch discussed making it the default and adding @notDiscardable (or something similar), as requiring the handling of return values may be onerous. That was pretty quickly discarded as "unsafe", as return values may play a critical role in API usage. Generally I discard any value that isn't either the primary result of the function (like map) or is critical to the proper usage of the API (like the AnyCancellable returned by Combine's sink). Given all of the roles Task { } serves, I'm not sure it meets that criteria.

I think you’re confusing the idea of forcing someone to not implicitly ignore a non-void Task with the idea of forcing them to somehow immediately handle the task result. Not ignoring the task doesn’t mean you have to block on it right away; it might mean saving it to block on it later, or it might mean having the task explicitly ignore the result or take some specific action with it (so that, in either case, the task becomes void-producing). Just like with everything else about non-discardable results, we can’t force the user to do something meaningful with the task, but we can catch simple errors of omission and encourage them to think about where that information should go.

7 Likes

I, for one, was surprised by errors being swallowed, I'd expect the app to crash

I have to agree here. I ran into this issue and at first didn't really understand why something wasn't working, and it was because a Task was swallowing an error and the compiler did nothing to help me understand that.

I truly believe, if a Task can throw, then the compiler should make us either handle it or explicitly ignore it. I understand that there are different reasons for using a Task, but if a Task either results in a non-Void value or some Error, then it should be clear that you aren't handling it properly.

Swift has had fantastic error handling and compiler warnings for unused results for a long, long time. It's some of the many great things that makes Swift a great language. This feels like a regression in that sense, because it is not clear what is happening and very easy to make a mistake.

To be clear, this should be fine:

Task {
  let thing = await someThing()
  print(thing)
}

But this should have some compiler help to let the developer know that the error is going to be silently swallowed:

Task {
  let thing = try await someThing()
  print(thing)
}

The compiler can then require any of the following to fix it:

let task = Task {
  let thing = try await someThing()
  print(thing)
}

// or

Task {
  let thing = try? await someThing()
  print(thing)
}

// or

Task {
  do {
    let thing = try await someThing()
    print(thing)
  } catch {
    print("Uh oh something went wrong", error.localizedDescription)
  }
}

All three of these satisfy that the developer has made a conscious decision to either handle the error or discard it and not accidentally just let a Task swallow it silently.

3 Likes

This is exactly how I feel. This behavior is completely and utterly inconsistent with the rest of Swift's error handling model and its approach to ignoring results. I am not aware of any other instance where it is this easy to swallow an error or ignore a result. This will lead to insidious bugs.

So then would you agree that the Task initializers should've been defined such that the initializer is only discardable when Success == Void and Failure == Never?

1 Like