Discussion on: Type-Safe Error Handling In TypeScript

View post

I think you have basically a good idea, but taking it to an extreme like this is not useful, IMO. I.e.:

Users of your API are not required to catch (the compiler
doesn't enforce it). This means you will eventually hit a
runtime error ... it's just a matter of time

True, but even with result ADTs, it's still just a matter of time, because runtime errors will always happen. You've changed your one "if not url, return the err result" line to not throw, but what about all of the code your function calls? I.e. you call randomNpmLibrary.whatever() or what not.

To literally never throw a runtime exception, you'd have to wrap your function implementation with a try catch:

try {
  ... regular code...
  if (boundaryCondition(...)) { return err(...);
  return result(...);
} catch (e) {
  return err(...); // something i called failed
}

Anytime you call code (either yours or 3rd party) that a) hasn't moved over to the error ADT code style, or b) isn't 100% perfect in its adherence to the ADT code style and still accidentally NPEs/runtime exceptions when it isn't supposed to.

So, I think chasing "no runtime exceptions" is just a false/unattainable premise.

The other pitfall is that, if you want to represent "all failures return error instead of throw", eventually your ADT's error type expands and expands until they become super generic Result<Data | AnyError>. And you've lost any notion of the type-safety that you were after.

The reason is exactly what happened to checked exceptions; you start with Result<Data, DbError>. But now there is also an error if the user passes the wrong data, so you do Result<Data, DbError | ConfigError>. Now all your callers have to match on the new error (which we assert is good), or, they need to pass the new | ConfigError up to their callers, i.e. bubble up the error. So you're updating a lot of return values to do "oh right, add | ConfigError for my new error condition".

Which is exactly what happened with checked exceptions, having to add "oh right, also throws XyzException" all over your codebase whenever you discovered/added code for a new error condition.

So, to avoid updating all those places, maybe you make a more generic error type, like Result<Data, DataLayerError>, but now you need to convert all non-DataLayerErrors (like NoSuchMethod/etc.) into a DataLayerError, which is exactly like what ServiceLayerExceptions in checked exceptions did. :-)

Basically, at some point its actually a feature that "a random error can happen, it will blow up the stack, and you can catch it at some point on top".

Granted, for certain things, like network calls which are both extremely important and also almost certain to fail at some point, it makes a lot of sense to represent those as ADT return values, and force your caller to handle both success + networkerror. And so you're exactly right that it's a useful pattern to use there.

But I just wanted to point out that that I think taking it to the extreme of "my functions will never throw" and "I will use ADTs to represent any/all error conditions" is a slippery slope to a leaky abstraction, where Result<Data, Error> return values (i.e. its now very generic b/c it has to represent all failures, so how type-safe is it?) are doing essentially the same thing as catch but in a manner that is inconsistent and non-idiomatic with the runtime/rest of the ecosystem.

Karel Kral • Dec 11 '19

Thanks for your explanation, Stephen. Your words exactly describe my opinion.

architectophile • Apr 10 '20 • Edited

This is exactly what I was thinking reading this article. Is it really possible not to throw any errors? As long as you depend on any third-party libraries, there'll always be throw errors. Unless you completely control all of your codebase and they all follow your 'neverthrow' rules, you can't avoid some unexpected throw errors after all. So it's not an ideal method to handle typed errors.

Gio • Apr 10 '20

This is not true. I'm using neverthrow in production right now at the company that I work at.

You have to make a distinction between code that you wrote that you know won't throw, and code that others have written that you are not sure will throw or not.

For example, at my company, we use knex for database access. Knex is a huge library and determining in which circumstances it will throw or not is not worth anyones time. Instead, database operations have try/catch arms to catch errors from knex.

By wrapping this kind of code inside of try/catch you guarantee a typesafe interface to the consumers of your functions. Here's an example:

import { ok, err } from 'neverthrow'

const getWidget = async (widgetId: number): Promise<Result<Widget, DatabaseError>> => {
  try {
    const widget = await db('widgets').select('*').where('id', widgetId);

    return ok(widget)
  } catch (e) {
    return err(getDbError(e))
  }
}

Now knex is being used in a typesafe way.

architectophile • Apr 11 '20 • Edited

Okay. I got your point. So when you need to use a third-party library that might be throwable you wrap it inside of a try/catch statement and turn it into a neverthrow style module.
But what if one of your colleagues added a new feature that uses a new third-party library and forgot to wrap it inside of a try/catch statement?

For example,

import { makeHttpRequest } from './http-api.ts'
import { throwableMethod } from 'third-party-lib'

const run = async () => {
  const result = await makeHttpRequest('https://jsonplaceholder.typicode.com/todos/1')

  result
    .map(responseData => {
      // do something with the success value
      throwableMethod('this is a new feature')
    })
    .mapErr(errorInstance => {
      // do something with the failure value
    })
}

run()

Wouldn't it lead to a more serious problem because inside your core codebase(like in your business rules) you don't have any try/catch statements than when you have some try/catch statements somewhere so the thrown errors will be caught at some point and you can handle it before it causes any serious problems?

Since at the beginning of your article you pointed out that "what if you or a colleague forget to wrap makeHttpRequest inside of a try / catch block.", I think that the same thing could happen even if you stick to use neverthrow style because when you use something new that you are not sure if is throwable or not, you must wrap it inside of a try/catch statement.

If there's something I got wrong, please let me know what I misunderstood.
Actually I admire your idea and work. I'm just curious if it's really resolving the core issues.

Gio • Apr 15 '20 • Edited

But what if one of your colleagues added a new feature that uses a new third-party library and forgot to wrap it inside of a try/catch statement?

Yeah this definitely can occur. However, code review should prevent this sort of situation from happening too often. And it's ok. People make mistakes, no one's perfect. The team can retractively wrap the unsafe code in a try catch as part of their post-mortem.

This module isn't about creating the perfect codebase. Nor am I saying that you should never throw exceptions (how ironic). Strict adherence to dogma is never good!

What I am trying to say is this:

using throw as a control flow mechanism is suboptimal
modelling the various states of your functions (including whether your function could fail or not) within the type system leads to self-documenting code that is more obvious to others and easier to maintain

Wouldn't it lead to a more serious problem because inside your core codebase(like in your business rules) you don't have any ...

Hmm this depends at what depth the error is coming from and whether something higher up on the call stack does have a try / catch. The team needs to collectively agree to this sort of approach (hello coding guidelines!). This is more of a cultural / non-technical problem around enforcing a certain thing in your codebase.

Also, having a "catch all" try catch statement isn't really going to help you much. If you don't know what you're catching then you can't do much with it. So the counterargument in favor of having one catch statement at the top of your program is kind of irrelevant. All you can do is log the error? And maybe return a 500 to your users?