[GitHub] [trafficcontrol] rawlinp commented on pull request #5162: Traffic Ops API Errors structure

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

rawlinp commented on pull request #5162:
URL: https://github.com/apache/trafficcontrol/pull/5162#issuecomment-710672368


   >>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.
   
   > No - they do need to both be checked, but I don't need to do it. That's what api.HandleErrs is for, it can all be checked in one place so I don't need to do a switch everywhere, or check two values every time. I really do think collapsing that would be an improvement.
   
   I agree, very few places would actually need to type switch. Most code would simply be checking `err != nil`, optionally adding context, then passing it up the chain. Maybe context would only be added if it was a `SysErr`, in which case you could use a helper function that only adds context if it's a `SysErr`.
   
   > It does seem more elegant, but the problem is that would entail changing logic, because there are places that return both errors, and I don't think that should be done without examining those use-cases.
   
   I know there are places where we simply return both errors if either are non-nil, but are there any cases where we actually _set_ both errors? I'm trying to think of a reason why an error would be both a system and a user error, and I'm coming up empty.
   
   > 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.
   
   So call the interface `APIError` consisting of `String() string` and `StatusCode() int`. What's important is that we have a single interface, with two concrete implementations (`SysErr` and `UserErr`). All functions would return `APIError`. Once the error is declared, you can't mutate `statusCode`, and you can't accidentally cast it to a plain `error` either.
   
   > If we wind up not making any new structure at all, to be honest I'd prefer "errCode, userErr, sysErr".
   
   I can get on board with that, assuming we can't come to a consensus on this `APIError` interface. Typing the error code first helps you remember what kind of error type you're dealing with to type next, and it's pretty easy to remember "user error first, sys error second" after typing about 12 return statements like that.


----------------------------------------------------------------
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