Play nicely with multierror libraries

[Southclaws]

I'm honestly not super sure how this will work...

There was a case where a function iterated through a list of items and performed work that errored. The output error only contained one set of fctx metadata but ideally you'd have access to all error metadata.

Same goes for human-readable messages.

[Southclaws]

It might be impossible, it might just be a case of the user needing to write the multierr handler code in their error handling procedure which, given the error chain contains a multierr, flattens each sub-error using Fault and then merges the information in a way that makes sense to them.

If it can't be solved in-library, it should at least be documented so users know how to handle this case,.

[omarkohl]

Some options maybe:

• Require a slice of errors to fault.Wrap() as the first parameter (i.e. breaking change). The slice can always contain just a single error.
• Modify fault.Wrap() to take a variadic argument of type interface{} and then check inside the function whether they are errors (i.e. multi-error case) or whether they are of the fault.Wrapper type. i.e. function signature would look like this func Wrap(err error, p ...interface{}) error .
• Create a fault.WrapMulti() function that takes a slice of errors as the first parameter. (my favorite)
• Create a fault.WrapMulti() function with variadic argument of type interface{} as explained above.

Internally wrapped errors could always be stored in an error slice, which in the majority of cases would contain just a single error.

Some things that are not clear is what should happen with chain.Root when the root is a slice of errors with more than one error and how to display the errors in the end (i.e. just join everything with : or try to visually distinguish errors at the same level from those that were wrapped?).

Just some thoughts, in case any of it is useful.

I use https://github.com/hashicorp/go-multierror and it is useful for cases such as validating a document because it is more user-friendly to display all errors at once instead of breaking at the first validation error. Wrapping those errors would be semantically incorrect since they all live at the same level. I am not yet using fault, but I am considering it. I liked the fact that you already thought about the multierror case.

[Southclaws]

I am in agreement of fault.WrapMulti Something either like multierr's Append or Combine methods.

A curried function could also result in a slightly nicer DX by not requiring a slice to be initialised by hand:

// given a signature of:
// func fault.Combine(...error) func(...Wrapper)
fault.Combine(err1, err2, err3)(fctx.With(ctx), ftag.With(ftag.InvalidArgument))

vs

fault.Combine([]error{err1, err2, err3}, fctx.With(ctx), ftag.With(ftag.InvalidArgument))

On a somewhat related note, I've also considered creating a curried wrapper API because I do find my team writing the same old wrappers over and over. Outside of special cases, there are generally always a given set of pre-defined wrappers being passed (fctx, fmsg, some other custom ones)

w := fault.With(fctx.With(ctx), fmsg.With("authentication process failed"))

return nil, w(err)
return nil, w(err, ftag.With(ftag.InvalidArgument))

But I'm not convinced this is a good API design yet...

---

Some things that are not clear is what should happen with chain.Root when the root is a slice of errors with more than one error and how to display the errors in the end (i.e. just join everything with : or try to visually distinguish errors at the same level from those that were wrapped?).

This is pretty much why I've shied away from this issue for so long 😅 one of the main goals of Fault was to provide a way that was human-friendly to represent errors for both deployed environments with structured logging and local dev with the terminal. I want to make sure that however multi-errors are displayed, it's simple and easy to read in a terminal but also scales well to log aggregators.

One issue with multiple errors (and the new Golang Unwrap() []error interface) is that it turns your flat one-dimensional linked-list style chain structure into a tree structure which can get quite unwieldy if you're doing a lot of work behind a single request (which can happen in business-logic-heavy applications) then these kinds of call chains can easily produce 2 or 3 sites where multi-errors are used.

I am definitely interested in hearing more of your thoughts on this discussion! We have the exact same use-case for validation which uses multi-errors heavily and, similarly, does not wrap those errors heavily as there's no need. Keen to understand more about your use-cases!

[Southclaws]

Okay so here's one use-case I'm just jotting down for my own reference:

We have an API that publishes content, content on our platform always has many contributors, when a request to publish comes in, a bunch of validation must be done for each contributor. And, as you mentioned in your issue, it's not desirable to exit on first error so multi-error libraries are useful for doing all of the validation for all items first, then checking if one or more failed and return a multi-error.

So we're effectively doing a for each user in contributors: validate their contribution, wrap errors into multierr

One of the issues we have at the moment is that when there's an error and fctx binds the context metadata to the error, it always only contains the last contributor's account ID in the metadata. This leads to confusion when the operational side of the business sees an error message saying account_id=123 when that wasn't the actual account causing the validation issue.

I think what I'd like out of this is maybe something that looks like this:

{
  "error": "one or more contributor validations failed",
  "errors": [
    {
      "error": "failed to validate",
      "account_id": "12345",
      "errors": [
        {
          "error": "account missing agreement acceptance to publish",
          "message": "Account has not accepted legal terms."
        },
        {
          "error": "account missing address",
          "message": "Account has not specified an address."
        }
      ]
    },
    {
      "error": "failed to validate",
      "account_id": "abcdef",
      "errors": [
        {
          "error": "account missing address",
          "message": "Account has not specified an address."
        }
      ]
    }
  ]
}

Now of course this comes with a huge step up in complexity as it's not a flat data structure any more and we're storing multiple instances of the same metadata from context (account_id) so this cannot be flattened to a single level as it'll just overwrite (which is what happens now.)

And of course the other consideration is what does this look like on the terminal and for when the structure gets flattened for HTTP responses and log aggregators. Right now we're keeping those very flat so there's no complex JSON objects, there aren't even different types, it's just all strings which keeps things simple.

One thought I had was to, instead of coming up with some complex nested interface, just turn the wrapped error back into multiple errors and spit them all out together as individual logs. What I'm most conscious of is that I/we use a lot of structured logging to correlate events/errors and our log aggregator platform cares where the fields are in the response. So account_id must stay inside .metadata.account_id otherwise we'd have to rewrite all of our indexing rules.

But I'm not keen on the idea of spitting out 4 error log entries for one error just because it wrapped 3 other errors. And this still doesn't result in a simple way to encode this metadata for HTTP responses (which we currently do so the frontend has some contextual metadata to use for human-friendly error messages)

[omarkohl]

My best guess is that the following is not necessary ...

// given a signature of:
// func fault.Combine(...error) func(...Wrapper)
fault.Combine(err1, err2, err3)(fctx.With(ctx), ftag.With(ftag.InvalidArgument))

... because I assume in most cases the list of errors is built programmatically ...

var validationErrors []error
...
validationErrors = append(newErr, validationErrors)
...
fault.Combine(validationErrors)

Concerning the formatting it seems the new stdlib errors.Join leads to newline-separated error messages. Probably it should be made configurable how to convert such a tree of errors into a log message or HTTP response. The remaining question then is what does a sane default look like? Newlines somehow do not sound so great.

Maybe something like this for the simple text-based representation e.g. for outputting in the console. Context info can be embedded in the strings of course. The main idea is to use square brackets to indicate a list of errors (multi-error) and within the list separate the individual errors via pipes.

one or more contributor validations failed: [failed to validate: [account missing agreement acceptance to publish | account missing address] |  failed to validate: account missing address]

[Southclaws]

assume in most cases the list of errors is built programmatically

Perhaps, though codebases I work on are a mix of both, a variadic argument (regardless of whether it's curried or not) feels like an ergonomic middle-ground for both: fault.Combine(e1, e2, e3)/fault.Combine(validationErrors...) But not super opinionated on this yet, it probably doesn't matter a whole lot whichever way we go.

Newlines somehow do not sound so great

Agree here, the output is also not very clear when you have slightly more complex trees in the stdlib:

One of Fault's main goals is to make the local dev experience nicer so I'm going to spend some time thinking about how best to render a tree of errors that 1. doesn't involve JSON, 2. doesn't involve some weird complex ASCII terminal tree and 3. is easy to read.

I like your suggestion of the square brackets approach, reminds me of when you fmt.Print out a slice or map.