26

Is there a way to clean up this (IMO) horrific-looking code?

    aJson, err1 := json.Marshal(a)
bJson, err2 := json.Marshal(b)
cJson, err3 := json.Marshal(c)
dJson, err4 := json.Marshal(d)
eJson, err5 := json.Marshal(e)
fJson, err6 := json.Marshal(f)
gJson, err4 := json.Marshal(g)
if err1 != nil {
    return err1
} else if err2 != nil {
    return err2
} else if err3 != nil {
    return err3
} else if err4 != nil {
    return err4
} else if err5 != nil {
    return err5
} else if err5 != nil {
    return err5
} else if err6 != nil {
    return err6
} 

Specifically, I'm talking about the error handling. It would be nice to be able to handle all the errors in one go.

5
  • The title is about error handling, but you seem to be returning them instead of going panic(), is there a reason why? And also for not returning immediatly as soon as 1 Marshall has failed ? Commented Mar 13, 2013 at 23:39
  • 7
    Why would I use panic() rather than returning the errors? From the docs you linked to: "The usual way to report an error to a caller is to return an error as an extra return value. " I feel that this is a "usual" case. It's not an error of such severity that using panic() feels justified. (Please correct me if I'm misunderstanding the use cases for the panic function.) Commented Mar 13, 2013 at 23:54
  • 3
    i know you got upvoted for stating what every gopher will agree with, not using/abusing panic, but the code sample above is just really bad/vague. Marshal everything but only return the first error we see afterwards? But then you clarify "handling all errors in one go", mostly nullifying the code sample. Again it's dependent on what your doing, and panic/recover might be appropriate. Personally, I'd error out immediately on a failed marshal and be more descriptive that cJson failed, attached with the err. But even that depends on the context of where this is being ran. Commented Mar 14, 2013 at 14:02
  • Good points. The code I gave was ambiguous. I would've checked for an error after each marshall — the only thing that stopped me was that I thought it looked neater to group together the marshall calls. IMO, it doesn't really matter that not all of the errors will be returned. The behaviour I was after was "return the first non-nil error." Commented Mar 14, 2013 at 16:50
  • Of interest, for Go 1.20 (Q4 2022): "proposal: errors: add support for wrapping multiple errors" Commented Oct 19, 2022 at 12:04

8 Answers 8

25
var err error
f := func(dest *D, src S) bool {
    *dest, err = json.Marshal(src)
    return err == nil
} // EDIT: removed ()

f(&aJson, a) &&
    f(&bJson, b) &&
    f(&cJson, c) &&
    f(&dJson, d) &&
    f(&eJson, e) &&
    f(&fJson, f) &&
    f(&gJson, g)
return err
Sign up to request clarification or add additional context in comments.

Comments

14

Put the result in a slice instead of variables, put the intial values in another slice to iterate and return during the iteration if there's an error.

var result [][]byte
for _, item := range []interface{}{a, b, c, d, e, f, g} {
    res, err := json.Marshal(item)
    if err != nil {
        return err
    }
    result = append(result, res)
}

You could even reuse an array instead of having two slices.

var values, err = [...]interface{}{a, b, c, d, e, f, g}, error(nil)
for i, item := range values {
    if values[i], err = json.Marshal(item); err != nil {
        return err
    }
}

Of course, this'll require a type assertion to use the results.

1 Comment

This answer deserves the best answer tag!
7

define a function.

func marshalMany(vals ...interface{}) ([][]byte, error) {
    out := make([][]byte, 0, len(vals))
    for i := range vals {
        b, err := json.Marshal(vals[i])
        if err != nil {
            return nil, err
        }
        out = append(out, b)
    }
    return out, nil
}

you didn't say anything about how you'd like your error handling to work. Fail one, fail all? First to fail? Collect successes or toss them?

Comments

5

I believe the other answers here are correct for your specific problem, but more generally, panic can be used to shorten error handling while still being a well-behaving library. (i.e., not panicing across package boundaries.)

Consider:

func mustMarshal(v interface{}) []byte {
    bs, err := json.Marshal(v)
    if err != nil {
        panic(err)
    }
    return bs
}

func encodeAll() (err error) {
    defer func() {
        if r := recover(); r != nil {
            var ok bool
            if err, ok = r.(error); ok {
                return
            }
            panic(r)
        }
    }()

    ea := mustMarshal(a)    
    eb := mustMarshal(b)
    ec := mustMarshal(c)

    return nil
}

This code uses mustMarshal to panic whenever there is a problem marshaling a value. But the encodeAll function will recover from the panic and return it as a normal error value. The client in this case is never exposed to the panic.

But this comes with a warning: using this approach everywhere is not idiomatic. It can also be worse since it doesn't lend itself well to handling each individual error specially, but more or less treating each error the same. But it has its uses when there are tons of errors to handle. As an example, I use this kind of approach in a web application, where a top-level handler can catch different kinds of errors and display them appropriately to the user (or a log file) depending on the kind of error.

It makes for terser code when there is a lot of error handling, but at the loss of idiomatic Go and handling each error specially. Another down-side is that it could prevent something that should panic from actually panicing. (But this can be trivially solved by using your own error type.)

Comments

3

You can use go-multierror by Hashicorp.

var merr error

if err := step1(); err != nil {
    merr = multierror.Append(merr, err)
}
if err := step2(); err != nil {
    merr = multierror.Append(merr, err)
}

return merr

1 Comment

That actually might be natively supported by a future version (Go 1.2x, for 2023), with a slice/tree of errors.
3

Go 1.22

Starting from this version, you can use cmp.Or from the standard library.

Or returns the first of its arguments that is not equal to the zero value. If no argument is non-zero, it returns the zero value.

The function is generic with a comparable type parameter. Since Go 1.20 basic interfaces can satisfy comparable, therefore this function works for errors too:

func main() {
    err1 := doSomething(1)
    err2 := doSomething(2)
    err3 := doSomething(3)
    err4 := doSomething(4)
    err5 := doSomething(5)
    err6 := doSomething(6)

    err := cmp.Or(err1, err2, err3, err4, err5, err6)
    if err != nil {
        fmt.Println(err)
        return
    }

}

func doSomething(n int) error {
    if n == 5 {
        return errors.New("wrong number: 5")
    }
    return nil
}

The code above will print the error message from the first non-nil error which is the err5 variable.

Playground: https://go.dev/play/p/rrq4qO9WV7c?v=gotip

NOTE: as for when an error variable is set to its zero value, keep in mind the usual caveats.

Comments

2

You can create a reusable method to handle multiple errors, this implementation will only show the last error but you could return every error msg combined by modifying the following code:

func hasError(errs ...error) error {
    for i, _ := range errs {
        if errs[i] != nil {
            return errs[i]
        }
    }
    return nil
}

aJson, err := json.Marshal(a)
bJson, err1 := json.Marshal(b)
cJson, err2 := json.Marshal(c)

if error := hasError(err, err1, err2); error != nil {
    return error
}

Comments

1

Another perspective on this is, instead of asking "how" to handle the abhorrent verbosity, whether we actually "should". This advice is heavily dependent on context, so be careful.

In order to decide whether handling the json.Marshal error is worth it, we can inspect its implementation to see when errors are returned. In order to return errors to the caller and preserve code terseness, json.Marshal uses panic and recover internally in a manner akin to exceptions. It defines an internal helper method which, when called, panics with the given error value. By looking at each call of this function, we learn that json.Marshal errors in the given scenarios:

  • calling MarshalJSON or MarshalText on a value/field of a type which implements json.Marshaler or encoding.TextMarshaler returns an error—in other words, a custom marshaling method fails;
  • the input is/contains a cyclic (self-referencing) structure;
  • the input is/contains a value of an unsupported type (complex, chan, func);
  • the input is/contains a floating-point number which is NaN or Infinity (these are not allowed by the spec, see section 2.4);
  • the input is/contains a json.Number string that is an incorrect number representation (for example, "foo" instead of "123").

Now, a usual scenario for marshaling data is creating an API response, for example. In that case, you will 100% have data types that satisfy all of the marshaler's constraints and valid values, given that the server itself generates them. In the situation user-provided input is used, the data should be validated anyway beforehand, so it should still not cause issues with the marshaler. Furthermore, we can see that, apart from the custom marshaler errors, all the other errors occur at runtime because Go's type system cannot enforce the required conditions by itself. With all these points given, here comes the question: given our control over the data types and values, do we need to handle json.Marshal's error at all?

Probably no. For a type like

type Person struct {
    Name string
    Age  int
}

it is now obvious that json.Marshal cannot fail. It is trickier when the type looks like

type Foo struct {
    Data any
}

(any is a new Go 1.18 alias for interface{}) because there is no compile-time guarantee that Foo.Data will hold a value of a valid type—but I'd still argue that if Foo is meant to be serialized as a response, Foo.Data will also be serializable. Infinity or NaN floats remain an issue, but, given the JSON standard limitation, if you want to serialize these two special values you cannot use JSON numbers anyway, so you'll have to look for another solution, which means that you'll end up avoiding the error anyway.

To conclude, my point is that you can probably do:

aJson, _ := json.Marshal(a)
bJson, _ := json.Marshal(b)
cJson, _ := json.Marshal(c)
dJson, _ := json.Marshal(d)
eJson, _ := json.Marshal(e)
fJson, _ := json.Marshal(f)
gJson, _ := json.Marshal(g)

and live fine with it. If you want to be pedantic, you can use a helper such as:

func must[T any](v T, err error) T {
    if err != nil {
        panic(err)
    }
    return v
}

(note the Go 1.18 generics usage) and do

aJson := must(json.Marshal(a))
bJson := must(json.Marshal(b))
cJson := must(json.Marshal(c))
dJson := must(json.Marshal(d))
eJson := must(json.Marshal(e))
fJson := must(json.Marshal(f))
gJson := must(json.Marshal(g))

This will work nice when you have something like an HTTP server, where each request is wrapped in a middleware that recovers from panics and responds to the client with status 500. It's also where you would care about these unexpected errors—when you don't want the program/service to crash at all. For one-time scripts you'll probably want to have the operation halted and a stack trace dumped.

If you're unsure of how your types will be changed in the future, you don't trust your tests, data may not be in your full control, the codebase is too big to trace the data or whatever other reason which causes uncertainty over the correctness of your data, it is better to handle the error. Pay attention to the context you're in!


P.S.: Pragmatically ignoring errors should be generally sought after. For example, the Write* methods on bytes.Buffer, strings.Builder never return errors; fmt.Fprintf, with a valid format string and a writer that doesn't return errors, also returns no errors; bufio.Writer aswell doesn't, if the underlying writer doesn't return. You will find some types implement interfaces with methods that return errors but don't actually return any. In these cases, if you know the concrete type, handling errors is unnecessarily verbose and redundant. What do you prefer,

var sb strings.Builder
if _, err := sb.WriteString("hello "); err != nil {
    return err
}
if _, err := sb.WriteString("world!"); err != nil {
    return err
}

or

var sb strings.Builder
sb.WriteString("hello ")
sb.WriteString("world!")

(of course, ignoring that it could be a single WriteString call)?

The given examples write to an in-memory buffer, which unless the machine is out of memory, an error which you cannot handle in Go, cannot ever fail. Other such situations will surface in your code—blindly handling errors adds little to no value! Caution is key—if an implementation changes and does return errors, you may be in trouble. Standard library or well-established packages are good candidates for eliding error checking, if possible.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.