Merge pull request #4 from edio/feature/human_readable_errors

Make some erros more descriptive and human-friendly
ncbi · Jan 30, 2018 · 3a7846a · 3a7846a
2 parents 31d722f + 970c020
commit 3a7846a
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 7 deletions.
diff --git a/acctest/main_test.go b/acctest/main_test.go
@@ -83,7 +83,24 @@ func TestShouldFailIfServerIsNotListening(t *testing.T) {
 	// then
 	assert.Equal(t, 127, exitcode)
 	assert.Empty(t, stdout)
-	assert.Contains(t, stderr, "error")
+	assert.Contains(t, stderr, "connection refused")
+}
+
+func TestShouldFailIfServerDoesNotImplementHealthCheckProtocol(t *testing.T) {
+	// given
+	srv, err := StartEmptyServer(port)
+	if err != nil {
+		log.Fatalf("can't start stub server: %v", err)
+	}
+	defer srv.GracefulStop()
+
+	// when
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr)
+
+	// then
+	assert.Equal(t, 127, exitcode)
+	assert.Empty(t, stdout)
+	assert.Equal(t, stderr, "rpc error: server doesn't implement gRPC health-checking protocol\n")
 }
 
 func TestShouldReturnServingForHealthyService(t *testing.T) {
@@ -149,12 +166,12 @@ func TestShouldFailIfServiceHealthCheckIsNotRegistered(t *testing.T) {
 	defer srv.GracefulStop()
 
 	// when
-	stdout, stderr, exitcode := runBin(t, stubSrvAddr, "foo")
+	stdout, stderr, exitcode := runBin(t, stubSrvAddr, "my.service.Foo")
 
 	// then
 	assert.Equal(t, 127, exitcode)
 	assert.Empty(t, stdout)
-	assert.Contains(t, stderr, "NotFound")
+	assert.Equal(t, stderr, "rpc error: unknown service my.service.Foo\n")
 }
 
 // TLS tests
@@ -173,7 +190,7 @@ func TestShouldFailOnTlsVerificationWithSelfSignedCert(t *testing.T) {
 	// then
 	assert.Equal(t, 127, exitcode)
 	assert.Empty(t, stdout)
-	assert.Contains(t, stderr, "rpc error")
+	assert.Contains(t, stderr, "TLS handshake")
 }
 
 func TestShouldBeAbleToSkipTlsVerification(t *testing.T) {

diff --git a/acctest/stubserver.go b/acctest/stubserver.go
@@ -57,3 +57,15 @@ func doStart(port int, options ...grpc.ServerOption) (server *grpc.Server, servi
 	go server.Serve(listener)
 	return server, service, nil
 }
+
+// StartEmptyServer starts gRPC server application with no services
+func StartEmptyServer(port int) (server *grpc.Server, err error) {
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
+	if err != nil {
+		return
+	}
+	server = grpc.NewServer()
+
+	go server.Serve(listener)
+	return server, nil
+}
diff --git a/main.go b/main.go
@@ -27,8 +27,10 @@ import (
 	"github.com/hashicorp/go-rootcerts"
 	"github.com/urfave/cli"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	hv1 "google.golang.org/grpc/health/grpc_health_v1"
+	"google.golang.org/grpc/status"
 	"os"
 	"time"
 )
@@ -145,7 +147,7 @@ func createConfig(flags *appFlags, args cli.Args) (config *appConfig, err error)
 
 	creds, err := parseCredentials(flags)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("can't parse TLS configuration: %s", err.Error())
 	}
 
 	config.creds = creds
@@ -169,7 +171,7 @@ func parseCredentials(flags *appFlags) (credentials.TransportCredentials, error)
 		creds := credentials.NewTLS(tlsConfig)
 		return creds, nil
 	default:
-		err := fmt.Errorf("Ambigous TLS configuration. At most one of --tls, --tls-insecure, --tls-cafile and --tls-capath is allowed")
+		err := fmt.Errorf("at most one of --tls, --tls-insecure, --tls-cafile and --tls-capath is allowed")
 		return nil, err
 	}
 }
@@ -222,7 +224,8 @@ func appMain(config *appConfig) *cli.ExitError {
 
 	connection, err := connect(ctx, config.serverAddress, config.creds)
 	if err != nil {
-		return cli.NewExitError(err.Error(), ExitCodeUnexpected)
+		// actually should never happen because we use non-blocking dialer and failFast RPC (defaults)
+		return cli.NewExitError(fmt.Sprintf("can't connect to application: %s", err.Error()), ExitCodeUnexpected)
 	}
 	defer connection.Close()
 
@@ -256,8 +259,32 @@ func check(ctx context.Context, connection *grpc.ClientConn, service string) (st
 	response, err := client.Check(ctx, &hv1.HealthCheckRequest{
 		Service: service,
 	})
+
 	if response != nil {
 		status = response.Status
 	}
+
+	err = toHumanReadable(err, service)
+
 	return
 }
+
+func toHumanReadable(err error, service string) error {
+	code := status.Code(err)
+	switch code {
+	case codes.OK:
+		return err // err is nil
+	case codes.Unavailable:
+		return fmt.Errorf("connection refused: application isn't listening or TLS handshake failed")
+	case codes.Unimplemented:
+		return fmt.Errorf("rpc error: server doesn't implement gRPC health-checking protocol")
+	case codes.NotFound:
+		return fmt.Errorf("rpc error: unknown service %s", service)
+	default:
+		if s, isRPCError := status.FromError(err); isRPCError {
+			// display only message from generic rpc errors, hide code
+			return fmt.Errorf("rpc error: %s", s.Message())
+		}
+		return err
+	}
+}