rob05c edited a comment on pull request #5162:
URL: https://github.com/apache/trafficcontrol/pull/5162#issuecomment-710554447
Personally, I don't like either a custom non-`error` struct, or a struct
that constains both and implements `error`.
The API really does need to do completely different things for user vs
system errors.
A single `error` interface seems more idiomatic-Go. But it's not really very
useful, if at all. Because you always need to check and handle both cases. And
if you ever don't, it's very easy to accidentally return a system error to a
user, or to log and not return a user error. You always need to check. It's
dangerous and bug-prone to hide that.
Moreover, if the common helper funcs just return an `error`, what does a
helper func that handles it do when it gets an `error` that isn't a
`UserAndSystemError`? Does it log? Or return to the user? Any answer is also
dangerous and bug-prone.
> any confusion about which error in the return signature is meant for
clients and which is only meant for logs
Thinking about it some more, I guess I'm not _that_ opposed to the `struct
Errors`. I can see the argument that it makes accidental swaps harder. But I am
opposed to that struct, or any single return, implementing `error`. I'm very
worried that anything that actually takes advantage of that is a bug.
> the (error, error, int) and (int, error, error)
That was my mistake, and I apologize. IMO the idiom should always be
"userErr, sysErr, errCode". The funcs that got it backward are my fault, and
should be fixed.
>The apierrors.Errors struct holds data for system errors, user errors, and
status codes, but we would only ever want to populate either SystemError or
UserError, never both. We need a solution that only allows us to return one or
the other, and the success status code should have nothing to do with the error
cases.
You want a Sum Type. Go doesn't do Monads sanely. Alas. I don't have a
strong opinion on whether any Errors type is `{UserErr, SystemErr}` or `{Err,
ErrIsUser}`. It does seem conceivable that an error could be both user and
system; but I don't think we've ever had that in practice.
I have no strong opinion on whether the return code is included in any Error
object. I do wish it could be conveniently strongly-typed, but all the standard
library funcs take `int`, which is why it isn't. An `api.HTTPCode` would
unfortunately make composing with the standard library painful.
TLDR
`-0` on `struct Errors` that doesn't implement `error`.
`-1` on any single "UserAndSystemErrAndOrCode" return that implements
`error`.
Also, can I request we name any new symbols `*Err` not `*Error`? IMO the
shorter name is more idiomatic Go, marginally easier to read and write, and
there's no other English word it would ever be confused with.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
