Use pointer receiver on pq.Error.Error()

[nhooyr]

The library returns *pq.Error and not pq.Error.

By using a value receiver, the library was documenting that consumers
should expect returned error values to contain pq.Error.

While *pq.Error implements all methods on pq.Error, *pq.Error is not
assignable to pq.Error and so you can't type assert an error value into
pq.Error if it actually contains *pq.Error.

In particular, this is a problem with errors.As. The following if
condition will always return false.

var pqe pq.Error
if errors.As(err, &pqe) {
  // Never reached as *pq.Error is not assignable to pqe.
  ...
}

[rafiss]

Would this break any users who have an if statement like this:

if pqErr := (*pq.Error)(nil); errors.As(err, &pqErr) { ... }

[nhooyr]

No that should work fine as this change only modifies the method receiver, not whether a pointer is returned.

The only thing this would break is if someone was creating pq.Error outside of this library and creating values not pointers. Their usage would break as pointer receivers implement the method for only the pointer type, not the value as well. I don't know why someone would be doing that but it is something to note.

[rafiss]

thanks for this change!

[dharmab]

Hello! In an unfortunate case of Hyrum's Law, this broke an internal package my team maintains. This is a shortened version of the function that broke:

func mapError(err error) error {
    if err == nil {
        return nil
    } else {
        switch v := err.(type) {
        case pq.Error:
            err = doThingWithErrPtr(&v)
        case *pq.Error:
            err = doThingWithErrPtr(v)
        // other cases for other errors
        }
    }
    // more code here that returns an error
}

Granted, this is a weird piece of code in a codebase with some technical debt and weird things in it. Just wanted to post this here in case anyone else experienced it. We downgraded to the previous version as a quickfix.

[nhooyr]

I'm confused, how could it have broken that, you were checking for both?

[dharmab]

Compilation fails with this error:

impossible type switch case: pq.Error
err (variable of type error) cannot have dynamic type pq.Error (Error method has pointer receiver)

See https://stackoverflow.com/a/24593723

[nhooyr]

Ah, just remove the case pq.Error: line. That's all you need to do.