pointfreeco / episode-code-samples

💾 Point-Free episode code.
https://www.pointfree.co
MIT License
939 stars 289 forks source link

Composable Architecture - Returning errors in Effects for Unhappy Cases #58

Closed mathieutozer closed 4 years ago

mathieutozer commented 4 years ago

How do I return errors from failed Futures?

The nthPrime function returns an Effect<Int?>, the optional signaling the unhappy case, but what if we want to return the error received from the API call itself? Currently wolframAlpha replaces errors with nil. I think I want to return the error from my API callback, but currently as it is I have to switch on the API's failure case and then call .success(nil), which isn't ideal. What's the recommended way of dealing with this scenario?

refreshVideos: {
    Future { callback in
        Network.shared.apollo.fetch(query: AllVideosQuery()) { result in
            switch result {
            case .success(let query):
                if let videos = query.data?.videosList.items.compactMap({ videoFrom($0.fragments.videoDetails) }) {
                    callback(.success(videos))
                } else {
                    callback(.success([]))
                }
            case .failure(_):
                // properly error?
                callback(.success([]))
            }
        }
    }.eraseToEffect()

Sorry if I'm missing something obvious!

mbrandonw commented 4 years ago

Hey @mathieutozer, good question! And it's definitely a bit of an oversight right now.

The way to handle this is to construct an Effect<Result<[Video], Error>>, and then you would have an action that takes that payload:

enum AppAction {
  ...
  case refreshVideosResponse(Result<[Video], Error>)
}

However, the current eraseToEffect only works when the Failure of the Publisher is Never. We should have another overload that works on unconstrained Publisher and it puts the failure in a Result.

extension Publisher {
  public func eraseToEffect() -> Effect<Result<Output, Failure>> {
    self.map(Result.success)
      .catch { Just(Result.failure($0)) }
      .eraseToEffect()
  }
}

If you use that method in your example you should get a proper Effect<Result<_, _>> that will probably be useful to you.

Does that make sense?

mathieutozer commented 4 years ago

Thanks @mbrandonw! I was able to conceive of the first step, but didn't have the chops to figure out the second.

I am now having trouble holding onto a long running cancellable effect (I think that is my issue). I tried implementing

@available(iOS 13.0, *)
public extension Publisher {
  func cancellable<Id: Hashable>(id: Id) -> Effect<Result<Output, Failure>> {
    return Deferred { () -> PassthroughSubject<Output, Failure> in
      cancellables[id]?.cancel()
      let subject = PassthroughSubject<Output, Failure>()
      cancellables[id] = self.subscribe(subject)
      return subject
    }
    .eraseToEffect()
  }

  static func cancel<Id: Hashable>(id: Id) -> Effect<Result<Output, Failure>> {
    .fireAndForget {
      cancellables[id]?.cancel()
    }
  }
}

As per other advice found in the issues here, but the other implementation where Failure == Never get used instead

@available(iOS 13.0, *)
extension Publisher where Failure == Never {
  public func cancellable<Id: Hashable>(id: Id) -> Effect<Output> {
    return Deferred { () -> PassthroughSubject<Output, Failure> in
      cancellables[id]?.cancel()
      let subject = PassthroughSubject<Output, Failure>()
      cancellables[id] = self.subscribe(subject)
      return subject
    }
    .eraseToEffect()
  }

  public static func cancel<Id: Hashable>(id: Id) -> Effect<Output> {
    .fireAndForget {
      cancellables[id]?.cancel()
    }
  }
}
mbrandonw commented 4 years ago

I believe there shouldn't be a need to define overloads on for cancellation. You should be able to get away with the one definition, as seen here: https://gist.github.com/mbrandonw/cb4848c9896680af6d250d9ae5f15cdd

Did you need the overload for something in particular?

mathieutozer commented 4 years ago

You are correct - my issue was that I hadn't set up a publisher correctly so the subscription closed once the first effect was received. Still a new to these patterns!