Merge pull request #21 from Financial-Times/fix/empty-error-field-log…

…ging

Handling of empty error coming from neo4j driver

people/cypher.go

@@ -3,6 +3,8 @@ package people
 import (
 	"fmt"
 	"time"
+	"errors"
+	"strings"
 
 	"github.com/Financial-Times/neo-model-utils-go/mapper"
 	"github.com/Financial-Times/neo-utils-go/neoutils"
@@ -112,6 +114,8 @@ func (pcw CypherDriver) Read(uuid string, transactionID string) (Person, bool, e
 
 	err := pcw.conn.CypherBatch([]*neoism.CypherQuery{query})
 	if err != nil {
+		err = handleEmptyError(err, "unknown error during query execution")
+
 		log.WithError(err).WithFields(log.Fields{"UUID": uuid, "transaction_id": transactionID}).Error("Error Querying Neo4J for a Person")
 		return Person{}, true, err
 	}
@@ -120,7 +124,7 @@ func (pcw CypherDriver) Read(uuid string, transactionID string) (Person, bool, e
 		p, f, e := pcw.ReadOldConcordanceModel(uuid, transactionID)
 		return p, f, e
 	} else if len(results) != 1 {
-		err := fmt.Errorf("Multiple people found with the same uuid: %s", uuid)
+		err := fmt.Errorf("multiple people found with the same uuid: %s", uuid)
 		log.WithFields(log.Fields{"UUID": uuid, "transaction_id": transactionID}).Error(err.Error())
 		return Person{}, true, err
 	}
@@ -160,6 +164,7 @@ func (pcw CypherDriver) ReadOldConcordanceModel(uuid string, transactionID strin
 
 	err = pcw.conn.CypherBatch([]*neoism.CypherQuery{query})
 	if err != nil {
+		err = handleEmptyError(err, "unknown error during query execution")
 		log.WithError(err).WithFields(log.Fields{"UUID": uuid, "transaction_id": transactionID}).Error("Query execution failed")
 		return Person{}, false, err
 	} else if len(results) == 0 || len(results[0].Rs) == 0 {
@@ -275,3 +280,40 @@ func filterToMostSpecificType(unfilteredTypes []string) string {
 	fullURI := mapper.TypeURIs([]string{mostSpecificType})
 	return fullURI[0]
 }
+
+func handleEmptyError(e error, defaultMessage string) error {
+
+	if e.Error() != "" {
+		return e
+	}
+
+	neoError, ok := e.(neoism.NeoError)
+
+	if !ok {
+		return errors.New(defaultMessage)
+	}
+
+	if neoError.Exception != "" {
+		neoError.Message = neoError.Exception
+		return neoError
+	}
+
+	if neoError.Cause != nil {
+		cause := fmt.Sprint(neoError.Cause)
+
+		if cause != "" {
+			neoError.Message = "Cause: " + cause
+			return neoError
+		}
+	}
+
+	if len(neoError.Stacktrace) > 0 {
+		neoError.Message = strings.Join(neoError.Stacktrace, ", ")
+		return neoError
+	}
+
+	neoError.Message = defaultMessage
+
+	return neoError
+}
+

people/cypher_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/Financial-Times/roles-rw-neo4j/roles"
 	"github.com/jmcvetta/neoism"
 	"github.com/stretchr/testify/assert"
+	"errors"
 )
 
 const (
@@ -199,6 +200,50 @@ func TestNewModelWithThingOnlyMembershipRelatedConceptsDoesNotReturnMembership(t
 	readConceptAndCompare(t, person, "7ceeafe5-9f9a-4315-b3da-a5b4b69c013a")
 }
 
+func TestEmptyErrorHandlingWithException(t *testing.T) {
+	neoError := neoism.NeoError{
+		Cause:      "Underlying infra",
+		Stacktrace: []string{"something.exception at some.class\n", "\tterribly.broken.class.exception\n"},
+		Exception:  "something.exception at some.class",
+	}
+
+	err := handleEmptyError(neoError, "test-message")
+	assert.Equal(t, "something.exception at some.class", err.Error())
+}
+
+func TestEmptyErrorHandlingWithCause(t *testing.T) {
+	neoError := neoism.NeoError{
+		Cause:      "Underlying infra",
+		Stacktrace: []string{"something.exception at some.class\n", "\tterribly.broken.class.exception\n"},
+	}
+
+	err := handleEmptyError(neoError, "test-message")
+	assert.Equal(t, "Cause: Underlying infra", err.Error())
+}
+
+func TestEmptyErrorHandlingWithStacktrace(t *testing.T) {
+	neoError := neoism.NeoError{
+		Stacktrace: []string{"something.exception at some.class\n", "\tterribly.broken.class.exception\n"},
+	}
+
+	err := handleEmptyError(neoError, "test-message")
+	assert.Equal(t, "something.exception at some.class\n, \tterribly.broken.class.exception\n", err.Error())
+}
+
+func TestEmptyErrorHandlingWithDefaultMessage(t *testing.T) {
+	neoError := neoism.NeoError{}
+
+	err := handleEmptyError(neoError, "test-message")
+	assert.Equal(t, "test-message", err.Error())
+}
+
+func TestEmptyErrorHandlingWithGenericError(t *testing.T) {
+	err := errors.New("")
+
+	err = handleEmptyError(err, "test-message")
+	assert.Equal(t, "test-message", err.Error())
+}
+
 func readConceptAndCompare(t *testing.T, expected Person, uuid string) {
 	actual, found, err := publicPeopleDriver.Read(uuid, testTransactionID)