Missing actions after forgetting an @ObservableState

[zgotsch]

Hi, I was struggling for a few hours today on a bug in my app and now that I've fixed it, I would like to understand better why it was occurring.

TL;DR: a missing @ObservableState caused actions to not make it to a child reducer. Why?

Basically, I have a feature which wraps another feature in an enum. A very simplified version looks like:

@ObservableState
struct State: Equatable {
  var wrapped: Wrapped
   
  @dynamicMemberLookup
  @CasePathable
  enum Wrapped: Equatable {
   case inner(Refreshable.State)
  }
}

Most of the functionality of the inner feature is not important, but I did implement a pull-to-refresh behavior, as demonstrated in the case studies. Unfortunately, when I added the await store.send(.refreshed).finish(), I noticed that the refreshes were not occurring as expected. (Aside: it does work, however, if the task is not awaited (just calling store.send(.refreshed)).)

The reducer for .refreshed is simple: it sends an api request and handles the response by sending more actions. I first noticed that something was wrong when I saw that my refreshes were resolving immediately even in adverse network conditions. When I looked at the server logs, my application wasn’t even making the network requests. Digging in a bit further, I saw that the request was throwing, but my reducer was never receiving the error! (The error that was being thrown is NSURLErrorCancelled, which seems to be thrown when you make simultaneous requests to the same URL, but I haven’t been able to find much documentation about this.)

⭐ The fix was to add the missing @ObservableState to the enum in my outer feature’s state (Wrapped in the example above). I see why that is important, but I have a couple of additional questions:

1. Why were my send calls in my .run effect (when handling .refreshed) not resulting in the actions making it to my reducer? Is there a way we can detect that this is happening and send a runtime warning?

2. Why does awaiting the task cause the NSURLErrorCancelled to be thrown? Looking at the code, I don’t understand how the behavior would differ. A Task (inside an EffectTask) gets spawned for the .run effect either way, right?

3. Why is it allowed to chain through non-ObservableState key paths (maybe this is just a limitation of the approach used to extend Store, and we should be happy to have it at all)? Example:

let outer = store.scope(state: \.wrapped, action: \.wrapped)
let inner = outer.scope(state: \.inner, action: \.self)
// this doesn’t work, the optional is still on the store state type
RefreshableView(store: inner)

// this works, since the top-level State is ObservableState, but we have no guarantees about intermediate states
if let scoped = store.scope(state: \.wrapped.inner, action: \.wrapped) {
  RefreshableView(store: scoped)
}

I tried to create a minimal example of this error to more thoroughly understand the problem, but was unable to reproduce the problem in a wrapper around the Refreshable example. Even the missing @ObservableState, the inner feature seemed to work just fine, and the refresh-initiated requests did not error.

[mbrandonw]

Hi @zgotsch, I can only guess at many of these questions without a project to look at. I know you tried to create one already, but having that would turn these guesses into more concrete answers.

1. Why were my send calls in my .run effect (when handling .refreshed) not resulting in the actions making it to my reducer? Is there a way we can detect that this is happening and send a runtime warning?

My guess is that your effect was cancelled, and therefore it is not allowed to send any more actions into the system. We could warn on this, but I think it would actually be very noisy and not what people actually care about. I think there are a lot of effects out there that don't proactively end their work upon cancellation and may accidentally send an action after being cancelled.

2. Why does awaiting the task cause the NSURLErrorCancelled to be thrown? Looking at the code, I don’t understand how the behavior would differ. A Task (inside an EffectTask) gets spawned for the .run effect either way, right?

My guess is that the .refreshable is cancelling its async closure, which in turn cancels the effect. In fact, now that I'm responding this, I'm wondering if you are actually running into this bug in SwiftUI. It seems that .refreshable cancels its async context when the body re-computes. I wonder if simply wrapping store.send in await Task { … }.value would fix it too.

This would also explain why you saw it working when using store.send(.refreshed) without the await. Because in that case the async cancellation does not trickle down to the store's effect.

And now that I think about it, this would also explain why the problem was fixed with adding @ObservableState to the parent state. Doing so caused the view to minimally observe state changes, whereas without it the view would be re-computing far too much. And so that probably caused the cancellation of .refreshable.

3. Why is it allowed to chain through non-ObservableState key paths (maybe this is just a limitation of the approach used to extend Store, and we should be happy to have it at all)? Example:

I'm not sure I understand the question or the code sample. Can you explain a bit more?

[zgotsch]

Wow this is an amazing answer and after some experimentation, I was able to verify that it exactly describes the behavior I was seeing.

I was able to observe that the Task associated with the refreshable was cancelled by logging Task.isCancelled. URLSession throws the NSURLErrorCancelled error if its task is cancelled. Wrapping the await store.send(...).finish() in an await Task {...}.value did solve the issue, even with the body recomputing too much (with the missing @ObservableState).

I am pretty new to Swift, so I didn't understand the Task cancellation semantics enough to understand that the cancelled requests could be the result of the refreshable closure being cancelled, so you've saved me a lot of confusion.

I have still been unable to reproduce this in a simpler environment -- the refreshable task is not being cancelled even though the body is being recalculated, so that is still a bit of a mystery for me. 😢

My question around question (3) was unclear, let me try asking it again.

I notice that the API only provides

func scope<ChildState, ChildAction>(
    state: KeyPath<State, ChildState?>,
    action: CaseKeyPath<Action, ChildAction>
) -> Store<ChildState, ChildAction>?
// (https://pointfreeco.github.io/swift-composable-architecture/main/documentation/composablearchitecture/store)

when State: ObservableState. I assumed that because of this type bound the observable state property of the parent state is important, but you can chain through non-ObservableState items. So in the example:

// this works, since the top-level State is ObservableState, but we have no guarantees about intermediate states
if let scoped = store.scope(state: \.wrapped.inner, action: \.wrapped) {
  RefreshableView(store: scoped)
}

I was able to chain through the field wrapped (part of my State (: ObservableState), but also through the .inner case on enum Wrapped, which is not ObservableState. This surprised me because I thought it would be equivalent to scoping twice, like in:

let outer = store.scope(state: \.wrapped, action: \.wrapped)
let inner = outer.scope(state: \.inner, action: \.self)
RefreshableView(store: inner)

but because Wrapped doesn't have the ObservableState conformance, you can't use it to get the Store?.

So I guess my questions are:

• Is it important that these two ways to scope the state are not interchangeable?
• Will anything bad/surprising happen if you're reaching through non-ObservableState (here)? Or will there just be more rerendering?

[mbrandonw]

Wrapping the await store.send(...).finish() in an await Task {...}.value did solve the issue, even with the body recomputing too much (with the missing @ObservableState).

Since you said you are new to Swift I want to expand on this a bit. The await Task { … }.value trick is a quick way to exit cooperative cancellation in an async context. For example, if you do something like this:

func load() async throws {
  let (data, _) = try await URLSession.shared.data(from: …)
  // ...
}

…then if the async context that load() is invoked from is cancelled, that will trickle down to URLSession, and deep in that framework it does the work of detecting cancellation and immediately ending the network request.

However, spinning up new, unstructured Tasks does not cooperate in cancellation, so if you do this:

func load() async throws {
  let (data, _) = try await Task { URLSession.shared.data(from: …) }.value
  // ...
}

…then cancelling the async context that load() is called from does not trickle down to the async context inside Task. So, that is why we can use this trick to execute async work in .refreshable but without letting .refreshable cancel our work.

For your other question, I think it is fine the way it is because you don't have access to the state in that intermediate key path. And so there is no concern for messing up observation. But I will have to think about it a bit more to be sure. Really I think the best way to explore this problem is to simply try writing a test that proves that by scoping with a composed key path where the intermediate layer is not ObservableState causes problems. Ideally it would be a simple unit test and would probably use withPerceptionTracking to prove that things are (or are not) observed properly.