feat(ResultAsync): implement fromSoundPromise

[CertainlyAria]

Why would someone need this?

There are two ways to create ResultAsync from promises.

Method 1

Directly calling new ResultAsync(promise). This is not documented & it is not consistent with other APIs.

Method 2

In order to have type safe error handling for promises, the programmer loses type safety with the current implementation.

Without fromSoundPromise, creating a ResultAsync<T,E> is done like:

There isn't any type safety with this approach.

• Somebody can change line 8 from throw 'unauthorized' to throw 'not-authorized'.
• Somebody can decide to refactor string exceptions to proper Error instances, i.e. Error('unauthorized') & Error('timeout')

In both of these cases, programmer has to remember to modify one behavior in two different places, without any support from the type system.

Another side effect of not having type safety, is the additional error type 'unknown-error'. The original function fails with either 'timeout' or 'unauthorized' while the Error type on line 14 is 'timeout' | 'unauthorized' | 'unknown-error'

Proposed Solution

A function called fromSoundPromise that accepts Promise<Result<T, E>> and returns ResultAsync<T, E>.

This will simplify the above snippet to:

For convenience, a SoundPromise<T, E> type is exported which just an alias for Promise<Result<T, E>>.

This is more compatible with async/await syntax. In most cases, a programmer can get Result<T, E> by awaiting SoundPromise<T, E> without any need for ResultAsync.

In more complex scenarios where chaining is required, ResultAsync can come to the rescue:

---

An eslint rule can be added in order to prevent any throw statements which would make promise based error handling 100% typesafe. Example ESLint Rules:

• eslint-plugin-functional/no-exceptions
• eslint-plugin-fp/no-throw

---

Here is the source code for the images above.

Not Type Safe:

async function getUserId(): Promise<number> {
  const rand = Math.random()

  if (rand < 0.1) {
    throw 'timeout'
  }
  if (rand < 0.2) {
    throw 'unauthorized'
  }

  return 5
}

const typeSafePromise = ResultAsync.fromPromise(getUserId(), (e: unknown) => {
  if (e == 'timeout') {
    return 'timeout'
  }
  if (e == 'unauthorized') {
    return 'unauthorized'
  }
  return 'unknown-error'
})

Type Safe:

async function getUserId(): SoundPromise<number, 'timeout' | 'unauthorized'> {
  const rand = Math.random()

  if (rand < 0.1) {
    return err('timeout')
  }
  if (rand < 0.2) {
    return err('unauthorized')
  }

  return ok(5)
}

const typeSafePromise = ResultAsync.fromSoundPromise(getUserId())

[supermacro]

Hey, @CertainlyAria thank you for the PR! Some observations:

1

I intentionally have not documented usage of new ResultAsync(promise) since, as you already pointed out, this is not consistent with the rest of the API

Therefore, I intend to not document this approach as the intended way is to use ResultAsync.fromPromise or ResultAsync.fromSafePromise.

2

There was no mention of ResultAsync.fromSafePromise ... how come?

3

This is a response to the following comments:

the programmer loses type safety with the current implementation

There isn't any type safety with this approach.

Somebody can change line 8 from throw 'unauthorized' to throw 'not-authorized'. Somebody can decide to refactor string exceptions to proper Error instances, i.e. Error('unauthorized') & Error('timeout')

In both of these cases, programmer has to remember to modify one behavior in two different places, without any support from the type system.

Another side effect of not having type safety, is the additional error type 'unknown-error'. The original function fails with either 'timeout' or 'unauthorized' while the Error type on line 14 is 'timeout' | 'unauthorized' | 'unknown-error' There are some edge cases here that the "from sound promise" approach doesn't take into consideration.

I think there may be some misunderstanding here. Note that unknown is in fact typesafe. The definition of typesafety is one where the compiler is not "tricked" or forced into avoiding a pitfall that would lead to a runtime exception. In other words; if the code compiles, but you get a runtime exception, then you do not have typesafe code.

Hence, the example code you labelled as Type Safe: is indeed not typesafe, since if I were to change the return err(...) back into their original throw analogues, you would end up with a UnhandledPromiseRejectionWarning.

Now ... The reason fromPromise has an (e: unknown): T => {...} signature for the error handler is because of the indeterminate nature of the JavaScript Promise API. This is not a question of typesafety but one of entropy or unpredicatbility. This very "unpredictability" is encoded into the typesystem by way of unknown. By using unknown we can still guarantee typesafety.

There is no way to know at compile-time what the set of possible errors are that a Promise may throw. In a simple case where you don't have a deeply nested call stack full of layers of promises, then yes, one can, using human judgment (e.g. just staring at the code), say that this function returns 'timeout' | 'unauthorized'. However this is not a strict rule that applies to Promises in general.

Example:

type SoundPromise<T, E> = Promise<Result<T, E>>

const runtimeUnchaughtPromiseExceptionWillHappen = ResultAsync.fromSoundPromise(getUserId())

async function getUserId(): SoundPromise<number, 'timeout' | 'unauthorized'> {
  const rand = Math.random()

  if (rand < 0.1) {
    return err('timeout')
  }

  if (rand < 0.2) {
    return err('unauthorized')
  }

  return somethingThatCantBeProvenAtCompileTime()
}

async function somethingThatCantBeProvenAtCompileTime(): SoundPromise<number, 'timeout' | 'unauthorized'> {
  if (Date.now() % 2 === 0) {
    throw new Error({ msg: 'this falls out of the scope of the return type of these functions' })
  }

   return ok(5)
}

---

Thus because of point 3, fromSoundPromise doesn't actually provide typesafety guarantees, and does not prevent uncaught promise rejections from occurring at runtime - which fromPromise does.

---

Perhaps I am missing something here. Let me know!

[CertainlyAria]

Sorry if my communication was not clear enough. Let me try again.

My Goal

Lets say that I want to implement the following logic:

1. Get userId by calling getUserId() (async function)

getUserId() can fail with timeout or unauthorized

2. Get the list of friends for that id by calling getFriends() (async function)

getFriends() can fail with internal-server-error

3. Properly handle each error type

getUserId implementation

async function getUserId(): Promise<Result<number, 'timeout' | 'unauthorized'>> {
  const rand = Math.random()

  if (rand < 0.1) {
    return err('timeout')
  }
  if (rand < 0.2) {
    return err('unauthorized')
  }

  return ok(5)
}

getFriends implementation

async function getFriends(id: number): Promise<Result<string[], 'internal-server-error'>> {
  const rand = Math.random()

  if (rand < 0.1) {
    return err('internal-server-error')
  }

  return ok(['friend 1', 'friend 2'])
}

2

There was no mention of ResultAsync.fromSafePromise ... how come?

The problem with ResultAsync.fromSafePromise is that it doesn't play well with async/await syntax.

ResultAsync.fromSafePromise(getUserId())
  .map((id) => {
    // id needs another unwrap:

    id.value // Property 'value' does not exist on type 'Result<number, "timeout" | "unauthorized">'.

    if (id.isOk()) {
      return ResultAsync.fromSafePromise(getFriends(id.value))
    } else {
      // have to do error checking inside the .map
      id.error
    }
  })
  .match(
    (friendList) => {
      // Same problem as above
      // type of friendList: ResultAsync<Result<string[], "internal-server-error">, unknown>
      /* Needs another unwrap & manual error checking here */
    },
    (err) => {
      // type of err: unknown
    },
  )

Note that here you cannot replace the map with andThen because there is nothing that you can do inside the else block

3

Note that unknown is in fact typesafe. The definition of typesafety is one where the compiler is not "tricked" or forced into avoiding a pitfall that would lead to a runtime exception. In other words; if the code compiles, but you get a runtime exception, then you do not have typesafe code.

100% agree with you. I should have chose my words more carefully. You don't lose type safety by going from string to unknown. But you lose some info about types & compiler won't be able to help you as much.

The point of fromSoundPromise is kind of similar to fromSafePromise. Programmer should know what he/she is doing. Throws should not happen inside a SoundPromise. One method of enforcing this is using the mentioned eslint rules.

Perhaps this short video can demonstrate what I want to do:

fromSafePromise completely falls apart because I want to have errors & I need error handling logic. fromPromise requires synchronization in two places:

1. throw statements
2. (e: unknown): T => {...} function

And because it relies on throws from the promise, the error handler function (e: unknown): T => {...} doesn't have any idea what is thrown inside the promise. Thus compiler cannot help the developer with the e.

---

With fromSoundPromise, in most cases, users can await the async function and access the Result<T, E> and use it like synchronous code. In more complex scenarios like this example, they can selectively switch from Promise<Result<T, E>> to ResultAsync<T, E> to get the additional functionality.

[CertainlyAria]

Any pushbacks against this? I can update the branch to fix the conflicts but I guess that's not the issue. Meanwhile, I think from fromResultPromise is a better name.

[ghost91-]

I would appreciate something like fromResultPromise quite a bit. We often have async functions that return Promise<Result<...>>. It's not easy to make the return a ResultAsync directly instead, because TypeScript expects async functions to return a Promise, not just a PromiseLike (which is the interface that ResultAsync implements. So having an easy dedicated way to convert a Promise<Result<...>> to a ResultAsync<...> would be highly appreciated.

At the moment, we use the constructor to do this, when needed, but it feels a bit clunky, and as has been discussed above, it is apparently intentionally undocumented. But I think some dedicated way to make working with async functions that return Promise<Result<...>>s easier would be great.

Not sure if we need the type alias, though. Just a method

export class ResultAsync<T, E> implements PromiseLike<Result<T, E>> {
  /* ... */
  static fromResultPromise<T, E>(promise: Promise<Result<T, E>>): ResultAsync<T, E> {
    return new ResultAsync(promise);
  }
  /* ... */
}

should be good enough. I guess just continuing to use the constructor is also acceptable, but as has been mentioned already, it doesn't fit that well into the rest of the API, so a dedicated method would be great.

[supermacro]

What are thoughts on this API?

The issue with the API suggested by @ghost91- is that the promise could be rejected / fail. Hence the need for a errorFn to catch a rejected promise and turn it into an E value.

[supermacro]

pushed a wip branch pr-396-from-result-promise-api

[ghost91-]

What’s the point of the additional signature? If you want to make it work with PromiseLike, why not just implement it for PromiseLike (which is a super type of Promise, if I'm not mistaken).

[CertainlyAria]

What are thoughts on this API?

The issue with the API suggested by @ghost91- is that the promise could be rejected / fail. Hence the need for a errorFn to catch a rejected promise and turn it into an E value.

At first glance I'm not sure if I like this. This means that the promise has two different & inconsistent ways of communicating the error:

• standard JS way of rejecting
• fulfilling but returning an instance of Error<T,E>

Whatever code that is producing this inconsistent promise should be fixed.

Considering that any promise rejection can be easily refactored to fulfill with Error<T, E>, I think it would be better if the consumer provides a Promise<Result<T,E>> which always fulfills with Result<T,E> and never rejects. (similar to ResultAsync.fromSafePromise)

[ghost91-]

Fully agree with @CertainlyAria

[supermacro]

and never rejects

This cannot be guaranteed by the very nature of how promises work. Hence why omitting the error handler is dangerous. Omitting the error handler then creates a "leaky" API where some users will eventually pass in promises that can reject.

[CertainlyAria]

and never rejects

This cannot be guaranteed by the very nature of how promises work. Hence why omitting the error handler is dangerous. Omitting the error handler then creates a "leaky" API where some users will eventually pass in promises that can reject.

It could be guaranteed by the consumer with a catch statement.

If this creates a "leaky" API, then normal synchronous use of neverthrow is also leaky

function doSth() {
    const rnd = Math.random()

    if(rnd < 0.5) {
        return ok({ myData: 'lucky' });
    } else {
        return err({reason: 'bad luck'})
    }
}

const result = doSth();

Users can still do

function doSth() {
    const rnd = Math.random()

    if(rnd < 0.2) {
        throw new Error('surprise!!')
    } else if(rnd < 0.5) {
        return ok({ myData: 'lucky' });
    } else {
        return err({reason: 'bad luck'})
    }
}

const result = doSth();

Ultimately it's up to the consumer fix the underlying function such that it communicates a failure in one consistent way.

[supermacro]

https://github.com/supermacro/neverthrow/issues/496

[supermacro]

After refreshing my memory on this PR. I still do not agree with this design.

async function getUserId(): Promise<number> {
  const rand = Math.random()

  if (rand < 0.1) {
    throw 'timeout'
  }
  if (rand < 0.2) {
    throw 'unauthorized'
  }

  return 5
}

Again, the nature of exceptions in JS (and most other langs) is such that there is no guarantee ever that a function only throws a known set of errors.

While looking at getUserId, it may seem as though the function only throws unauthorized or timeout, but it may be the case that Math.random() suddenly starts throwing in future versions of Node / V8. Or that Math.random() is replaced with something else and that other thing may throw deep within its call stack.

This is the unfortunate downside with JS - as such, unknown has to be used as the error type.