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

proposal: Go 2: error handling with error receiver function #36338

Open
drupsys opened this issue Dec 31, 2019 · 16 comments
Open

proposal: Go 2: error handling with error receiver function #36338

drupsys opened this issue Dec 31, 2019 · 16 comments

Comments

@drupsys
Copy link

@drupsys drupsys commented Dec 31, 2019

Hello everyone,

What if a function that receives only an error could be used as an error receiver for a failed function call?

func errorReceiver(err error) error {
    return fmt.Errorf("some error: %v", err) // do something
}

OK this may be completely crazy also maybe someone already suggested something similar but hear me out...

Overview

// Higher order function that returns an error receiver
func errorHandler(src string) (func(error) error) {
    return func(err error) error {
        return fmt.Errorf("Failed to open file %s: %v", err)
    }
}

func example(src string) error {
    errorReceiver := errorHandler(src)
    r := os.Open(src) or errorReceiver

    // do something with r

    return nil
}

Assume that os.Open fails with an error, since the signature of errorReceiver takes an error as argument the errorReceiver could be called with the os.Open error. Lets see what that would do to the order of execution.

  1. r := os.Open(src) or errorReceiver
  2. os.Open(src) returns nil, fmt.Error("Some error")
  3. call errorReceiver(fmt.Error("Some error"))
  4. If result of errorReceiver is still an error then assign errorReceiver's error to example's return error and exit from example
  5. If result of errorReceiver is nil then continue execution of example

With only one new language concept we are able to reusable handle errors e.g.

func errorHandler(src, dst string) (func(error) error) {
    return func(err error) error {
        return fmt.Errorf("copy %s %s: %v", src, dst, err)
    }
}

func CopyFile(src, dst string) (err error) {
    errorReceiver := errorHandler(src, dst)

    r := os.Open(src) or errorReceiver
    defer r.Close()

    w := os.Create(dst) or errorReceiver
    defer func() {
        w.Close()
        if err != nil {
            os.Remove(dst) // only if a “try” fails
        }
    }()

    io.Copy(w, r) or errorReceiver
    w.Close() or errorReceiver

    return nil
}

If in addition to this go was also able to infer the return type of higher order functions then you could end up with pretty clean syntax for error handlers. The above error handler rewritten would look something like.

// The return type of func errorHandler(src, dst string) is implied to be func(error) error
func errorHandler(src, dst string) => func(err error) error {
    return fmt.Errorf("copy %s %s: %v", src, dst, err)
}

Although even without it, this would be a significant improvement.

Handling errors and recovering from them

What if you actually want to handle the error not just simply enrich it? A more sophisticated implementation of error receiver could be implemented. So far all examples of error receiver returned an error, lets go back to my initial example.

func errorHandler(src string) (func(error) error) {
    return func(err error) error {
        return fmt.Errorf("Failed to open file %s: %v", err)
    }
}

What if error receiver here return just the *File, which is the first return value of os.Open, e.g.

func errorReceiver(err error) *File {
    return &File{} // return some default file instead
}

func example(src string) error {
    r := os.Open(src) or errorReceiver

    // do something with r

    return nil
}

Since errorReceiver is no longer returning an error but the expected value of os.Open then the result of errorReceiver could just be assigned to r and the execution of the example could continue, this effectively handles the error.
Note that if os.Open returned more than one non error value e.g. func Open() (T1, T2, ...Tn, error) then error receiver would have to have the following signature func(error) (T1, T2, ...Tn)

More concretely error receiver would have the following valid signatures:

  • func(error) error - Error receiver that always results in a failure
  • func(error) T1, T2, ...Tn - Error receiver that always results in recovery
  • func(error) T1, T2, ...Tn, error - Error receiver that may recover or fail

Final example to show a case where an error receiver itself might recover or fail

func errorHandler(user string) (func(error) error) {
    return func(err error) error {
        return fmt.Errorf("User %s profile could not be found: %v", user, err)
    }
}

func downloadProfileFromAnotherSource(user string) (func(error) (Profile, error)) {
    return func(err error) (Profile, error) {
        profile := serviceB.Find(user) or errorHandler(user)
        return profile, nil
    }
}

func DownloadProfileA(user string) (Profile, error) {
    profile := serviceA.Find(user) or downloadProfileFromAnotherSource(user)
    return profile, nil
}

Conclusion

I think this is a rather neat approach since the only new thing added here is, admittedly, a slightly bizarre behaviour of error receiver when it returns, although the official try(...) error handling proposal was effectively doing the same thing, everything else here is just basic functions. Or am I just having brainfarts? Anyway, I hope someone will find this interesting.

Thanks.

@gopherbot gopherbot added this to the Proposal milestone Dec 31, 2019
@gopherbot gopherbot added the Proposal label Dec 31, 2019
@ianlancetaylor ianlancetaylor changed the title Proposal: Error handling with error receiver function proposal: Go 2: error handling with error receiver function Dec 31, 2019
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 31, 2019

If I write code like this

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    if b {
        f, errorReceiver := os.Open("file")
        // Do something with f
    }
}

I don't understand how we can reliably and simply decide that the assignment to errorReceiver from the call to os.Open should call the existing variable rather than define a new variable of type error.

@drupsys
Copy link
Author

@drupsys drupsys commented Dec 31, 2019

@ianlancetaylor that is an excellent point, I was really trying to avoid it but I guess the only way for this to work is to have a syntax change that would explicitly state that errorReceiver must be called, something like:

f, ^errorReceiver := os.Open("file") I've used ^ here but I'm not sure what symbol would be appropriate or f, errorReceiver(err) := os.Open("file") or f, call errorReceiver := os.Open("file") or f := os.Open("file"):errorReceiver this one is more like an error capture, syntax here would mean capture error with errorReceiver.

I like the last one a lot personally, so your example would become:

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    if b {
        f := os.Open("file"):errorReceiver
        // Do something with f
    }
}

Which would resolve the ambiguity of my initial proposal.

@drupsys
Copy link
Author

@drupsys drupsys commented Jan 1, 2020

P.S. now that I think about it this

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    if b {
        f := os.Open("file"):errorReceiver
        // Do something with f
    }
}

is just syntactic sugar for

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    if b {
        f, err := os.Open("file")
        if err != nil {
                if err2 := errorReceiver(err); err2 != nil {
                        return err2
                } 
        }
        // Do something with f
    }
}
@icholy
Copy link

@icholy icholy commented Jan 2, 2020

I looks like using : might prevent using it as an expression for a map literal key.

func key() (string, error) {
    return "key", nil
}

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    if b {
        m := map[string]bool{
            key():errorReceiver: true,
        }
    }
}
@drupsys
Copy link
Author

@drupsys drupsys commented Jan 2, 2020

I don't think symbol should be the focus here as we can easily use something else, in fact I don't think I like 1 character symbol at all, since this affects the flow of control it should probably be a keyword:

  • key() or errorReceiver
  • key() handle errorReceiver
  • ...

But more importantly are there any fundamental flaws with this approach that I'm not seeing, would this solve majority of issues that people have with the current error handling and most importantly is this go like. These are the questions I don't have answers to.

@icholy
Copy link

@icholy icholy commented Jan 3, 2020

Well then this isn't too different from try then. It was suggested that it could/would be extended with an optional second "receiver" parameter.

func F(b bool) error {
    errorReceiver := func(err error) error { return err }
    f := try(os.Open("file"), errorReceiver)
    // Do something with f
}
@drupsys
Copy link
Author

@drupsys drupsys commented Jan 4, 2020

TLDR; You should not expect any error handling proposal to be drastically different from try and there is a good reason for it.

I was not aware try proposal had this variant, also if we ever have anything to help with redundant error handling I do not believe it will be massively different from try, at least for as long as we are trying to solve the same problem.

To clarify what I mean, the problem we are all trying to address (as far as I can tell) is highly repetitive error handling, the solution to repetitive code is well known simply factor out code into reusable functions. We should be able to apply the same solution to error handling, I do not see any reason to reinvent the wheel here.

This proposal, try and every other proposal I've read are similar at the core, they are all using some kind of early termination method, more importantly there is no other way to resolve this problem (well... that is not entirely true, there is one other method I can think of... fall-through but I don't think that's a right solution), in other words whatever the solution we chose will involve some kind of early termination and will be largely similar to every other proposal.

If you think about it, this solution, try even more traditional try catch are all just different permutations of the same solution. At its core try proposal was correct, I had some personal issues with the exact implementation, for example I think it is a very bad idea to have a special function that looks like a function but does not behave like a standard function and yes I realise we have panic but just because it was done once doesn't mean it is a good idea.

@urandom
Copy link

@urandom urandom commented Jan 6, 2020

I was not aware try proposal had this variant, also if we ever have anything to help with redundant error handling I do not believe it will be massively different from try, at least for as long as we are trying to solve the same problem.

It wasn't. It appeared in a few comments, but it was never part of the proposal itself. Which is a problem, since one of the biggest criticisms was that you couldn't easily add context to the error (you'd have to use named returns and defers, which added quite a bit of verbosity).

Your proposal, to me, seems to cover 2 of the biggest criticisms of the original try proposal.

With this one, you can easily add context to an error. You can also have helper functions in the stdlib that can make it even easier (e.g.: fmt.ErrorHandler(fmt string, args ...interface{}) func(error) error).

Unlike try, you are not proposing a function, therefore there will be no confusion as to whether this is a function or some sort of control flow. I think this was the biggest mistake of the original try proposal. Whether this proposal goes with a symbol, or (my personal preference) a keyword (or), there can be no confusion as to what the code does. With the proper keyword, a reader might not even need to have knowledge of the language to have an educated guess as to what the construct might do.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 28, 2020

I know that you are not insisting on a particular syntax, but the syntax does matter. One problem with the :handler syntax is that it is obscure. It seem easy to miss the :, and as noted it is confusing when used with composite literals.

Also, it will be very cryptic to a person new to Go. At least try was a clear indication that something was happening, though people would have to learn that it could affect flow control. The : does not have any clear meaning at all (I think).

So I think this needs more attention to syntax.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jan 28, 2020

FYI, here's the discussion of the try version that used an explicit error handler and some of the questions around it: https://github.com/golang/proposal/blob/master/design/32437-try-builtin.md#design-iterations.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 28, 2020

Since this is a language change proposal, could you please fill out the template at https://go.googlesource.com/proposal/+/bd3ac287ccbebb2d12a386f1f1447876dd74b54d/go2-language-changes.md .

When you are done, please reply to the issue with @gopherbot please remove label WaitingForInfo.

Thanks!

@guilt
Copy link

@guilt guilt commented Feb 2, 2020

How about an alternate syntax:

erecv <- f := os.Open(file)

I'd also prefer an inline variant:

{return nil, err} <- f, err := os.Open(file)  
@drupsys
Copy link
Author

@drupsys drupsys commented Feb 9, 2020

@ianlancetaylor

"One problem with the :handler syntax is that it is obscure. It seem easy to miss the :, and as noted it is confusing when used with composite literals" - I agree.

"Also, it will be very cryptic to a person new to Go" - Also agree.

As I was saying : is not a good operator for this, I've come to like or a lot over the last couple of weeks. However, using a word for this operator, a word that has meaning, is making me re-think what the purpose of this operator is.

My original proposal intended the : operator to do two things

  1. one of:
    • attach context to errors
    errorReceiver := func(err error) error { return err }
    f := os.Open("file") or errorReceiver
    // (file), (err)     or (err)
    • handle error
    errorReceiver := func(err error) *File { return &File{...} }
    f := os.Open("file") or errorReceiver
    // (file), (err)     or (file)
  2. return from current function if err after handler is executed is still not nil.
    • these would not terminate current function
    errorReceiver := func(err error) error { return nil }
    f := os.Open("file") or errorReceiver
    // (file), (err)     or (nil)
    
    errorReceiver := func(err error) *File { return &File{...} }
    f := os.Open("file") or errorReceiver
    // (file), (err)     or (file)
    • this would terminate current function
    errorReceiver := func(err error) error { return err }
    f := os.Open("file") or errorReceiver
    // (file), (err)     or (err)

The meaning of word or would work great with 1 but or does not imply that the current function will terminate in any way so behaviours of 2 are obscured. I haven't worked through the logic of this fully but I think there are actually two operators hiding here.

Perhaps something like the below is more logically sound:

errorReceiver := func(err error) error { return err }
f, err := os.Open("file") or errorReceiver
handle err

or more concisely:

errorReceiver := func(err error) error { return err }
f := handle (os.Open("file") or errorReceiver)

or operator would implement behaviours described in 1
handle operator would implement behaviours described in 2

Just wanted to give an update as I was away thinking about this for a couple of weeks.

@drupsys
Copy link
Author

@drupsys drupsys commented Feb 9, 2020

@guilt I kind of agree with others that symbols are not the way to go (... I'll walk myself out).

@urandom
Copy link

@urandom urandom commented Feb 9, 2020

@drupsys I personally don't think that the function handler passed to the operator should return any other type other than the one passed in. It should always look like (in a generic syntax):
func (type T)(t T) T { ... }

I would agree that if the handler returns nil, the control should not be terminated. Though any other assignment (in your example the *File) should be zeroed, and the user should then have to initialize that variable in a separate control flow (if file == nil { ... })

@guilt
Copy link

@guilt guilt commented Feb 9, 2020

@drupsys - For me it's less about symbols but more about making error handling concise to relay, where possible.

I'm also all for having it explicitly modeled as a variable (err) than an invisible variable (throw)

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

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.