-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements to typed errors docs #173
Conversation
:::info Further discussion | ||
|
||
This section was created as a response to | ||
[this issue in our repository](https://github.com/arrow-kt/arrow-website/issues/161). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, great!
I wanted to add some more details to this doc, we can do it here or in a subsequent PR.
- You can update
Lce
to work overLceRaise<Any?>
and expose it overRaise<E>
to make it interopt with all other DSLs. See Quiver, https://github.com/cashapp/quiver/pull/26/files#diff-2a7ca3e3751a2611788369c9a4c63f27ee97ed2403d24e58d1476e01f57937b0R48 DialogResult
can just have abind()
function defined overRaise<E>
when we have context receivers- Building DSLs with context receivers will be easier and more flexible. Add some examples, see also Quiver PR discussion. Update to 1.2.0-RC release block/quiver#26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for cleaning this up! 🙏 I think it's good to split, because we probably want to expand the typed error document.
Some feedback I also got at KotlinConf, but I still need to make time for it is that in the typed errors we never show larger examples. I.e. what if I call a function marked with Raise<E>
from another function, what are you options? (recover
or propagate error).
Co-authored-by: Simon Vergauwen <[email protected]>
I've just pushed a big rework of the information there, making more clear how one is supposed to use |
is Lce.Failure -> raise(this) | ||
Lce.Loading -> raise(Lce.Loading) | ||
} | ||
- **Composability:** Typed errors can be easily combined and propagated through a series of function calls, making writing modular, composable code easier. With exceptions, ensuring errors are correctly propagated through a complex codebase can be difficult. Patterns like accumulation, which are at your fingertips using typed errors, become quite convoluted using exceptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another suggestion to the readme @serras. I've got some feedback from colleagues that when they read this documentation, they were looking to find a code example that illustrate computation chaining for a production use case. I remember something like that used to exist in the old docs about comprehension.
Having said that, may I suggest adding a snippet along these lines - perhaps in the "From logical failures
" section? This is the same code I put in kotlinlang https://kotlinlang.slack.com/archives/C0B9K7EP2/p1680694416774339?thread_ts=1680388003.721149&cid=C0B9K7EP2. Feel free to modify this as appropriate.. Notice that I've also used sealed class as the failure case. I noticed most production usecases that we've encountered uses sealed classes as well to model the failures. I'm not going to be surprised if many users out there also do the same. I hope that can give something more concrete for users.
context(Raise<UpdateUserFailure>) // this is a different error boundary
suspend fun updateUser(...): User = ...
context(Raise<ProvisioningFailure>)
suspend fun checkManagedStatus(...): Unit = ensure(user.isManaged) {
ProvisioningFailure.NotManaged
}
context(Raise<ProvisioningFailure>)
suspend fun checkProvisioningPolicy(...): Unit = ...
context(Raise<ProvisioningFailure>)
suspend fun provision(): User {
checkManagedStatus(...)
checkProvisioningPolicy(...)
val updatedUser: User = recover({ updateUser(...) }) {
when (it) {
is UpdateUserFailure.UserNotFound -> raise(ProvisioningFailure.InvalidUser)
is UpdateUserFailure.UpdateRejected -> raise(ProvisioningFailure.InvalidUpdate)
}
}
updatedUser
}
We can also encourage user to prepare for context receiver accordingly by providing a version of the composition in either { }
, i.e. explicitly pointing the docs to using recover ({ eitherValue.bind() }) { ... }
instead of eitherValue.mapLeft { }.bind()
. The sooner users use recover (...) { }
the easier they can transition to context receivers style.
suspend fun provision(): Either<ProvisioningFailure, User> = either {
checkManagedStatus(...).bind()
checkProvisioningPolicy(...).bind()
val updatedUser: User = recover({ updateUser(...).bind() }) {
when (it) {
is UpdateUserFailure.UserNotFound -> raise(ProvisioningFailure.InvalidUser)
is UpdateUserFailure.UpdateRejected -> raise(ProvisioningFailure.InvalidUpdate)
}
}
updatedUser
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the very concrete example, @myuwono. We got similar feedback from other people, which only reinforces the fact that our docs are lacking in that respect. I made some changes in this document to clarify the situation, do you think the read better now?
I'm hesitant, though, to make the introduction even longer. Maybe we should have a full worked-out example where we describe these ideas, like how to design your error hierarchy, the different boundaries, and so on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a brilliant idea.
This PR introduces a few improvements to the typed errors section, based on discussions I had with attendees to KotlinConf'23.