Fix datastore error for inexistent app (#493)

* fix datastore error inexistent app

* fix get route error handling

* fix API errors handling and tests

api/datastore/bolt/bolt.go

@@ -229,6 +229,8 @@ func (ds *BoltDatastore) GetApp(ctx context.Context, name string) (*models.App,
 				return err
 			}
 			res = app
+		} else {
+			return models.ErrAppsNotFound
 		}
 		return nil
 	})
@@ -239,15 +241,11 @@ func (ds *BoltDatastore) GetApp(ctx context.Context, name string) (*models.App,
 }
 
 func (ds *BoltDatastore) getRouteBucketForApp(tx *bolt.Tx, appName string) (*bolt.Bucket, error) {
-	var err error
 	// todo: should this be reversed?  Make a bucket for each app that contains sub buckets for routes, etc
 	bp := tx.Bucket(ds.routesBucket)
 	b := bp.Bucket([]byte(appName))
 	if b == nil {
-		b, err = bp.CreateBucket([]byte(appName))
+		return nil, models.ErrAppsNotFound
-		if err != nil {
-			return nil, err
-		}
 	}
 	return b, nil
 }
@@ -418,6 +416,10 @@ func (ds *BoltDatastore) GetRoute(ctx context.Context, appName, routePath string
 		}
 
 		v := b.Get([]byte(routePath))
+		if v == nil {
+			return models.ErrRoutesNotFound
+		}
+
 		if v != nil {
 			err = json.Unmarshal(v, &route)
 		}

api/datastore/bolt_test.go

@@ -67,7 +67,7 @@ func TestBolt(t *testing.T) {
 	app, err := ds.GetApp(ctx, testApp.Name)
 	if err != nil {
 		t.Log(buf.String())
-		t.Fatalf("Test GetApp: error: %s", err)
+		t.Fatalf("Test GetApp: unexpected error: %s", err)
 	}
 	if app.Name != testApp.Name {
 		t.Log(buf.String())
@@ -101,9 +101,9 @@ func TestBolt(t *testing.T) {
 		t.Fatalf("Test RemoveApp: error: %s", err)
 	}
 	app, err = ds.GetApp(ctx, testApp.Name)
-	if err != nil {
+	if err != models.ErrAppsNotFound {
 		t.Log(buf.String())
-		t.Fatalf("Test GetApp: error: %s", err)
+		t.Fatalf("Test GetApp: expected error to be `%v`, but it was `%v`", models.ErrAppsNotFound, err)
 	}
 	if app != nil {
 		t.Log(buf.String())
@@ -240,11 +240,7 @@ func TestBolt(t *testing.T) {
 	}
 
 	route, err = ds.GetRoute(ctx, testRoute.AppName, testRoute.Path)
-	if err != nil {
+	if err != models.ErrRoutesNotFound {
-		t.Log(buf.String())
-		t.Fatalf("Test GetRoute: error: %s", err)
-	}
-	if route != nil {
 		t.Log(buf.String())
 		t.Fatalf("Test RemoveApp: failed to remove the route")
 	}

api/server/apps_create.go

@@ -42,13 +42,8 @@ func (s *Server) handleAppCreate(c *gin.Context) {
 	}
 
 	app, err := s.Datastore.InsertApp(ctx, wapp.App)
-	if err == models.ErrAppsAlreadyExists {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrAppsCreate)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusConflict, simpleError(err))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrAppsCreate)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/apps_delete.go

@@ -37,13 +37,8 @@ func (s *Server) handleAppDelete(c *gin.Context) {
 	}
 
 	err = s.Datastore.RemoveApp(ctx, app.Name)
-	if err == models.ErrAppsNotFound {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrAppsRemoving)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusNotFound, simpleError(err))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrAppsRemoving)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/apps_get.go

@@ -6,24 +6,15 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api"
-	"github.com/iron-io/functions/api/models"
-	"github.com/iron-io/runner/common"
 )
 
 func (s *Server) handleAppGet(c *gin.Context) {
 	ctx := c.MustGet("ctx").(context.Context)
-	log := common.Logger(ctx)
 
 	appName := c.MustGet(api.AppName).(string)
 	app, err := s.Datastore.GetApp(ctx, appName)
-
+	if err != nil {
-	if err != nil && err != models.ErrAppsNotFound {
+		handleErrorResponse(c, err)
-		log.WithError(err).Error(models.ErrAppsGet)
-		c.JSON(http.StatusInternalServerError, simpleError(models.ErrAppsGet))
-		return
-	} else if app == nil {
-		log.WithError(err).Error(models.ErrAppsNotFound)
-		c.JSON(http.StatusNotFound, simpleError(models.ErrAppsNotFound))
 		return
 	}
 

api/server/apps_list.go

@@ -6,19 +6,16 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api/models"
-	"github.com/iron-io/runner/common"
 )
 
 func (s *Server) handleAppList(c *gin.Context) {
 	ctx := c.MustGet("ctx").(context.Context)
-	log := common.Logger(ctx)
 
 	filter := &models.AppFilter{}
 
 	apps, err := s.Datastore.GetApps(ctx, filter)
 	if err != nil {
-		log.WithError(err).Error(models.ErrAppsList)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/apps_update.go

@@ -45,13 +45,8 @@ func (s *Server) handleAppUpdate(c *gin.Context) {
 	}
 
 	app, err := s.Datastore.UpdateApp(ctx, wapp.App)
-	if err == models.ErrAppsNotFound {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrAppsUpdate)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusNotFound, simpleError(err))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrAppsUpdate)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/error_response.go

@@ -0,0 +1,35 @@
+package server
+
+import (
+	"context"
+	"errors"
+	"github.com/gin-gonic/gin"
+	"github.com/iron-io/functions/api/models"
+	"github.com/iron-io/runner/common"
+	"net/http"
+)
+
+var ErrInternalServerError = errors.New("Something unexpected happened on the server")
+
+func simpleError(err error) *models.Error {
+	return &models.Error{Error: &models.ErrorBody{Message: err.Error()}}
+}
+
+var errStatusCode = map[error]int{
+	models.ErrAppsNotFound:        http.StatusNotFound,
+	models.ErrAppsAlreadyExists:   http.StatusConflict,
+	models.ErrRoutesNotFound:      http.StatusNotFound,
+	models.ErrRoutesAlreadyExists: http.StatusConflict,
+}
+
+func handleErrorResponse(c *gin.Context, err error) {
+	ctx := c.MustGet("ctx").(context.Context)
+	log := common.Logger(ctx)
+	log.Error(err)
+
+	if code, ok := errStatusCode[err]; ok {
+		c.JSON(code, simpleError(err))
+	} else {
+		c.JSON(http.StatusInternalServerError, simpleError(err))
+	}
+}

api/server/routes_create.go

@@ -90,13 +90,8 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
 	}
 
 	route, err := s.Datastore.InsertRoute(ctx, wroute.Route)
-	if err == models.ErrRoutesAlreadyExists {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrRoutesCreate)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusConflict, simpleError(models.ErrRoutesAlreadyExists))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrRoutesCreate)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/routes_delete.go

@@ -7,25 +7,16 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api"
-	"github.com/iron-io/functions/api/models"
-	"github.com/iron-io/runner/common"
 )
 
 func (s *Server) handleRouteDelete(c *gin.Context) {
 	ctx := c.MustGet("ctx").(context.Context)
-	log := common.Logger(ctx)
 
 	appName := c.MustGet(api.AppName).(string)
 	routePath := path.Clean(c.MustGet(api.Path).(string))
 
 	if err := s.Datastore.RemoveRoute(ctx, appName, routePath); err != nil {
-		if err == models.ErrRoutesNotFound {
+		handleErrorResponse(c, err)
-			log.WithError(err).Debug(models.ErrRoutesRemoving)
-			c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound))
-		} else {
-			log.WithError(err).Error(models.ErrRoutesRemoving)
-			c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesRemoving))
-		}
 		return
 	}
 

api/server/routes_get.go

@@ -7,25 +7,17 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api"
-	"github.com/iron-io/functions/api/models"
-	"github.com/iron-io/runner/common"
 )
 
 func (s *Server) handleRouteGet(c *gin.Context) {
 	ctx := c.MustGet("ctx").(context.Context)
-	log := common.Logger(ctx)
 
 	appName := c.MustGet(api.AppName).(string)
 	routePath := path.Clean(c.MustGet(api.Path).(string))
 
 	route, err := s.Datastore.GetRoute(ctx, appName, routePath)
-	if err != nil && err != models.ErrRoutesNotFound {
+	if err != nil {
-		log.WithError(err).Error(models.ErrRoutesGet)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
-		return
-	} else if route == nil {
-		log.Debug(models.ErrRoutesNotFound)
-		c.JSON(http.StatusNotFound, simpleError(models.ErrRoutesNotFound))
 		return
 	}
 

api/server/routes_list.go

@@ -7,12 +7,10 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/iron-io/functions/api"
 	"github.com/iron-io/functions/api/models"
-	"github.com/iron-io/runner/common"
 )
 
 func (s *Server) handleRouteList(c *gin.Context) {
 	ctx := c.MustGet("ctx").(context.Context)
-	log := common.Logger(ctx)
 
 	filter := &models.RouteFilter{}
 
@@ -28,13 +26,8 @@ func (s *Server) handleRouteList(c *gin.Context) {
 		routes, err = s.Datastore.GetRoutes(ctx, filter)
 	}
 
-	if err == models.ErrAppsNotFound {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrRoutesGet)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusNotFound, simpleError(err))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrRoutesGet)
-		c.JSON(http.StatusInternalServerError, simpleError(ErrInternalServerError))
 		return
 	}
 

api/server/routes_update.go

@@ -52,13 +52,8 @@ func (s *Server) handleRouteUpdate(c *gin.Context) {
 	}
 
 	route, err := s.Datastore.UpdateRoute(ctx, wroute.Route)
-	if err == models.ErrRoutesNotFound {
+	if err != nil {
-		log.WithError(err).Debug(models.ErrRoutesUpdate)
+		handleErrorResponse(c, err)
-		c.JSON(http.StatusNotFound, simpleError(err))
-		return
-	} else if err != nil {
-		log.WithError(err).Error(models.ErrRoutesUpdate)
-		c.JSON(http.StatusInternalServerError, simpleError(models.ErrRoutesUpdate))
 		return
 	}
 

api/server/server.go

@@ -3,7 +3,6 @@ package server
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -253,12 +252,6 @@ func (s *Server) bindHandlers() {
 	engine.NoRoute(s.handleSpecial)
 }
 
-var ErrInternalServerError = errors.New("Something unexpected happened on the server")
-
-func simpleError(err error) *models.Error {
-	return &models.Error{&models.ErrorBody{Message: err.Error()}}
-}
-
 type appResponse struct {
 	Message string      `json:"message"`
 	App     *models.App `json:"app"`

api/server/server_test.go

@@ -132,7 +132,7 @@ func TestFullStack(t *testing.T) {
 		{"delete myroute2", "DELETE", "/v1/apps/myapp/routes/myroute2", ``, http.StatusOK, 0},
 		{"delete app (success)", "DELETE", "/v1/apps/myapp", ``, http.StatusOK, 0},
 		{"get deleted app", "GET", "/v1/apps/myapp", ``, http.StatusNotFound, 0},
-		{"get delete route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusInternalServerError, 0},
+		{"get deleteds route on deleted app", "GET", "/v1/apps/myapp/routes/myroute", ``, http.StatusNotFound, 0},
 	} {
 		_, rec := routerRequest(t, srv.Router, test.method, test.path, bytes.NewBuffer([]byte(test.body)))