Skip to content
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

Why is there no ResultAsync.fromThrowable? #488

Closed
ehaynes99 opened this issue Jul 24, 2023 · 8 comments
Closed

Why is there no ResultAsync.fromThrowable? #488

ehaynes99 opened this issue Jul 24, 2023 · 8 comments

Comments

@ehaynes99
Copy link
Contributor

ehaynes99 commented Jul 24, 2023

Result.fromThrowable takes a function as an argument, and returns a new function that returns a Result. There is no corollary for ResultAsync, which really diminishes the utility of the library. There have been a lot of issues in here mentioning how people want to have an async function that returns a ResultAsync. This is just fundamentally wrong. It defeats the entire purpose of using a library like this, because the whole point is that the errors are moved from thrown, untyped values into the return type of the function.

For example:

// doesn't compile because it's not returning a `Promise`, and thats a GOOD THING
const getOrders = async (username: string): ResultAsync<Order[], AxiosError> => {
  // This is bad, because it can throw
  const { data } = await axios.get(`/users/username/${username}`)
  const { id } = data
  return ResultAsync.fromPromise(axios.get(`orders/customer-id/${id}`).then(({ data }) => data), (cause) => cause as AxiosError)
}

To use such a function, you would have to handle the potential error in the result, but you would also still need to wrap it in a try/catch

try {
  await getOrders('someUser').match(
    (orders: Order[]) => console.log(orders),
    // typesafe error type
    (error: AxiosError) => res.send(500, { message: error.message }),
  )
} catch (error: unknown) {
  // no type safety
  res.send(500, { message: error instanceof Error ? error.message : 'unknown error' })
}

fromPromise does not provide sufficient type safety, because it's possible to have synchronous error thrown while creating said promise:

const updateUser = (user: Partial<User>): Promise<User> => {
  const { id, ...updates } = user
  if (!id || Object.keys(updates).length === 0) {
    // since this is not an `async` function, this throws in a synchronous context
    throw new TypeError(`invalid user value: ${user}`)
  }
  return axios.put(`/users/$id`, updates).then(({ data }) => data)
}
// `TypeError` will be thrown here, not captured in the result
const userResult = ResultAsync.fromPromise(updateUser({}), (err: unknown) => err as TypeError | AxiosError)

For ResultAsync to actually provide safety, it's rather critical IMO to be able to convert an arbitrary function that returns a Promise to a wrapper enclosed with ResultAsync that is able to handle both types of errors, not just the rejected Promise

const updateUser = ResultAsync.fromThrowable(
  (user: Partial<User>): Promise<User> => {
    const { id, ...updates } = user
    if (!id || Object.keys(updates).length === 0) {
      // wrapper could capture sync errors
      throw new TypeError(`invalid user value: ${user}`)
    }
    // and also async errors from a rejected `Promise`
    return axios.put(`/users/$id`, updates).then(({ data }) => data)
  },
  (err: unknown) => err as AxiosError | TypeError,
)
@ehaynes99
Copy link
Contributor Author

An implementation might look like this, which would "hoist" any synchronous errors into the async context.

export const fromThrowable = <Fn extends (...args: readonly any[]) => Promise<any>, E>(
  fn: Fn,
  errorFn: (err: unknown) => E,
): ((...args: Parameters<Fn>) => ResultAsync<ReturnType<Fn> extends Promise<infer R> ? R : never, E>) => {
  return (...args) => {
    try {
      return ResultAsync.fromPromise(fn(...args), errorFn)
    } catch (error) {
      return ResultAsync.fromPromise(Promise.reject(error), errorFn)
    }
  }
}

@paduc
Copy link
Contributor

paduc commented Sep 14, 2023

@ehaynes99 Thanks for this ! I think this proposal merits a PR ! Could you do it ?

@supermacro
Copy link
Owner

#496

@ehaynes99
Copy link
Contributor Author

Better late than never. I wasn't using this library for a while, but I added a PR for this.

@danawoodman
Copy link

It doesn't seem ResultAsync.fromThrowable actually exists in v6.1.0 (latest) package, am I missing something?

image

image

image

@supermacro
Copy link
Owner

Just published now under v6.2.0:

https://github.com/supermacro/neverthrow/releases/tag/v6.2.0

@borolgs
Copy link

borolgs commented Apr 22, 2024

Broken build on npm. There is no dist directory.

@supermacro
Copy link
Owner

Fixed in v6.2.1 .. sorry about that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants