From 68541dc5fd796db8cc774ef7b4c60157a7859bed Mon Sep 17 00:00:00 2001 From: jjfreund Date: Thu, 29 Aug 2024 22:49:58 -0700 Subject: [PATCH] Fixed error handling for bookmarks manager (#137) * Fixed error handling for bookmarks manager * add additional error message handling updates --------- Co-authored-by: Joe Freund Co-authored-by: Leland Garofalo --- cmd/sheltertech-go/integration_test.go | 37 ++++++++++++++++++ internal/bookmarks/manager.go | 52 +++++++++++++++++++++----- internal/common/types.go | 31 +++++++++++++++ internal/db/manager.go | 24 ++++-------- 4 files changed, 118 insertions(+), 26 deletions(-) create mode 100644 internal/common/types.go diff --git a/cmd/sheltertech-go/integration_test.go b/cmd/sheltertech-go/integration_test.go index a3e3fdcb..9198dcb4 100644 --- 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 index 95391c4d..b043484e 100644 --- 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 new file mode 100644 index 00000000..c73288f7 --- /dev/null +++ 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 index 78f97677..7fef20f1 100644 --- 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 {