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

Make "bubbling" up errors more ergonomic. #479

Open
0xksure opened this issue May 11, 2023 · 10 comments
Open

Make "bubbling" up errors more ergonomic. #479

0xksure opened this issue May 11, 2023 · 10 comments

Comments

@0xksure
Copy link

0xksure commented May 11, 2023

First off, as a rust developer, this library makes node development so much more fun and ergonomic. However, and you are probably aware, there are still some limits that I would like to explore. It might be that I'm not that deep into the lib yet. I would really like to see an ergonomic way to handle a result and return an err() to the caller if an err was encountered.

In rust the famous match arms works like this

let val = match thisReturnsAResult.match(
(v) -> v,
(err) -> return Err(err)
) 

However, with neverthrow this is not that easy and I more than often default to

let val = match thisReturnsAResult.match(
(v) -> v,
(err) ->  {
  /// some logging
 return B
}
) 

if(val is B) return err("some error message")
...

where B is some subset of the return value A. This looks really ugly and is most likely an antipattern.

Is there a better pattern to use here instead or are there changes we can make to the lib to support something like this?

@0xksure 0xksure changed the title Make it "bubbling" up errors more ergonomic. Make "bubbling" up errors more ergonomic. May 11, 2023
@paduc
Copy link
Contributor

paduc commented May 11, 2023

Hello @0xksure

.match() is meant to be used primarily as a method to unwrap a Result, in which case any behavior for value/error cases would be inside the callbacks.

Maybe you would return a value but I can’t think of a case where you would return a Result… (unwrapping and wrapping in the same call ? That’s what map and mapErr are for !).

IMHO, you should only use match to unwrap the Result at the end of a call stack (ie at an application interface with the user).

@0xksure
Copy link
Author

0xksure commented May 12, 2023

Hello @0xksure

.match() is meant to be used primarily as a method to unwrap a Result, in which case any behavior for value/error cases would be inside the callbacks.

That makes sense. It just differs a bit in how you would usually do it in rust.

Maybe you would return a value but I can’t think of a case where you would return a Result… (unwrapping and wrapping in the same call ? That’s what map and mapErr are for !).

According to the linter rule you have to unwrap, match or unwrapOr a Result. I therefore assumed going straight to mapErr was an anti pattern.

IMHO, you should only use match to unwrap the Result at the end of a call stack (ie at an application interface with the user).
Exactly, that makes sense. Then why is the linter so strict on unwrapping the result?

@0xksure
Copy link
Author

0xksure commented May 23, 2023

ping @paduc

@paduc
Copy link
Contributor

paduc commented May 23, 2023

@0xksure I’m not sure what you are trying to achieve. If you have a Result and want to return an Err if the Result is an Err, then you should use mapErr.

I believe you might want to have the value of it’s an Ok(value) and an Err if it’s an Err. But an Err is only useful in the context of a Result. So it would not make sense to mix a value with an Err.

@0xksure
Copy link
Author

0xksure commented May 23, 2023

@0xksure I’m not sure what you are trying to achieve. If you have a Result and want to return an Err if the Result is an Err, then you should use mapErr.

@paduc I simply want a similar control flow to this https://doc.rust-lang.org/std/result/. An example is

fn write_info(info: &Info) -> io::Result<()> {
    // Early return on error
    let mut file = match File::create("my_best_friends.txt") {
           Err(e) => return Err(e),
           Ok(f) => f,
    };
    if let Err(e) = file.write_all(format!("name: {}\n", info.name).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("age: {}\n", info.age).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("rating: {}\n", info.rating).as_bytes()) {
        return Err(e)
    }
    Ok(())
}

where the method exits early in a match if it's an Err.

I believe you might want to have the value of it’s an Ok(value) and an Err if it’s an Err. But an Err is only useful in the context of a Result. So it would not make sense to mix a value with an Err.

No I want to return the Err immediately if I match on an Error. This is what I mean by bubbling up errors. The root caller can decide what to do with the error instead of having one of the child methods making the decision.

UPDATE: I see from the signature https://github.com/supermacro/neverthrow/blob/master/src/result.ts#L307 that it will be pretty hard to replicate the same control flow as in Rust. However, it would be really cool to have a method that allows to exit the scope and return an err to the caller. Do you have any ideas for this @paduc?

@0xksure
Copy link
Author

0xksure commented May 23, 2023

It seems as this https://github.com/supermacro/neverthrow/pull/448/files is at least closer to what I imagine, given of course that the Error type is the same

@paduc
Copy link
Contributor

paduc commented May 23, 2023

@paduc I simply want a similar control flow to this https://doc.rust-lang.org/std/result/. An example is

fn write_info(info: &Info) -> io::Result<()> {
    // Early return on error
    let mut file = match File::create("my_best_friends.txt") {
           Err(e) => return Err(e),
           Ok(f) => f,
    };
    if let Err(e) = file.write_all(format!("name: {}\n", info.name).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("age: {}\n", info.age).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("rating: {}\n", info.rating).as_bytes()) {
        return Err(e)
    }
    Ok(())
}

I don't know Rust but in typescript, I would write it:

function write_info(info): Result<null, Error> {
  
  // let's say createFile returns a Result<File, Error>
  // and file.write_all returns a Result<null, Error>

  return createFile('my_best_friends.txt')
    // andThen only executes if the Result is Ok()
    // so it "returns" at the first Err()
    // PS: I used Result.map() to hand the file instance to the next step
    .andThen((file) => file.write_all('name: {}\n').map(() => file))
    .andThen((file) => file.write_all('age: {}\n').map(() => file))
    .andThen((file) => file.write_all('rating: {}\n'))
}

@0xksure
Copy link
Author

0xksure commented May 23, 2023

@paduc I simply want a similar control flow to this https://doc.rust-lang.org/std/result/. An example is

fn write_info(info: &Info) -> io::Result<()> {
    // Early return on error
    let mut file = match File::create("my_best_friends.txt") {
           Err(e) => return Err(e),
           Ok(f) => f,
    };
    if let Err(e) = file.write_all(format!("name: {}\n", info.name).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("age: {}\n", info.age).as_bytes()) {
        return Err(e)
    }
    if let Err(e) = file.write_all(format!("rating: {}\n", info.rating).as_bytes()) {
        return Err(e)
    }
    Ok(())
}

I don't know Rust but in typescript, I would write it:

I was under the impression that neverthrow was highly inspired by the Rust Result implementation.

function write_info(info): Result<null, Error> {
  
  // let's say createFile returns a Result<File, Error>
  // and file.write_all returns a Result<null, Error>

  return createFile('my_best_friends.txt')
    // andThen only executes if the Result is Ok()
    // so it "returns" at the first Err()
    // PS: I used Result.map() to hand the file instance to the next step
    .andThen((file) => file.write_all('name: {}\n').map(() => file))
    .andThen((file) => file.write_all('age: {}\n').map(() => file))
    .andThen((file) => file.write_all('rating: {}\n'))
}

Thanks for the example, it makes it way easier to reason about. It seems like I might have to change the way I think about using neverthrow to a more functional way of handling Results. Although, the PR that's out for replicating the rust ? functionality is pretty cool and would make the package a bit more rusty.

@paduc
Copy link
Contributor

paduc commented May 23, 2023

You are right about rust being the inspiration for this library but I didn’t create it, @supermacro did 😉. I’m just a contributor.

The PR uses generators which I’m not familiar with either. So I can’t judge the value.

@0xksure
Copy link
Author

0xksure commented May 24, 2023

Then Rust is worth checking out. I would imagine a lot of users of the package are writing or experimenting with rust on the side. Rust in typescript would be so nice.

I'm not that familiar either, but if it's solves the problem then that's cool.

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

2 participants