Public interfaces should return std error rather than custom type

[ifraixedes]

Returning custom error type rather standard error in public interfaces is inconvenient.

First reason is that it forces to other implementations to use the same error type although it satisfies the standard error interface and the return errors shouldn't be a strong requirement for public interfaces, rather than being a standard error; on the other hand it's idiomatic to return standard errors.

Second reason is that when the returned error is nil, it cannot be coerced to a standard error without losing the the ability to be compare with nil.

For example (simple silly one in terms of logic), it cannot be done something like this

func myInstances() ([]*ec2.DescribeInstancesOutput, error) {
    // Do whatever initializations or other custom operations here
    // in order to get the AWS Reader implementation instance, which
    // below is set to the variable `c`

    return c.GetInstances(nil)
}

// In other place where the call to such function is done
var instances, err = myInstances()

// although the `c.GetInstances` returned `nil`, `err` isn't nil, because it has been coerced
// `error` producing a interface which value isn't nil, it's a pointer to the original error type 
// which value is `nil`, making it incomparable to `nil`.
if err != nil {
    os.Exit(1)
}

fmt.Println(instances)

The big problem is that it's very easy to get trap with the coercion issue and struggling to understand why you're getting an error when actually, you aren't getting any, and makes the library harder to use. A correct usage of the library for the previous example is:

func myInstances() ([]*ec2.DescribeInstancesOutput, error) {
    // Do whatever initializations or other custom operations here
    // in order to get the AWS Reader implementation instance, which
    // below is set to the variable `c`

    var instances, err = c.GetInstances(nil)
    if err != nil {
        return nil, err
    }

    return instances, nil
 }

// In other place where the call to such function is done
var instances, err = myInstances()
if err != nil {
  os.Exit(1)
}

fmt.Println(instances)

This playground example show the actual case https://play.golang.org/p/vHN0Oxjylmd

If it looks good to you for making the chance, I could send a PR to make the changes, although those changes will be breaking changes, some consumers may break if they have been using the specific returned error type.

[ifraixedes]

What do you think about this, do I make the change?

[xlr-8]

That is fine with me, the original idea, was to provide a wrapper as simple as possible regarding the calls of AWS, as it is a pain to change calls between services. So that was to allow it on top of running multiple instances. Now I'm not opposed to it, as long as we don't lose information in the process.

[ifraixedes]

The goal isn't to remove the specific error types, otherwise we'd lose information (as you commented) which is quite valuable as the error can happen on the request to one region, but not on others; hence basically is to change the signature of the functions to return the standard error for avoiding such gotcha and it's up to the caller to do a type assertion when needed.

I'm going to work on this and see if we can improve the the caller experience rather than leave to it the only possibility of doing a type assertion, but I cannot guarantee for now.