From fecf53fd208617f14353ec2e6b4141de42995d52 Mon Sep 17 00:00:00 2001 From: Tim Wundenberg Date: Thu, 8 May 2025 22:58:14 +0200 Subject: [PATCH] feat(account): #49 refactor error handling --- db/account.go | 4 +++ handler/account.go | 5 +--- handler/error.go | 13 ++++++-- .../middleware/cross_site_request_forgery.go | 4 --- handler/middleware/gzip.go | 8 +++-- handler/middleware/logger.go | 2 +- handler/root_and_404.go | 5 ++-- main.go | 3 +- service/account.go | 23 +++++++------- template/account/account.templ | 30 ------------------- 10 files changed, 39 insertions(+), 58 deletions(-) diff --git a/db/account.go b/db/account.go index ca2d980..95e90e9 100644 --- a/db/account.go +++ b/db/account.go @@ -1,6 +1,7 @@ package db import ( + "database/sql" "spend-sparrow/log" "spend-sparrow/types" @@ -100,6 +101,9 @@ func (db AccountSqlite) Get(userId uuid.UUID, id uuid.UUID) (*types.Account, err WHERE user_id = ? AND id = ?`, userId, id) if err != nil { + if err == sql.ErrNoRows { + return nil, ErrNotFound + } log.Error("account Get: %v", err) return nil, types.ErrInternal } diff --git a/handler/account.go b/handler/account.go index 9bbc76b..49d451e 100644 --- a/handler/account.go +++ b/handler/account.go @@ -2,15 +2,13 @@ package handler import ( "fmt" + "net/http" "spend-sparrow/handler/middleware" - "spend-sparrow/log" "spend-sparrow/service" t "spend-sparrow/template/account" "spend-sparrow/types" "spend-sparrow/utils" - "net/http" - "github.com/a-h/templ" "github.com/google/uuid" ) @@ -70,7 +68,6 @@ func (h AccountImpl) handleAccountItemComp() http.HandlerFunc { idStr := r.PathValue("id") if idStr == "new" { comp := t.EditAccount(nil) - log.Info("Component: %v", comp) h.r.Render(r, w, comp) return } diff --git a/handler/error.go b/handler/error.go index 5b75d13..6d8e043 100644 --- a/handler/error.go +++ b/handler/error.go @@ -3,9 +3,9 @@ package handler import ( "errors" "net/http" - "spend-sparrow/service" "spend-sparrow/utils" + "strings" ) func handleError(w http.ResponseWriter, r *http.Request, err error) { @@ -13,9 +13,18 @@ func handleError(w http.ResponseWriter, r *http.Request, err error) { utils.TriggerToastWithStatus(w, r, "error", "You are not autorized to perform this operation.", http.StatusUnauthorized) return } else if errors.Is(err, service.ErrBadRequest) { - utils.TriggerToastWithStatus(w, r, "error", err.Error(), http.StatusBadRequest) + utils.TriggerToastWithStatus(w, r, "error", extractErrorMessage(err), http.StatusBadRequest) return } utils.TriggerToastWithStatus(w, r, "error", "Internal Server Error", http.StatusInternalServerError) } + +func extractErrorMessage(err error) string { + errMsg := err.Error() + if errMsg == "" { + return "" + } + + return strings.SplitN(errMsg, ":", 2)[0] +} diff --git a/handler/middleware/cross_site_request_forgery.go b/handler/middleware/cross_site_request_forgery.go index 663a2fc..0f68067 100644 --- a/handler/middleware/cross_site_request_forgery.go +++ b/handler/middleware/cross_site_request_forgery.go @@ -37,10 +37,6 @@ func (rr *csrfResponseWriter) Write(data []byte) (int, error) { return rr.ResponseWriter.Write([]byte(dataStr)) } -func (rr *csrfResponseWriter) WriteHeader(statusCode int) { - rr.ResponseWriter.WriteHeader(statusCode) -} - func CrossSiteRequestForgery(auth service.Auth) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/handler/middleware/gzip.go b/handler/middleware/gzip.go index 2d5ea62..77f5e45 100644 --- a/handler/middleware/gzip.go +++ b/handler/middleware/gzip.go @@ -27,11 +27,13 @@ func Gzip(next http.Handler) http.Handler { w.Header().Set("Content-Encoding", "gzip") gz := gzip.NewWriter(w) - gzr := gzipResponseWriter{Writer: gz, ResponseWriter: w} - next.ServeHTTP(gzr, r) + wrapper := gzipResponseWriter{Writer: gz, ResponseWriter: w} + + next.ServeHTTP(wrapper, r) err := gz.Close() - if err != nil { + if err != nil && err != http.ErrBodyNotAllowed { + // if err != nil { log.Error("Gzip: could not close Writer: %v", err) } }) diff --git a/handler/middleware/logger.go b/handler/middleware/logger.go index 126d38e..f0ec9a0 100644 --- a/handler/middleware/logger.go +++ b/handler/middleware/logger.go @@ -27,8 +27,8 @@ type WrappedWriter struct { } func (w *WrappedWriter) WriteHeader(code int) { - w.ResponseWriter.WriteHeader(code) w.StatusCode = code + w.ResponseWriter.WriteHeader(code) } func Log(next http.Handler) http.Handler { diff --git a/handler/root_and_404.go b/handler/root_and_404.go index 569b927..1b94d1b 100644 --- a/handler/root_and_404.go +++ b/handler/root_and_404.go @@ -1,12 +1,13 @@ package handler import ( + "fmt" + "net/http" "spend-sparrow/handler/middleware" + "spend-sparrow/log" "spend-sparrow/service" "spend-sparrow/template" - "net/http" - "github.com/a-h/templ" ) diff --git a/main.go b/main.go index 4f271ed..3649a8c 100644 --- a/main.go +++ b/main.go @@ -134,8 +134,7 @@ func createHandler(d *sqlx.DB, serverSettings *types.Settings) http.Handler { middleware.CacheControl, middleware.CrossSiteRequestForgery(authService), middleware.Authenticate(authService), - middleware.Log, - // Gzip last, as it compresses the body middleware.Gzip, + middleware.Log, ) } diff --git a/service/account.go b/service/account.go index 969d470..48ac0cd 100644 --- a/service/account.go +++ b/service/account.go @@ -75,8 +75,8 @@ func (s AccountImpl) Add(user *types.User, name string) (*types.Account, error) return nil, types.ErrInternal } - savedAccount, _ := s.db.Get(user.Id, newId) - if savedAccount == nil { + savedAccount, err := s.db.Get(user.Id, newId) + if err != nil { log.Error("account %v not found after insert: %v", newId, err) return nil, types.ErrInternal } @@ -94,11 +94,11 @@ func (s AccountImpl) Update(user *types.User, id uuid.UUID, name string) (*types account, err := s.db.Get(user.Id, id) if err != nil { + if err == db.ErrNotFound { + return nil, fmt.Errorf("account %v not found: %w", id, ErrBadRequest) + } return nil, types.ErrInternal } - if account == nil { - return nil, fmt.Errorf("account %v not found: %w", id, ErrBadRequest) - } timestamp := s.clock.Now() account.Name = name @@ -121,11 +121,11 @@ func (s AccountImpl) Get(user *types.User, id uuid.UUID) (*types.Account, error) account, err := s.db.Get(user.Id, id) if err != nil { + if err == db.ErrNotFound { + return nil, fmt.Errorf("account %v not found: %w", id, ErrBadRequest) + } return nil, types.ErrInternal } - if account == nil { - return nil, fmt.Errorf("account %v not found: %w", id, ErrBadRequest) - } return account, nil } @@ -151,6 +151,9 @@ func (s AccountImpl) Delete(user *types.User, id uuid.UUID) error { account, err := s.db.Get(user.Id, id) if err != nil { + if err == db.ErrNotFound { + return fmt.Errorf("account %v not found: %w", id, ErrBadRequest) + } return types.ErrInternal } @@ -168,9 +171,9 @@ func (s AccountImpl) Delete(user *types.User, id uuid.UUID) error { func (s AccountImpl) validateAccount(name string) error { if name == "" { - return fmt.Errorf("empty \"name\": %w", ErrBadRequest) + return fmt.Errorf("field \"name\" needs to be set: %w", ErrBadRequest) } else if !safeInputRegex.MatchString(name) { - return fmt.Errorf("only letters, dashes and numbers for \"name\": %w", ErrBadRequest) + return fmt.Errorf("use only letters, dashes and spaces for \"name\": %w", ErrBadRequest) } else { return nil } diff --git a/template/account/account.templ b/template/account/account.templ index 8050f9b..b0fb7b9 100644 --- a/template/account/account.templ +++ b/template/account/account.templ @@ -18,36 +18,6 @@ templ Account(accounts []*types.Account) {
for _, account := range accounts { @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) - @AccountItem(account) }