fix: Join errors only if necessary

[mitar]

Standard library's errors.Join always joins errors which are a bit trickier to work with than regular errors. So adding a simple check to make sure errors.Join is really necessary.

[mitar]

@alecthomas Any chance this could be merged in?

[alecthomas]

What does "which are a bit trickier to work with than regular errors" mean?

The only issue I've had with them is the formatting, but other than that they seem totally fine.

[mitar]

Exactly. When serializing them to JSON for logging, I get now unnecessary nested errors. Similarly, when printing them to the console, recursively, there is unnecessary extra level because of this join.

So yea, errors.Is and errors.As work well. But when not just traversing but marshaling or formatting, this is unnecessary.

[alecthomas]

Mmm...that doesn't make much sense does it? errors.Join() can't be JSON marshalled anyway, and in the case where there is more than one error you're going to have to deal with it anyway?

I don't think this makes sense to be in kong, vs. wherever you're consuming the error.

Happy to reconsider if you have a compelling reason.

[mitar]

Oh, I have whole errors package which supports JSON marshaling to JSON of arbitrary errors and trees of errors, it also extracts stack traces and other potential extra metadata/details, etc. So yes, errors can be JSON marhaled. Same for formatting.

So having one unnecessary join in-between, it makes things ugly. I can of course special-case it on my side. But from looking at Kong's code it seems pretty trivial to fix this. It is also not in any critical path. Check is simple. So I decided to contribute it here.

[mitar]

I would claim that stdlib's Join should not be joining if there is only one non-nil error in there. My package's Join does not do that. Which makes it easy to use in defer and other statements, where you want to combine it with some other error, but you do not want at every call introduce a new join error, even if there was no error from defer. See example here.

[mitar]

I investigated. The main issue is that I use pattern with Kong's ctx.Run to run Run method on the config struct. The main issue to me is that Kong currently always unconditionally and unnecessary changes the error returned from Run method (which I implement and control) so when I receive it from ctx.Run it is not the same as what I returned. And it does that even if I do not have AfterRun hooks defined. I understand that error is wrapped inside it, but then to get original error I have to do extra conditional (if there is only one joined error) unwrapping after ctx.Run. One reason for that is to check if error has a stack trace, and joined error never does, while original error might have it. And I cannot just blindly check if any error wrapped has a stack trace because there might be a stack trace somewhere wrapped deeper in, but that might not be a stack trace we want.

So in summary, I would assume ctx.Run is just a thin wrapper to call struct's Run and not that it is modifying its return value.