2

I have some trouble with go routines and channels regarding error handling.

Firstly I have a function that listen for messages (in a infinite for loop):

func main() {

    messageChannel := make(chan messageHandler.MessageInfo)

    for {
        if token := client.Subscribe("#", 0, func(client MQTT.Client, msg MQTT.Message) {
            go messageHandler.DecodeMessage(msg, messageChannel)
            select {
            case messageInfo := <-messageChannel:
                //Handle
            }

        }); token.Wait() && token.Error() != nil {
            fmt.Println(token.Error())
        }
    }


}

But in the DecodeMessage function, there could arise multiple errors.

func DecodeMessage(msg mqtt.Message, c1 chan MessageInfo) {

    //do something, might result in error

    //do another thing, might result in error

    c1 <- MessageInfo{...}
}

Normally I would just return from the function. But seems a bit trickier with routines. I've looked at this post, but if both errors would occur, I would only see the last error message.

Example:

func DecodeMessage(msg mqtt.Message, c1 chan MessageInfo) {

    var returnError error

    if err != nil {
        returnError = err
    }

    if err != nil {
        returnError = err
    }

    c1 <- MessageInfo{
        Error: returnError,
        ...
    }
}

Should I have an array of some sort and append all errors? Is it bad practice to have multiple errors in one routine?

The best thing, for me, is that the routine would exit on an error and return that error like it would do "normally". Is that possible?

gel
  • 281
  • 3
  • 19
  • Are you able to clarify why you are not able to return after the first error? Why do you need to continue on to the next thing in `DecodeMessage` after the first error? – jsageryd Apr 25 '18 at 19:01
  • How would I do that? Could I just use return as normally? – gel Apr 25 '18 at 19:03
  • Yes. This sort of pattern is pretty common and otherwise looks pretty similar to your typical Go pattern of `if err != nil { return err }` – Venantius Apr 25 '18 at 19:07
  • But I read this: https://stackoverflow.com/questions/20945069/catching-return-values-from-goroutines, which says it's a bad idéa. – gel Apr 25 '18 at 19:08
  • @gel If you had a normal function, would you continue processing even after the first error? If not, don't do it here either. Pass the error on the channel and return. Or perhaps better yet -- avoid the channel altogether in `DecodeMessage` and handle the channel sending outside of that function. Might make it easier to test it as well. – jsageryd Apr 25 '18 at 19:19

3 Answers3

0

I'll start by saying that having to go through all the error checks for a function before returning even if they fail is a bit of a code smell. It might mean that you have something weird going on and there may be a better way to accomplish what you're trying to do.

However, assuming that you've boiled down your problem to this, then I see two options, depending on how the errors have to be handled.

If the errors can be handled one-by-one and don't really rely on each other, then you can just create an error channel and send them back one by one as you encounter the errors. See the following working example:

package main

import (
    "errors"
    "fmt"
    "strings"
)

func main() {
    errCh := make(chan error)
    go HandleErrorsSerially("bad error", errCh)
    for err := range errCh {
        fmt.Printf("Found error serially: %v\n", err)
    }
}

func HandleErrorsSerially(msg string, errCh chan<- error) {
    if strings.Contains(msg, "error") {
        errCh <- errors.New("message contained string 'error'")
    }
    if strings.Contains(msg, "bad") {
        errCh <- errors.New("message contained string 'bad'")
    }
    close(errCh)
}

Alternatively, if you need to have a view of all the errors that occurred all at once (because two errors happening simultaneously may indicate some special circumstances) then you'd have to append them all to an array and then pass them through a channel. See the following working example:

package main

import (
    "errors"
    "fmt"
    "strings"
)

func main() {
    errArrCh := make(chan []error)
    go HandleErrorsTogether("bad error", errArrCh)
    errArr := <-errArrCh
    fmt.Printf("Found the following errors together: %v\n", errArr)
}

func HandleErrorsTogether(msg string, errArrCh chan<- []error) {
    errArr := make([]error, 0)
    if strings.Contains(msg, "error") {
        errArr = append(errArr, errors.New("message contained string 'error'"))
    }
    if strings.Contains(msg, "bad") {
        errArr = append(errArr, errors.New("message contained string 'bad'"))
    }
    errArrCh <- errArr
    close(errArrCh)
}
goose
  • 98
  • 7
0

I can see cases where it's useful to return multiple errors, such as when you are parsing messages and there are multiple bad fields and you need to summarise these back to a client.

I think the best approach is to use a package like hashicorp's multierror which allows multiple errors to be collected with formatting in a structure type that implements error interface and so can still be sent on a chan error. The receive side can then either process as just a standard error or extract the information on each individual error.

The multierror documentation is pretty good, just read through the examples on the github page.

GilesW
  • 485
  • 2
  • 7
0

The best thing, for me, is that the routine would exit on an error and return that error like it would do "normally".

Of course, you can, and get last error or all error are both pretty strange.

  1. Get last error is unhelpful for debugging, compare with getting first error.
  2. Get all error is same if the first error will cause the following failure, the following error message is unhelpful; Another situation is these errors do not have the association, I think this means you have to sperate they into different concurrency part for better controlling.

Ok, now back to original problem. Consider the code:

func foo(errTo chan error) {
    defer close(errTo)
    v, err := CouldFailOne()
    if err != nil {
        errTo <- err
        return // Yp, just stop this routine, let it join back to Invoker
    }
    v2, err := CloudFailTwo()
    if err != nil {
        errTo <- err
        return
    }
    // As the previous error handle until end of the function
}

If you want to return value from this kind of function. Just use a channel, and send the value into it only no error raise. I think this style will be more clear and like return style, just became using a channel to return the error.

林子篆
  • 84
  • 1
  • 4