Fixed error handling for bookmarks manager (#137)

* Fixed error handling for bookmarks manager * add additional error message handling updates --------- Co-authored-by: Joe Freund <[email protected]> Co-authored-by: Leland Garofalo <[email protected]>
ShelterTechSF · Aug 30, 2024 · 68541dc · 68541dc
1 parent bde8177
commit 68541dc
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 26 deletions.
diff --git a/cmd/sheltertech-go/integration_test.go b/cmd/sheltertech-go/integration_test.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"github.com/sheltertechsf/sheltertech-go/internal/categories"
 	"github.com/sheltertechsf/sheltertech-go/internal/changerequest"
+	"github.com/sheltertechsf/sheltertech-go/internal/common"
 	"github.com/sheltertechsf/sheltertech-go/internal/services"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -165,6 +166,42 @@ func TestPostServicesChangeRequest(t *testing.T) {
 	assert.Equal(t, http.StatusCreated, res.StatusCode)
 }
 
+func TestGetBookmarksBadRequest(t *testing.T) {
+	startServer()
+
+	res, err := http.Get(bookmarkUrl + "?user_id=a")
+	require.NoError(t, err)
+	defer res.Body.Close()
+
+	body, err := ioutil.ReadAll(res.Body)
+	require.NoError(t, err)
+
+	serviceResponse := new(common.Error)
+	err = json.Unmarshal(body, serviceResponse)
+	require.NoError(t, err)
+
+	assert.Equal(t, serviceResponse.StatusCode, http.StatusBadRequest, "Response contains a 400")
+	assert.NotEmpty(t, serviceResponse.Error, "Response has some error message")
+}
+
+func TestGetBookmarkByIDError(t *testing.T) {
+	startServer()
+
+	res, err := http.Get(bookmarkUrl + "/0")
+	require.NoError(t, err)
+	defer res.Body.Close()
+
+	body, err := ioutil.ReadAll(res.Body)
+	require.NoError(t, err)
+
+	serviceResponse := new(common.Error)
+	err = json.Unmarshal(body, serviceResponse)
+	require.NoError(t, err)
+
+	assert.Equal(t, serviceResponse.StatusCode, http.StatusInternalServerError, "Response contains a 400")
+	assert.Equal(t, serviceResponse.Error, common.InternalServerErrorMessage)
+}
+
 func TestSwaggerDocs(t *testing.T) {
 	viper.SetDefault("SERVE_DOCS", "true")
 	startServer()

diff --git a/internal/bookmarks/manager.go b/internal/bookmarks/manager.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/sheltertechsf/sheltertech-go/internal/common"
 	"github.com/sheltertechsf/sheltertech-go/internal/db"
 
 	"github.com/go-chi/chi/v5"
@@ -30,10 +31,26 @@ func (m *Manager) Get(w http.ResponseWriter, r *http.Request) {
 	userId := r.URL.Query().Get("user_id")
 
 	if userId != "" {
-		iUserId, _ := strconv.Atoi(userId)
-		dbBookmarks = m.DbClient.GetBookmarksByUserID(iUserId)
+		iUserId, err := strconv.Atoi(userId)
+		if err != nil {
+			log.Printf("%v", err)
+			common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
+			return
+		}
+		dbBookmarks, err = m.DbClient.GetBookmarksByUserID(iUserId)
+		if err != nil {
+			log.Printf("%v", err)
+			common.WriteErrorJson(w, http.StatusInternalServerError, common.InternalServerErrorMessage)
+			return
+		}
 	} else {
-		dbBookmarks = m.DbClient.GetBookmarks()
+		var err error
+		dbBookmarks, err = m.DbClient.GetBookmarks()
+		if err != nil {
+			log.Printf("%v", err)
+			common.WriteErrorJson(w, http.StatusInternalServerError, common.InternalServerErrorMessage)
+			return
+		}
 	}
 	response := Bookmarks{
 		Bookmarks: FromDBTypeArray(dbBookmarks),
@@ -46,9 +63,16 @@ func (m *Manager) GetByID(w http.ResponseWriter, r *http.Request) {
 	id, err := strconv.Atoi(chi.URLParam(r, "id"))
 	if err != nil {
 		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
+		return
 	}
 
-	dbBookmark := m.DbClient.GetBookmarkByID(id)
+	dbBookmark, err := m.DbClient.GetBookmarkByID(id)
+	if err != nil {
+		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusInternalServerError, common.InternalServerErrorMessage)
+		return
+	}
 
 	response := FromDBType(dbBookmark)
 
@@ -63,7 +87,9 @@ func (m *Manager) Submit(w http.ResponseWriter, r *http.Request) {
 	bookmark := &Bookmark{}
 	err := json.Unmarshal(body, bookmark)
 	if err != nil {
-		writeStatus(w, http.StatusInternalServerError)
+		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
+		return
 	}
 
 	dbBookmark := &db.Bookmark{
@@ -77,11 +103,11 @@ func (m *Manager) Submit(w http.ResponseWriter, r *http.Request) {
 	err = m.DbClient.SubmitBookmark(dbBookmark)
 	if err != nil {
 		log.Print(err)
-		writeStatus(w, http.StatusInternalServerError)
+		common.WriteErrorJson(w, http.StatusInternalServerError, common.InternalServerErrorMessage)
+		return
 	}
 
 	writeStatus(w, http.StatusCreated)
-
 }
 
 func (m *Manager) Update(w http.ResponseWriter, r *http.Request) {
@@ -92,7 +118,9 @@ func (m *Manager) Update(w http.ResponseWriter, r *http.Request) {
 	bookmark := &Bookmark{}
 	err := json.Unmarshal(body, bookmark)
 	if err != nil {
-		writeStatus(w, http.StatusInternalServerError)
+		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
+		return
 	}
 
 	dbBookmark := &db.Bookmark{
@@ -106,8 +134,9 @@ func (m *Manager) Update(w http.ResponseWriter, r *http.Request) {
 
 	err = m.DbClient.UpdateBookmark(dbBookmark)
 	if err != nil {
-		log.Print(err)
-		writeStatus(w, http.StatusInternalServerError)
+		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusInternalServerError, common.InternalServerErrorMessage)
+		return
 	}
 
 	writeStatus(w, http.StatusCreated)
@@ -119,11 +148,14 @@ func (m *Manager) DeleteByID(w http.ResponseWriter, r *http.Request) {
 	bookmarkId, err := strconv.Atoi(chi.URLParam(r, "id"))
 	if err != nil {
 		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
+		return
 	}
 
 	err = m.DbClient.DeleteBookmarkByID(bookmarkId)
 	if err != nil {
 		log.Printf("%v", err)
+		common.WriteErrorJson(w, http.StatusBadRequest, err.Error())
 	}
 
 }

diff --git a/internal/common/types.go b/internal/common/types.go
@@ -0,0 +1,31 @@
+package common
+
+import (
+	"encoding/json"
+	"log"
+	"net/http"
+)
+
+type Error struct {
+	Error      string `json:"error"`
+	StatusCode int    `json:"status_code"`
+}
+
+var InternalServerErrorMessage = "Internal Server Error"
+
+func WriteErrorJson(w http.ResponseWriter, statusCode int, errorMsg string) {
+	object := Error{
+		Error:      errorMsg,
+		StatusCode: statusCode,
+	}
+	output, err := json.Marshal(object)
+	if err != nil {
+		log.Println("error:", err)
+	}
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(statusCode)
+	_, err = w.Write(output)
+	if err != nil {
+		panic(err)
+	}
+}
diff --git a/internal/db/manager.go b/internal/db/manager.go
@@ -183,27 +183,28 @@ func scanNotes(rows *sql.Rows) []*Note {
 	return notes
 }
 
-func (m *Manager) GetBookmarks() []*Bookmark {
+func (m *Manager) GetBookmarks() ([]*Bookmark, error) {
 	var rows *sql.Rows
 	var err error
 	rows, err = m.DB.Query(findBookmarksSql)
 	if err != nil {
 		log.Printf("%v\n", err)
 	}
-	return scanBookmarks(rows)
+	return scanBookmarks(rows), err
 }
 
-func (m *Manager) GetBookmarksByUserID(userId int) []*Bookmark {
+func (m *Manager) GetBookmarksByUserID(userId int) ([]*Bookmark, error) {
 	var rows *sql.Rows
 	var err error
 	rows, err = m.DB.Query(findBookmarksByUserIDSql, userId)
 	if err != nil {
 		log.Printf("%v\n", err)
+		return nil, err
 	}
-	return scanBookmarks(rows)
+	return scanBookmarks(rows), err
 }
 
-func (m *Manager) GetBookmarkByID(bookmarkId int) *Bookmark {
+func (m *Manager) GetBookmarkByID(bookmarkId int) (*Bookmark, error) {
 	row := m.DB.QueryRow(findBookmarksByIDSql, bookmarkId)
 	return scanBookmark(row)
 }
@@ -223,19 +224,10 @@ func scanBookmarks(rows *sql.Rows) []*Bookmark {
 	return bookmarks
 }
 
-func scanBookmark(row *sql.Row) *Bookmark {
+func scanBookmark(row *sql.Row) (*Bookmark, error) {
 	var bookmark Bookmark
 	err := row.Scan(&bookmark.Id, &bookmark.Order, &bookmark.UserID, &bookmark.FolderID, &bookmark.ServiceID, &bookmark.ResourceID)
-	if err != nil {
-		switch err {
-		case sql.ErrNoRows:
-			fmt.Println("No rows were returned!")
-			return nil
-		default:
-			panic(err)
-		}
-	}
-	return &bookmark
+	return &bookmark, err
 }
 
 func (m *Manager) GetAddressesByServiceID(serviceId int) []*Address {