Emulation of Rust's ? operator

[tsuburin]

This is what I proposed in #444 , with some superficial modification to match the current APIs.

Closes #444

[k13w]

nice

[0xksure]

So basically I can do something like

const caller = ():Result<string,MyError> => {
  // foobar: Result<string,MyError>
  const res = yield* foobar().safeUnwrap()
  return res + "hello"
)
}

[0xksure]

Any contributors reviewing this?

[supermacro]

Sorry, life recently has made it tough to be as attentive as I used to be. I'll see if I can have a look this coming weekend.

[mklifo]

Bump

[tsuburin]

Any updates?

[inaiat]

Hello!
This PR is awesome! I like a lot!
Any news?

[supermacro]

#496

[supermacro]

Would anyone here be willing to contribute to the docs / readme? A few examples would be much appreciated for folks who aren't so familiar with generators as well (myself included).

[supermacro]

Extremely basic docs have been added in this commit: 1fd6a05

[supermacro]

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

[tsuburin]

#448 (comment)
I'll make a PR.

[ghost91-]

@tsuburin I know I'm a bit late to the party, but thanks a lot for this change, it's a great improvement!.

I just have one small question, though: is there a specific reason why safeTry takes a () => Generator<Err<never, E>, Result<T, E>> instead of a () => Generator<Err<never, E>, T>? I'm asking, because the latter would allow to create highly composable functions that all return Generator<Err<never, E>, T> and that can directly be used with safeTry.

In addition, it might also make sense to allow passing arguments to the function passed to safeTry.

Example that shows this in combination:

/* --- signatures for safeTry --- */

declare function safeTry<T, E, Args extends unknown[]>(
  body: (...args: Args[]) => Generator<Err<never, E>, T>,
  ...args: Args
): Result<T, E>;
declare function safeTry<T, E, Args extends unknown[]>(
  body: (...args: Args) => AsyncGenerator<Err<never, E>, T>,
  ...args: Args
): Promise<Result<T, E>>;

/* --- functions returning generators --- */

function myParseInt(input: string): Generator<Err<never, Error>, number> {
  const n = Number.parseInt(input);
  const result = Number.isNaN(n) ? err(new Error("Parsing failed!")) : ok(n)
  return result.safeUnwrap();
}

function random(): Generator<Err<never, Error>, number> {
  const n = Math.random();
  const result = n >= 0.5 ? ok(n) : err(new Error("Random number was too small!"));
  return result.safeUnwrap();
}

function* h(input: string): Generator<Err<never, Error>, number> {
  const n1 = yield* myParseInt(n);
  const n2 = yield* random();
  return n1 + n2;
}

/* --- usage of these functions with safeTry --- */

const result1 = safeTry(h, "42"); // ok(42 + <random number>), if the random number was large enough, otherwise err(...)
const result2 = safeTry(h, "abcdefg"); // err(...), because parsing failed

// but we can also pass myParseInt and random to safeTry directly:

const result3 = safeTry(myParseInt, "0"); // ok(0)
const result4 = safeTry(random); // ok(<random number>), if the random number was large enough, otherwise err(...)

Support for passing in arguments could easily be added without a breaking change. But I believe, if we wanted to change the type of generator function that needs to be passed in according to what I described above, that would require a breaking change :/

Of course, there is ways to do this, too, with the current api, e.g., you could have the composable functions all return Generator<Err<never, E>, Ok<T, never>>, and then always access .value on the result of yield* (and wrap the finally returned value in an ok). Similarly, you could write the composable functions as explained above, and define a lift function:

function lift<T, E, Args extends unknown[]>(fn: (...args: Args) => Generator<Err<never, E>, T>): (...args: Args) => Generator<Err<never, E>, Ok<T, never>> {
  return function* (...args: Args) {
    return some(yield* fn(...args));
  }
}

and use that when passing one of the composable functions to safeParse. (Not sure if you can write a lift function that works both for Generators and AsyncGenerators, or if separate functions are needed)

But both of these solutions are more cumbersome than what I described above, and I don't really see a downside (aside from the fact, that it would now be a breaking change).

EDIT: Possibly, it could be even more more consistent/simple, if Generator<E, T> is used. Of course, then safeUnwrap would also need to return Generator<E, T>. Do you see any conceptual problem with that?

[tsuburin]

@ghost91- Thank you for the comment.

is there a specific reason why safeTry takes a () => Generator<Err<never, E>, Result<T, E>> instead of a () => Generator<Err<never, E>, T>?

Although I haven't tested, technically the latter will also work, I guess. If so, this is a matter of taste. Personally I prefer the current interface, because I think you may want to return Errs from safeTry's body by usual return statements.
For example, lets say you want to add two integers and verify the sum is less than 20. With current interface you can write as follows.

safeTry(function*() {
  const sum = (yield* parseInt("12").safeUnwrap()) + (yield* parseInt("10").safeUnwrap());
  if (sum >= 20) {
    return err("sum is too big")
  }
  return ok(sum)
})

Howereve, with your proposal, because the only way to return Err is by yield*ing, you need to first make Err instance and then call safeUnwrap immediately.

safeTry(function*() {
  const sum = (yield* parseInt("12").safeUnwrap()) + (yield* parseInt("10").safeUnwrap());
  if (sum >= 20) {
    return yield* err("sum is too big").safeUnwrap()
  }
  return sum
})

The latter looks a bit clumsy for me.

I'm asking, because the latter would allow to create highly composable functions that all return Generator<Err<never, E>, T> and that can directly be used with safeTry.

With current interface, you can compose functions like the following.

function h(input: string): Result<number, Error> {
  return safeTry(function*() {
    const n1 = yield* parseInt(input)
    const n2 = yield* random()
    return ok(n1 + n2)
  })
}

Or, if it's still a bit cumbersome, you can write as

const h = (input: string) => safeTry<number, Error>(function* (){
  // The same body as above here...
})

In my opinion, we should keep the generators used in this feature from hanging around in the library's user's code. This is because it can let the users use the generators in wrong ways. The generator made by .safeUnwrap() is intended to be used just once, otherwise throws an Error. So, I think the idea of composing generators directly is not for this library. (Technically it looks interesting, though...)

In addition, it might also make sense to allow passing arguments to the function passed to safeTry.

If your motivation is to feed arguments to the body of safeTry, the h in the above examples already achieved. If you have other reasons, please tell me.

[ghost91-]

Hey @tsuburin thanks a lot for your response!

The main motivation behind the idea of composing functions that return generators directly is to reduce boilerplate in as many places as possible. With your example for h, you need a safeTry(function* () { ... }) in every function that wants to compose other functions, and on top of that, you could not put the parseInt and random functions into safeTry directly, since they return the wrong kind of generator. If you changed the type of generator as I suggested, you could simply compose them (avoiding an indentation in the code) and only need safeTry at one place at the top, where you really want to convert it back to a Result.

Howereve, with your proposal, because the only way to return Err is by yield*ing, you need to first make Err instance and then call safeUnwrap immediately.

safeTry(function*() {
 const sum = (yield* parseInt("12").safeUnwrap()) + (yield* parseInt("10").safeUnwrap());
 if (sum >= 20) {
    return yield* err("sum is too big").safeUnwrap()
  }
  return sum
})

The latter looks a bit clumsy for me.

It's actually simpler, just yield the Err directly (or if we also removed the Err from the type of Generator, just the error value):

safeTry(function*() {
  const sum = (yield* parseInt("12").safeUnwrap()) + (yield* parseInt("10").safeUnwrap());
  if (sum >= 20) {
    yield err("sum is too big"); // or even just yield "sum is too big"
  }
  return sum;
});

I see your point that it might sometimes be convenient to be able to return a Result directly. But on the other hand, that is also a bit inconsistent: There are 2 ways to "return" errors, either yield them, or return them. With my suggested approach, there is a clear separation: You yield errors, and you return the ok values.

The reason I started to investigate this at all is that I was initially very confused why safeUnwrap returns a different kind of generator than what safeTry expects. I would have expected

safeTry(() => result.safeUnwrap());

to work. But it does not, you need to to

safeTry(function* () {
  return ok(yield* result.safeUnwrap());
});

which (to me) feels confusing.

One other reason for modelling it the way I suggested: This whole functionality is basically Hakell's do notation for the Either monad (Result is just a specific type of Either), in the same way that async/await is more or less just the do notation for the Promise monad. And in the do notation, you also return plain values, and not wrapped values.

async/await in JavaScript is actually bit special in this regard, because it allows you to do both: You can just return a plain value from an async function, and it will be wrapped in a promise automatically. But you can also return a promise from an async function, without getting a promise wrapped in another promise—the promises are automatically collapsed into a single one.

Regardless, to me it seems natural that you can return plain values and don't have to wrap them in ok(...). If it's possible to support returning either T or Result<E, T>, that would also be fine (and would mimic the behavior of async/await in JavaScript), but I'm not sure if that can easily be done in a type safe way, without language/compiler support. Because, how would you handle things if T itself is a Result<U, F>? Maybe it's possible, but it increases the complexity dramatically 😅.

I see that you have modeled this more after how Rust's ? works. I didn't have much knowledge about it before and just assumed, it would behave the same as the do notation. But apparently, it does not, it also requires you to still explicitly wrap returned values in Ok. This kind of makes sense, since in Rust, you just have a regular function (returning an Error), so you need to return something that matches the return type of the function. But since we need a special kind of function/notation anyways, we do not need that "limitation".

I also get the point about not using the generators outside of safeTry. If you wanted to prevent that all together, you could even not make safeUnwrap a method on Result, but instead, pass it as a parameter to the generator function within safeTry:

safeTry(function* (unwrap) {
  const a = yield* unwrap(someResult);
  return ok(a); // I'd still prefer return a, though :)
});

That way, this unwrapping functionality is only available inside callbacks for safeTry (unless you "save" it somewhere globally, from inside such a function... But it should be extremely obvious that that's not intended...). This approach has the downside of needing even more boiler plate for safeTry, though (need to specify the unwrap parameter).

When going this route, you don't even really need generator functions anymore. You can also propagate the error by wrapping it inside a specific error class, throwing that in unwrap, and catching it in safeTry. See e.g. Do-notation for Either in Typescript (which interestingly also makes you return Right(...) instead of just ...). With that approach, it would look like

safeTry((unwrap) => {
  const a = unwrap(someResult);
  return ok(a); // I'd still prefer return a, though :)
});

This solution might be an interesting alternative because of its simplicity (generators can be scary for some people).

Anyways, sorry for the long rant. At least, I now have a better understanding of where you are coming from (Rust's ?). I still think that my suggested alternative would be slightly preferable (I still simply don't really see any downside to it), but maybe that's also just personal preference. I totally get that the small (perceived) benefits might not be worth a breaking change, now that the functionality has already been released (it's unfortunate that I'm just a bit too late...).

The nice thing is: Even if people here don't agree, I can just implement my own safeTry function, since it's just a free standing function that doesn't need any access to internal stuff :)

[tsuburin]

@ghost91- Thank you for your reply, too. I'm not so familiar with Haskell, so your point is really interesting to me. I have some experience of Rust but not of Haskell so much.

I completely agree on that it's preferable to reduce boilerplate as much as possible. However, another aspect I had in designing this feature was ease of understanding. In that aspect, I didn't really want to use generator at all. (This depends on individual, and if you argue generators are easy to understand, it is completely fair, but for me, it's not.)

Actually,

safeTry((unwrap) => {
  const a = unwrap(someResult);
  return ok(a); // I'd still prefer return a, though :)
});

this was exactly what I considered first, and gave up because I found it is very difficult to implement this safeTry correctly. There are two problems I couldn't solve. One is that there is no way to pass type-checking of catch-ed errors. In typescript, caught errors are inferred to be unknown, and even if I wrap the thrown error in a dedicated class and check by instanceof, the compiler can't check its contents' type. Of course, as long as everything are used correctly, we can assert that the contents are what we expect, but it is not acceptable for such kind of libraries, I thought. The other problem is that if someone use try-catch statements in this safeTry the functionality can be completely broken because it is possible that what is thrown by unwraps is caught and not thrown again.
So, finally I decided to use generators, and settled down what it is now. To me, generators are used here because there is no other way (at least as far as I could come up with). And so, honestly, I haven't considered composing generators at all.

It's actually simpler, just yield the Err directly (or if we also removed the Err from the type of Generator, just the error value):

safeTry(function*() {
  const sum = (yield* parseInt("12").safeUnwrap()) + (yield* parseInt("10").safeUnwrap());
  if (sum >= 20) {
    yield err("sum is too big"); // or even just yield "sum is too big"
  }
  return sum;
});

Oh, yes, this should work. But here you use yield* and yield, and it may be confusing for someone as return and yield* are for you. (Again, this is just a matter of taste, and I never mean you are wrong.)

safeTry(function* (unwrap) {
  const a = yield* unwrap(someResult);
  return ok(a); // I'd still prefer return a, though :)
});

That way, this unwrapping functionality is only available inside callbacks for safeTry (unless you "save" it somewhere globally, from inside such a function... But it should be extremely obvious that that's not intended...). This approach has the downside of needing even more boiler plate for safeTry, though (need to specify the unwrap parameter).

This idea seems worth considering. (Including the downside you mentioned, because it might be as obvious as this is that safeUnwrap should not be used outside safeTry, from its documentation.) For now, I guess we should wait for comments from others.

If it's possible to support returning either T or Result<E, T>, that would also be fine (and would mimic the behavior of async/await in JavaScript), but I'm not sure if that can easily be done in a type safe way, without language/compiler support. Because, how would you handle things if T itself is a Result<U, F>? Maybe it's possible, but it increases the complexity dramatically 😅.

As you pointed out, it is difficult (or may be impossible) to tell whether a Result instance is intended to be T or Result<U, F>, if we want to do it with the same function. However, also as you mentioned, you can implement your own function, and maybe you can make an issue or PR to hear comments from others. (I guess it is better to move to an issue if we continue this conversation, because this is a closed PR.)

Anyways, again, thank you for your comments with a lot of points I haven't noticed. 😄

[ghost91-]

One is that there is no way to pass type-checking of catch-ed errors. In typescript, caught errors are inferred to be unknown, and even if I wrap the thrown error in a dedicated class and check by instanceof, the compiler can't check its contents' type. Of course, as long as everything are used correctly, we can assert that the contents are what we expect, but it is not acceptable for such kind of libraries, I thought.

Since the dedicated class can only be thrown by unwrap, I believe it's safe to assume that the contents are correct, unwrap itself is typed so that only the correct kind of result can be passed to it. The only way I can see this breaking is with your next point, which is valid:

The other problem is that if someone use try-catch statements in this safeTry the functionality can be completely broken because it is possible that what is thrown by unwraps is caught and not thrown again.

Here is an example how this combination could break:

export function safeTry<T, E>(body: (unwrap: <U>(result: Result<U, E>) => U) => T): Result<T, E> {
  try {
    return ok(body(unwrap))
  } catch (e) {
    if (e instanceof UnwrapError) {
      return err(e.error) // typed any, but we _know_ that it should be E, because the only way UnwrapError could have been thrown is from unwrap, unless users _somehow_ get a hold of UnwrapError
    }
    throw e
  }
}

function unwrap<T, E>(result: Result<T, E>): T {
  if (result.isOk()) {
    return result.value
  }
  throw new UnwrapError(result.error)
}

class UnwrapError<E> extends Error {
  constructor(readonly error: E) {
    super()
  }
}

const result = safeTry((unwrap: <U>(result: Result<U, number>) => U) => {
  try {
    const result: Result<number, number> = err(42)
    const a = unwrap(result)
    return a
  } catch (e: any) {
    const constructor = e.constructor
    throw new constructor('boom!')
  }
})
// result typed as Result<number, number>, but it actually is Err<never, string> :(

I would argue that getting the constructor from a thrown error and then creating a new instance with different parameters is not something you should ever do, but it's possible. I think it would be acceptable to ignore this problem, because it's so obscure (and you need to use any, or type assertions for the caught error).

The fact that you can have a try catch inside the callback, which does not rethrow the custom error, is a much bigger problem, because try catch is still common syntax, and it completely breaks the functionality. I think this is a killer argument to not do it this way. Thanks for bringing it up, I had not thought of it (since we don't throw any errors, we always use neverthrow :D).

I mostly wanted to understand the reasoning behind the design choices. I think, I have a good understanding of that now. I'll think about whether it makes sense to continue and open a new issue to continue the discussion, if needed. Thanks for your input, I really appreciate it!

[tsuburin]

@ghost91-
Sorry if this is unnecessary, but let me make just one more comment for both your information and later references.

Since the dedicated class can only be thrown by unwrap, I believe it's safe to assume that the contents are correct, unwrap itself is typed so that only the correct kind of result can be passed to it.

This assumption is not safe, at least with your example above, because a kind of mistakes like the following is quite possible.

safeTry((unwrap: <U>(result: Result<U, number>) => U) => {
    const result = safeTry((umwrap: <T>(result: Result<T, string>) => T) => { // mistyped as umwrap
        const r: Result<number, number> = err(3);
        const x = unwrap(r); // unwrap from the outer safeTry is called
        return x; 
    })
   // result is typed as Result<number, string> but is actually Err<number, number>
})

[ghost91-]

Fair point. Unless I am missing something, that's solvable by creating the class inside safeParse:

export function safeTry<T, E>(body: (unwrap: <U>(result: Result<U, E>) => U) => T): Result<T, E> {
  class UnwrapError<E> extends Error {
    constructor(readonly error: E) {
      super()
    }
  }

  function unwrap<T, E>(result: Result<T, E>): T {
    if (result.isOk()) {
      return result.value
    }
    throw new UnwrapError(result.error)
  }

  try {
    return ok(body(unwrap))
  } catch (e) {
    if (e instanceof UnwrapError) {
      return err(e.error)
    }
    throw e
  }
}

Because then for every invocation of safeTry, you will have a different UnwrapError class. So with this implementation, in your example, the error would be rethrown by the inner safeParse, and caught by the outer one. result never gets assigned a value.

But at this point, it all becomes a bit weird and annoying, and I'm not sure I'm not missing any other edge cases. So probably the generator based version really is better.