Make Reducer generic over errors

[DivineDominion]

I wanted to discuss what you think about this, but figured it's easier with actual code in hand, so here's a PR -- not necessarily meant to merge as-is, but as a starting point.

So with SE-0413 Typed throws we get an alternative to the (cumbersome) @rethrows protocol annotation: set the associated error to Never to express that the function cannot throw.

My motivation to look at this was that even the simple Counter example had to handle a throwing store dispatcher -- even though there were no errors.

With modern Swift and typed throws, we can essentially tell the compiler that this is not "throwing any error, or maybe none at all", but be very precise.

At the cost of a 3rd generic type parameter.

Counter: non-throwing

I started using this after making the reducer generic in d661685 by implementing a test suite to see whether it compiles and does its job:

import CounterData
import CounterUI
import ImmutableData
import Foundation
import Testing

@Suite struct CounterUITests {
    @Test func storeDispatch() {
        let store = Store(
            initialState: CounterState(),
            reducer: CounterReducer.reduce
        )
        func value() -> Int {
            return store.select(CounterState.selectValue())
        }

        #expect(value(), 0)

        store.dispatch(action: .didTapDecrementButton)

        #expect(value(), -1)

        store.dispatch(action: .didTapIncrementButton)
        store.dispatch(action: .didTapIncrementButton)

        #expect(value(), 1)
    }
}

This led me to tweak CounterUI and friends instead that do a similar job at exercising the API.

In a4c7f1a you see that one preview is now commented-out because you cannot decide that sometimes you want errors, sometimes you don't.

In real-world scenarios, that should (could?) be a feature. But here we see the first limitation of the approach. The Store and Dispatcher need to declare that errors are expected.

This reflects my motivation and I would call this scenario a 'win'.

Animals

AnimalsReducer does in fact throw an error sometimes. The error needs to be public. You decided to put it inside the reducer, so it's AnimalsReducer.Error.

There's no typealias for the concrete dispatcher, so a lot of diff noise where:

- ImmutableData.Dispatcher<AnimalsState, AnimalsAction>
+ ImmutableData.Dispatcher<AnimalsState, AnimalsAction, AnimalsReducer.Error>

Same for the Store.

The store double needs to declare throws(AnimalsReducer.Error) even though it doesn't for compatibility as a stand-in. This may be a benefit (clarity) or downside 🤷

Quakes

Same story to Animals, but dependencies fetched faster :)

[vanvoorden]

@DivineDominion Ahh… interesting! I'll put a little thought into this… it might be a good idea to have this ready for a 1.0.0 release.

FWIW… the very early versions of ImmutableData were built from the 5.10 toolchain compiler. By the time I made the decision to require 6.0 and up the tutorial book was already in progress and the version that was released did not implement typed throws.

[vanvoorden]

@DivineDominion BTW do you know what the runtime requirements would be for a typed throws? I know this requires the 6.0 toolchain compiler… but once that code is compiled are there any runtime requirements on how far back that code deploys to?

[DivineDominion]

@DivineDominion BTW do you know what the runtime requirements would be for a typed throws? I know this requires the 6.0 toolchain compiler… but once that code is compiled are there any runtime requirements on how far back that code deploys to?

No, I have no clue, or how to find that out except ask on the forums :)

[vanvoorden]

At the cost of a 3rd generic type parameter.

An alternative to a third parameter is for the Error type (which might be Never) to be defined as an associatedtype on State or Action. But then that means that State and Action are now defined as protocols in the ImmutableData infra… which is a bummer because now product engineers defining their state have to import and depend on ImmutableData.

Conceptually… I could see the argument that an "error" does conceptually "belong" to a "state". I just don't know if I want to introduce a new protocol here. Hmm…

[vanvoorden]

An orthogonal observation here is that one of the important design philosophies of the whole ImmutableData approach is that View layers should not explicitly depend on implementation details and internal structure of the State layers. This leads to view components "selecting" slices of state with Selectors. When the internal structure of State needs to be refactored or updated… it should not directly lead to build errors in the view components. It could lead to build errors in the selector functions… but that should be expected as part of the pattern.

If we do make the decision to formally define the type of error being thrown… we then potentially begin to "couple" our view component implementations to details made in the State layer. If the engineer building the Reducer decides that the type of error thrown needs to be changed… this is now a potential breaking change in our view component trees. If the engineer building the Reducer transitions from throwing no errors now needs to throw… this is now a potential breaking change in our view component trees.

I could see product engineers defining a Reducer with 10 action values. None of those action values lead to an error being thrown. The product engineer builds 20 or 30 view components assuming that no error is being thrown from dispatch. Now we need to add action number 11 and this action might throw an error. We now need to define our Reducer as throwing and this leads to a build errors across all 20 or 30 view components that previously assumed that dispatch could never throw.

[vanvoorden]

Thinking through things from another POV… I can count two major side effects from shipping typed throws on Dispatchers:

1. Dispatchers that never throw can clean up ceremony and boilerplate in view components that need code to handle an error that could never happen.
2. Dispatchers that throw can now specify exactly what type of Error is thrown.

If you can think of number 3 and beyond please also bring that to the discussion.

So let's think through number 1: cleaning up view component code when Dispatchers never throw. Once product engineers build complex products with 10s and 100s and even 1000s of action values I think that most real world products would throw. If "just one" action value can throw an error… that means the reducer itself is now throwing.

For the Counter sample app… yes there is some boilerplate and ceremony from the try-catch when we know that dispatch would never throw. But I think that it is a small subset of real world product engineers that would be shipping apps that can never throw.

And then this also leads back to a previous point: if a product engineer builds their component trees around a reducer that never throws and then adds one action that might throw… this is now a breaking change across those component trees that now need to implement a try-catch. This looks like mental friction… and I could see this "locking in" product engineers to never throw from Reducers as an alternative to introducing a breaking change.

Thinking through number 2: what do we think the tradeoffs are of view components knowing exactly what error is thrown without having to unwrap an existential Any error type? I do agree there is a small performance improvement from defining this error at static compile time… but what about other effects? Does this improve the implementation code in view components in an impactful way?

This also leads back to a previous point: we can now potentially couple our view component implementation to logic built in our Reducer. This can make view component code brittle and more vulnerable to breaking changes if the engineer working on Reducer needs to refactor the underlying Error.

If we did implement typed throws there is nothing stopping a product engineer from choosing the type to be Any Error. Which then brings them back to the current behavior. So that is a workaround.

On a different topic I think something to keep in mind for both solutions is that we would like to think about ways to keep view component code loosely coupled to internal details from our State layer. Usually that means that properties of State are not public and are derived through Selector functions. I believe a potential "best practice" for error handling might look the same. Whatever error is thrown from a Reducer… which could be a static type or could be a dynamic box… I think a good move to defend against brittle view components might be for the actual logic to then transform that error value into something useful for the view component lives somewhere outside that view component. This might look like a "error selector" that lives in the state layer. If a product engineer needs to update the structure of the errors that are thrown… this can lead to a breaking change in the error selectors. But the error selectors are an abstraction layer going into the view components and the view component code itself would not directly break from that.

And then another topic here worth keeping in mind is that from the perspective of view components we are asking ourselves whether or not a dispatcher can throw… which is not necessarily the same as asking ourselves whether or not a reducer can throw. Our current ImmutableData.Store implementation does not directly throw any errors other than what might come from the Reducer… but we could brainstorm situations where we do want to throw errors directly from ImmutableData.Store independent of whether or not the product engineer build a reducer that throws. Giving the product engineer the ability to define what kind of error can be thrown from ImmutableData.Dispatcher can limit the ability for us to add new code inside ImmutableData.Store that makes its own decisions whether or not to throw.

Tying this back to the two side effects from the start:

Dispatchers that never throw can clean up ceremony and boilerplate in view components that need code to handle an error that could never happen.

I expect the majority of product engineers using ImmutableData build Reducers that throw from at least one action value. I also believe that if we one day decide to give ImmutableData.Store itself the ability to throw then this is a breaking change from all the view components that do not throw from reducers.

Dispatchers that throw can now specify exactly what type of Error is thrown.

I do agree there would be a runtime performance improvement from statically dispatching an error without an existential box to erase the type. But if we did statically dispatch an error I believe I would see a lot of benefits from keeping that error opaque. The actual data and properties of that error should not need to be exposed as public for the view component to access directly. An abstraction layer like an error selector might be the best choice then to keep the view components loosely coupled from the actual construction of this error type.

Thinking through both of those… I'm leaning in the direction of not shipping typed throws on dispatchers for 1.0.0. I'm not completely opposed and I'm also open to continue listening more for what some pros and cons might look like.

[vanvoorden]

cc @octovolt for philosophy and history of error handling from flux and redux.

[vanvoorden]

I did a little more exploring for some history behind error handling in Flux and Redux. It looks like Flux did allow for Dispatchers to throw errors:

https://legacy.reactjs.org/blog/2014/07/30/flux-actions-and-the-dispatcher.html#why-we-need-a-dispatcher

And Redux also allows for Dispatchers to throw errors:

https://redux.js.org/api/store#dispatchaction

Because Reducers can sometimes do work they are not supposed to be doing it looks like Redux Dispatchers are setup to accomdate Reducers that throw:

https://github.com/reduxjs/redux/blob/v5.0.1/src/createStore.ts#L298-L303

This implies that Reducers can sometimes perform work that might throw. What is less clear to me is if it was ever a legit pattern for product engineers building their JS reducers to throw from inside their reducers. I found arguments on both sides of that question…