[GitHub] [trafficcontrol] rob05c edited a comment on pull request #5162: Traffic Ops API Errors structure

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org