Improve ai error handling

[Caleb-T-Owens]

☕️ Reasoning

I've introduced a new "Result" type which provides a better error handling flow. With this I've ensured that the handling of any failure is left up to the consumer of the aiService. This means we can choose when to or to not display errors

🧢 Changes

[mtsgrd]

I instinctively introducing results into javascript isn't a good thing. What are you solving for that we can't do with good ol' throws?

[Caleb-T-Owens]

I instinctively introducing results into javascript isn't a good thing. What are you solving for that we can't do with good ol' throws?

@mtsgrd

Javascript is a language without tracked errors, this means that by looking at any one function, I have no idea whether or not it throws an error.

For example, take some:

declare function chooseLunchLocation(favouriteLunchLocations: Location[]): Location;

that sounds like a function that should always work. Turns out, it actually throws a ChottoOverflowException!

In practice, that means that if I'm calling any function, I need to look through its entire call tree manually to determine if there are any uncaught exceptions.

If I instead defined it as:

declare function chooseLunchLocation(favouriteLunchLocations: Locations[]): Result<Location>;

It makes it very clear that whenever I consume it, I need to handle that failure in some way, either by following an alternate logic path, or by returning another failure in the same function.

In this case, by using Result, it makes it very clear which functions in the AiService can fail, and then leaves the responsibility of handling that failure up to where it is consumed.

[Caleb-T-Owens]

In my mind, exceptions should be reserved for when something truly is an exceptional circumstance and we don't expect any part of the call tree to be able to deal with it. Similar to panicking in rust. If we expect the consuming function to deal with it, then we should signal that

[Byron]

In the past, I once tried to manually add type-safety to Python (even before one could do type-annotations). It failed spectacularly as it wasn't ever 'perfect', while pretending it was.

Here, I also think that exceptions are akin to Rust panics, which aren't meant to communicate errors at all and can happen anytime.

Trying to turn TS into Rust that way is probably going to feel unsatisfactory, but I really wonder if there is some nice syntactic sugar to make error handling more obvious where it was deemed necessary. Or to turn exceptions in foreign functions into a Result of sorts.

And in case you are wondering (like myself :D), what my business here is: When dealing with secrets over in my PR I have to keep asking how the frontend can be taught to be more aware of secrets itself to prevent accidental leakage, so stronger typing would be of great use. And generally, better error handling is as well, so I really appreciate @Caleb-T-Owens idea here even though my old gut tells me that it's a dangerous thing to teach a codebase new tricks like that without full buy-in.

[mtsgrd]

Thanks @Byron, that resonates with me. @Caleb-T-Owens I tried searching for examples of others having tried this, and am mostly finding reasons it is likely not a good idea. What do you think?

[Caleb-T-Owens]

I'm very cognisant of some of the ergonomic issues that can come with adding FP features into typescript (as someone who has implemented at least two different versions of Monads before 😅 ).

Part of the reason I chose to implement the Result type myself is that it doesn't lock us into a larger FP ecosystem. Keeping the strengths of the TS type system in mind was also why I chose to implement the Result as a union of two variants rather than as a class; because it is very easy to escape and supported by the type system very well. (Have a field day reading about HKTs: microsoft/TypeScript#1213)

[Caleb-T-Owens]

I tried searching for examples of others having tried this, and am mostly finding reasons it is likely not a good idea. What do you think?

@mtsgrd
I would want to know how we can avoid throwing exceptions without a result type, and still provide semantic hints as to what is an error.

You will have a very hard time convincing me that the control flow of exceptions is easy to follow, predictable, or even expected.

Short of wrapping every function we call in a try catch clause, there is no "safe" way to handle exceptions.

[mtsgrd]

You will have a very hard time convincing me that the control flow of exceptions is easy to follow, predictable, or even expected.

I think that's a few steps ahead of where I'm at. I would first like to understand how other comparable apps solve this problem? I haven't yet come across this as an established pattern in typescript.

[Caleb-T-Owens]

@mtsgrd

Perhaps my perspective is colored. I came into TypeScript after spending 3 months learning Haskell where there are no exceptions, and the idea of throwing and catching is almost implicitly considered a bad idea. I have bought into of using some monad to encapsulate failure rather than just throwing because it does seem preferable.

I don't think the problem is well solved by any one library, as they often want you to use their entire pure functional style "standard" library (https://github.com/lodash/lodash/wiki/FP-Guide, https://effect.website/, https://gcanti.github.io/fp-ts/, https://rxjs.dev/api). And, I for sure don't think that we want to go down that path.

Often, people who don't appreciate exceptions end up using languages which just don't have them (https://go.dev/doc/faq#exceptions, https://go.dev/blog/errors-are-values, https://doc.rust-lang.org/book/ch09-02-recoverable-errors-with-result.html, https://wiki.haskell.org/Handling_errors_in_Haskell, https://www.purescript.org/, https://elm-lang.org).

The mid point which anecdotally I've seen from other js libraries, is that they tend to roll their own data types which encapsulate error states with some valid or ok flag (https://tauri.app/v1/api/js/http#responset, https://www.twilio.com/docs/verify/api/verification#start-a-verification-with-sms, https://developers.intercom.com/docs/references/rest-api/api.intercom.io/contacts/createcontact), or have some sort of callback that is called when an error occurs (https://nodejs.org/docs/latest/api/fs.html#fscpsrc-dest-options-callback).

Maybe I am thinking down the wrong lines, but for sure it would be good to discuss when and how we throw exceptions because wrapping everything in a try catch is certainly not the way to go 😆.

[ndom91]

I spent all of my time with JS / TS using good old try/catch, so instinctively I'm partial to that. But I did want to share something I came across recently that seems relevant, this Effect library. It's basically 'ts-fp' v3 (they seem to have merged projects) and API-wise looks very similar to RxJS. Long story short, they describe themselves as a "standard library for TS" and also handle errors as values.

• "Why not to throw errors": https://effect.website/docs/guides/essentials/creating-effects#why-not-throw-errors
• Error management: https://effect.website/docs/guides/error-management/expected-errors