Responding to PR comments, better error handling.

api/application.go

@@ -5,9 +5,12 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"github.com/18F/e-QIP-prototype/api/eqip"
 	"html/template"
 	"strconv"
+
+	"github.com/pkg/errors"
+
+	"github.com/18F/e-QIP-prototype/api/eqip"
 )
 
 // SectionInformation represents a structure to quickly organize the different
@@ -512,13 +515,17 @@ type FormStatusInfo struct {
 }
 
 // FormStatus returns the application metadata.
-func FormStatus(context DatabaseService, account int, locked bool) []byte {
+func FormStatus(context DatabaseService, account int, locked bool) ([]byte, error) {
+	hash, err := Hash(context, account)
+	if err != nil {
+		return nil, err
+	}
 	meta := &FormStatusInfo{
 		Locked: locked,
-		Hash:   Hash(context, account),
+		Hash:   hash,
 	}
 	js, _ := json.Marshal(meta)
-	return js
+	return js, nil
 }
 
 // Application returns the application state in JSON format.
@@ -619,13 +626,13 @@ func PurgeAccountStorage(context DatabaseService, account int) {
 }
 
 // Hash returns the SHA256 hash of the application state in hexadecimal
-func Hash(context DatabaseService, account int) string {
+func Hash(context DatabaseService, account int) (string, error) {
 	jsonBytes, err := Application(context, account, true)
 	if err != nil {
-		return ""
+		return "", errors.Wrap(err, "Unable to generate hash")
 	}
 	hash := sha256.Sum256(jsonBytes)
-	return hex.EncodeToString(hash[:])
+	return hex.EncodeToString(hash[:]), nil
 }
 
 // Catalogue eturns an array of the sub-sections of the form

api/database.go

@@ -1,5 +1,22 @@
 package api
 
+import (
+	"github.com/pkg/errors"
+)
+
+type DatabaseErrorNotFound string
+
+func (d DatabaseErrorNotFound) Error() string {
+	return string(d)
+}
+
+func IsDatabaseErrorNotFound(err error) bool {
+	if _, ok := errors.Cause(err).(DatabaseErrorNotFound); ok {
+		return true
+	}
+	return false
+}
+
 // DatabaseService represents a persisted data storage.
 type DatabaseService interface {
 	Configure()

api/http/hash.go

@@ -30,5 +30,12 @@ func (service HashHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	fmt.Fprint(w, api.Hash(service.Database, account.ID))
+	hash, err := api.Hash(service.Database, account.ID)
+	if err != nil {
+		service.Log.WarnError(api.HashingFailure, err, api.LogFields{})
+		RespondWithStructuredError(w, api.HashingFailure, http.StatusInternalServerError)
+		return
+	}
+
+	fmt.Fprint(w, hash)
 }

api/http/status.go

@@ -30,5 +30,12 @@ func (service StatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	fmt.Fprint(w, string(api.FormStatus(service.Database, account.ID, account.Locked)))
+	statusBytes, err := api.FormStatus(service.Database, account.ID, account.Locked)
+	if err != nil {
+		service.Log.WarnError(api.HashingFailure, err, api.LogFields{})
+		RespondWithStructuredError(w, api.HashingFailure, http.StatusInternalServerError)
+		return
+	}
+
+	fmt.Fprint(w, string(statusBytes))
 }

api/http/submit.go

@@ -68,7 +68,10 @@ func (service SubmitHandler) generatePdfs(account *api.Account) error {
 		return err
 	}
 
-	hash := api.Hash(service.Database, account.ID)
+	hash, err := api.Hash(service.Database, account.ID)
+	if err != nil {
+		return err
+	}
 
 	for _, p := range pdf.DocumentTypes {
 		signedOn, ok := service.Pdf.SignatureAvailable(application, p)

api/integration/form_version_test.go

@@ -10,9 +10,9 @@ import (
 	"testing"
 
 	"github.com/18F/e-QIP-prototype/api"
+	"github.com/18F/e-QIP-prototype/api/env"
 	"github.com/18F/e-QIP-prototype/api/http"
 	"github.com/18F/e-QIP-prototype/api/log"
-	"github.com/18F/e-QIP-prototype/api/mock"
 	"github.com/18F/e-QIP-prototype/api/postgresql"
 )
 
@@ -24,7 +24,7 @@ type serviceSet struct {
 }
 
 func cleanTestServices() serviceSet {
-	env := &mock.Native{}
+	env := &env.Native{}
 	os.Setenv(api.LogLevel, "info")
 	env.Configure()
 
@@ -58,7 +58,7 @@ func TestFormVersionReturned(t *testing.T) {
 
 	_, err := account.Get(services.db, -1)
 	if err != nil {
-		if err.Error() == "pg: no rows in result set" {
+		if api.IsDatabaseErrorNotFound(err) {
 			_, err := account.Save(services.db, -1)
 			if err != nil {
 				t.Fatal(err)
@@ -121,6 +121,7 @@ func TestFormVersionReturned(t *testing.T) {
 }
 
 func TestFormVersionSave(t *testing.T) {
+	// The client cannot set the form version via the API, this test confirms it's an error.
 
 	services := cleanTestServices()
 
@@ -134,7 +135,7 @@ func TestFormVersionSave(t *testing.T) {
 
 	_, err := account.Get(services.db, -1)
 	if err != nil {
-		if err.Error() == "pg: no rows in result set" {
+		if api.IsDatabaseErrorNotFound(err) {
 			_, err := account.Save(services.db, -1)
 			if err != nil {
 				t.Fatal(err)

api/log.go

@@ -36,6 +36,7 @@ const (
 	EntityError                                = "Error getting entity data"
 	EntitySaveError                            = "Error getting entity data"
 	FormDecodingError                          = "Error serializing form"
+	HashingFailure                             = "Error calculating hash"
 	InvalidJWT                                 = "Invalid JSON web token"
 	JWTError                                   = "Failed to generate JSON web token"
 	JWTSecretNotSet                            = "JSON web token secret is not set"

api/postgresql/db.go

@@ -83,17 +83,17 @@ func (service *Service) Find(query interface{}, callback func(query interface{})
 
 // FindAll instances of a type of model.
 func (service *Service) FindAll(query interface{}) error {
-	return service.database.Model(query).Select()
+	return explicitNotFoundError(service.database.Model(query).Select())
 }
 
 // Where is a conditional selection based on the model in the data store.
 func (service *Service) Where(model interface{}, condition string, params ...interface{}) error {
-	return service.database.Model(model).Where(condition, params...).Select()
+	return explicitNotFoundError(service.database.Model(model).Where(condition, params...).Select())
 }
 
 // ColumnsWhere is a conditional selection based on the model in the data store only returning specific quoted columns.
 func (service *Service) ColumnsWhere(model interface{}, columns []string, condition string, params ...interface{}) error {
-	return service.database.Model(model).Column(columns...).Where(condition, params...).Select()
+	return explicitNotFoundError(service.database.Model(model).Column(columns...).Where(condition, params...).Select())
 }
 
 // Count return the number of rows found.
@@ -146,5 +146,12 @@ func (service *Service) Delete(query interface{}) error {
 
 // Select returns the model from the data store
 func (service *Service) Select(query interface{}) error {
-	return service.database.Select(query)
+	return explicitNotFoundError(service.database.Select(query))
+}
+
+func explicitNotFoundError(err error) error {
+	if err == pg.ErrNoRows {
+		return api.DatabaseErrorNotFound("NOT_FOUND")
+	}
+	return err
 }

api/submission.go

@@ -115,7 +115,11 @@ func (entity *Submission) Valid() (bool, error) {
 // Save will create or update the database.
 func (entity *Submission) Save(context DatabaseService, account int) (int, error) {
 	entity.ID = account
-	entity.Hash = Hash(context, account)
+	hash, err := Hash(context, account)
+	if err != nil {
+		return -1, err
+	}
+	entity.Hash = hash
 
 	if err := context.CheckTable(entity); err != nil {
 		return entity.ID, err

docker-compose.yml

@@ -10,8 +10,8 @@ services:
       - ./conf/postgres.conf:/srv/postgresql.conf
       - ./api/eapp.crt:/srv/server.crt
       - ./api/eapp.key:/srv/server.key
-    expose:
-      - '5432'
+    ports:
+      - '5432:5432'
     networks:
       - eapp