Better error description

[rafaeljusto]

As we use only a string to describe low level errors, they can be very generic sometimes. We should create an error structure to store some context information to give a more detailed log information or to return better messages for the API user.

[rafaeljusto]

Could be an interface like:

type Error interface {
  Code() int
}

But this kind of solution would change all returns from error to Error.

[rafaeljusto]

Creating our own error and replacing the error returns isn't a good idea! We can still return Go error. The strategy could be encapsulate any external error to a system error. For internal server errors is a good idea to log it when it happens (with file and line). We will also need to detect on the uppers system levels if it's a external error or not, because the programmer could forgot to encapsulate it.

To improve the error response message, the error itself should follow the response protocol. And now we start to think that the protocol isn't a part of the REST component, but a part of the core.

[rafaeljusto]

Retrieve file and line:

_, file, line, ok := runtime.Caller(1)
if !ok {
  file = "???"
  line = 0
}

[rafaeljusto]

The error don't need to follow the response protocol! We could have an interface like this:

type ReportError interface {
  Message(language string) string
  Code() string
}

SystemError is an structure that only encapsulates another error, but when created it logs the file and line where the problem occurred.

func NewSystemError(err error) SystemError {
  // Log here!
  return SystemError{
    Error: err,
  }
}

type SystemError struct {
  Error error
}

func (err SystemError) Error() string {
  return err.Error.Error()
}

Then, in the upper level (maybe in an interceptor) we could do the following check:

switch err.(type) {
  case SystemError:
    // Return 500. Error was already logged in a lower level
  case ReportError:
    // Build error protocol calling Message and Code methods
  default:
    // Return 500. Log about low level error not encapsulated
}

[rafaeljusto]

Now all parts of the system need to generate a protocol message when there's an input error, so we need to move the protocol package to the project root level. That's the easy part!

The first cycle problem was the Pagination in the dao package that used protocol.Message, because the protocol package has dao.Pagination references. I solved this moving the pagination to the model package.

After some days working on it we have a cycle dependency between the packages protocol, messages and model. This happens because now in the model package when we need to alert an input error we use the protocol.Message. I tried to solve this creating a transcoder package, but the cycle problem persists due to the language check in the model package. This change is becoming a huge system refactory that could bring a lot of other problems.

Maybe what we could do is:

1. Move protocol package to root
2. Create transcoder package and move functions from protocol to it

This will probably solve the crazy cycle reference problem (or not!). But a less intrusive approach would be a special function in the package shelter/errors for input problems. Still thinking about...

[rafaeljusto]

I tried the approach to create a special function on shelter/errors. It seams that the only place that I need to return an input error is in the protocol package (that makes sense!). And for that, I need to create a better solution for the pagination object. One idea is to move it to the model package, and create a correspondent pagination package in the protocol with the conversion functions.

I also get stuck in some places that I'm not sure yet if I need to return a system error or not. In this situations the error could be encapsulated again by an input error in an upper layer.

As this refactoring will change a lot of things on the system, I need to think better on each move... But this solution appears to be a good one.

[rafaeljusto]

Cool ideas for encapsulating errors:
https://github.com/juju/errgo

[rafaeljusto]

Checking the juju errgo project we can have a new approach. All external errors are encapsulated into a system error that stores the file, line and low level error. For places that returns input errors the function will look like this:

func Check() ([]MessageResponse, error) {

}

As the only place that we detect the necessity of input errors is the protocol, we don't have a package dependency problem. This approach also allows us to identify an internal server error or a bad request. Another great reason to use this is that we can log only when we want, giving a less verbose log.

[rafaeljusto]

Steps

1. Protocol attributes must be all pointers
2. Create an Apply method in each model to receive (import) protocol objects
3. Create an Protocol method in each model to transform it into a protocol object
4. Create a Validate method for each protocol object to verify the attributes
5. Create a Normalize method for each protocol object to transform the attributes
6. Create an interface (visible only in the interceptor package) with Validate and Normalize functions
7. Create interceptors to call protocol Validate and Normalize methods
8. Change log package to use syslog (see Log in syslog #61)
9. Change interceptor to create specific log for each request with context
10. Change handlers to log non-users errors in syslog in the correct level
11. Refactory MessageResponse --> Message
12. Go through the project finding the remaining low level errors not encapsulated by SystemError

[rafaeljusto]

We could add the links to the message responses on an interceptor:

func (i *Interceptor) Before(w http.ResponseWriter, r *http.Request) {
  messageResponse.Links = []Link{
    {
        Types: []LinkType{LinkTypeRelated},
        HRef:  r.URL.RequestURI(),
    },
  }
}

With that we could simplify the message creation. The problem is if someday we start saving domains in groups, like a PUT in /domains, we will have problems to identify errors to a specific domain in the list.