Clients can't differentiate server-sent and client-synthesized errors

[akshayjshah]

Talked to @robbertvanginkel about this for a while today: in short, the problem is that clients can't tell whether an *Error was sent by the server, or whether it was synthesized within the client code (e.g., to wrap an underlying networking error). grpc-go has the same problem, and documents the resulting complexity thoroughly.

I don't think we can differentiate based on status code, because many codes are semantically appropriate for both clients and servers.

We could do something like this:

type synthesizedError struct {
  err error
}

func (e *synthesizedError) Unwrap() error { return e.err }
func (e *synthesizedError) Error() string { return e.err.Error() }

func IsFromServer(err error) bool {
  if se := new(synthesizedError); errors.As(err, &se) {
    return false
  }
  return true
}

and then be careful to always wrap client-created errors in synthesizedError. Introducing this change would be backward-compatible, so let's defer deciding until we've released Connect and have some more user experience reports.

Edit, months later: it's probably simpler and much less error-prone to do the opposite of what I suggested above, and specially flag errors that are actually read off the wire.

[jhump]

This is related to an issue I filed with grpc-go a long time ago: grpc/grpc-go#1150

At FullStory, we used a custom client interceptor (that was wired up in a custom Dial function that also included all of the other defaults needed to securely communicate with another backend service) to wrap all such errors received from a client so that we could tell if they were remote or not, very similar to the above code proposal. We used the term "incoming" error to distinguish it from an error that was constructed in the server, though perhaps "remote" or "synthesized" is more clear?

We also then used a custom server interceptor (configured in our grpc server scaffolding) to examine all errors returned from the handler code and not bother propagating remote errors. This was due to some confusing issues that can arise when remote errors are propagated by default (described in the above linked grpc-go issue). It is also a potential security issue to unconditionally retransmit/propagate remote errors as they could include sensitive data in error details. For cases where you do want the error to be propagated as is, this was easily achieved by calling a function that unwrapped (or, actually, re-wrapped) the error so that the server scaffolding would send it as is.

[akshayjshah]

@elliotmjackson If you're interested, this is a good issue to start with!