From afc61e02f1db8fac5ecd0466ceaf6edd6296eda9 Mon Sep 17 00:00:00 2001 From: Ruidy Date: Mon, 17 Nov 2025 19:26:45 +0100 Subject: [PATCH] refactor: improve booking error handling and responses Refactor booking retrieval to return errors instead of nil values, enabling more robust error handling throughout the booking, payment, and PDF endpoints. Add custom HTTP error page rendering for not found and internal server errors. Update interfaces and tests to match new method signatures. This improves user feedback and code maintainability. --- internal/repository/booking/pg_store.go | 9 ++-- internal/server/handle_bookings.go | 41 ++++++++++++++++++- internal/server/handle_payments.go | 10 ++++- internal/server/handle_pdf.go | 5 ++- internal/server/server.go | 22 ++++++++++ internal/service/booking/service.go | 16 ++++++-- .../service/booking/stripe_payment_link.go | 2 +- internal/service/booking/stripe_sync_test.go | 2 +- 8 files changed, 94 insertions(+), 13 deletions(-) diff --git a/internal/repository/booking/pg_store.go b/internal/repository/booking/pg_store.go index c40f10b..9483440 100644 --- a/internal/repository/booking/pg_store.go +++ b/internal/repository/booking/pg_store.go @@ -81,10 +81,13 @@ func (ps *PgStore) CardTotal(from, to time.Time) (float64, error) { return total, nil } -func (ps *PgStore) Get(id int) *booking.Booking { +func (ps *PgStore) Get(id int) (*booking.Booking, error) { var b booking.Booking - ps.db.Preload("Items").Preload("Payments").First(&b, id) - return &b + res := ps.db.Preload("Items").Preload("Payments").First(&b, id) + if err := res.Error; err != nil { + return nil, err + } + return &b, nil } func (ps *PgStore) Create(b *booking.Booking) error { diff --git a/internal/server/handle_bookings.go b/internal/server/handle_bookings.go index 093d1e6..1120038 100644 --- a/internal/server/handle_bookings.go +++ b/internal/server/handle_bookings.go @@ -16,6 +16,7 @@ import ( "github.com/go-chi/chi/v5" u "github.com/rjNemo/underscore" + "github.com/rjNemo/rentease/assets" "github.com/rjNemo/rentease/internal/config" "github.com/rjNemo/rentease/internal/constant" "github.com/rjNemo/rentease/internal/service/booking" @@ -167,7 +168,11 @@ func handleBookingPage(bs *booking.Service, hc *config.Host) http.HandlerFunc { return } - b := bs.One(id) + b, err := bs.One(id) + if err != nil { + renderHTTPErrorPage(w, http.StatusNotFound) + return + } var eid string if b.ExternalID == nil { @@ -380,7 +385,10 @@ func handleCreateItem(bs *booking.Service, hc *config.Host) http.HandlerFunc { return } - b := bs.One(bid) + b, err := bs.One(bid) + if bookingLookupFailed(w, err) { + return + } itemName := r.FormValue("item") itm, ok := hc.Items[itemName] @@ -550,3 +558,32 @@ func handleBookingCancel(bs *booking.Service) http.HandlerFunc { } } } + +func bookingLookupFailed(w http.ResponseWriter, err error) bool { + if err == nil { + return false + } + + if errors.Is(err, booking.ErrBookingNotFound) { + renderHTTPErrorPage(w, http.StatusNotFound) + return true + } + + renderHTTPErrorPage(w, http.StatusInternalServerError) + return true +} + +func renderHTTPErrorPage(w http.ResponseWriter, status int) { + pagePath := fmt.Sprintf("assets/html/HTTP%d.html", status) + page, err := assets.Static.ReadFile(pagePath) + if err != nil { + http.Error(w, http.StatusText(status), status) + return + } + + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(status) + if _, err := w.Write(page); err != nil { + slog.Error("failed to write error page", slog.Any("path", pagePath), slog.Any("error", err)) + } +} diff --git a/internal/server/handle_payments.go b/internal/server/handle_payments.go index de7bc2a..7417fe7 100644 --- a/internal/server/handle_payments.go +++ b/internal/server/handle_payments.go @@ -34,14 +34,20 @@ func handleCreatePayment(bs *booking.Service, hc *config.Host) http.HandlerFunc } } - b := bs.One(id) + b, err := bs.One(id) + if bookingLookupFailed(w, err) { + return + } if _, err := bs.CreatePayment(b.ID, amount, r.FormValue("paymentMethod")); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - nb := bs.One(id) + nb, err := bs.One(id) + if bookingLookupFailed(w, err) { + return + } component := view.PaymentList( u.Map(nb.Payments, func(p booking.Payment) *view.PaymentViewModel { diff --git a/internal/server/handle_pdf.go b/internal/server/handle_pdf.go index 6334eb2..0d11dc9 100644 --- a/internal/server/handle_pdf.go +++ b/internal/server/handle_pdf.go @@ -22,7 +22,10 @@ func handlePdfCreateInvoice(bs *booking.Service, hc *config.Host) http.HandlerFu return } - b := bs.One(id) + b, err := bs.One(id) + if bookingLookupFailed(w, err) { + return + } filePath, err := bs.BuildInvoice(b, hc) if err != nil { diff --git a/internal/server/server.go b/internal/server/server.go index 78b7ace..745ff5f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -130,5 +130,27 @@ func NewRouter(filesystem embed.FS, debug bool, origins []string) (*chi.Mux, err fileServer := http.StripPrefix("/static/", http.FileServer(http.FS(assetsFS))) r.Handle("/static/*", fileServer) + notFoundHandler, err := newHTTPErrorHandler(filesystem, http.StatusNotFound) + if err != nil { + return nil, fmt.Errorf("failed to setup not found handler: %w", err) + } + r.NotFound(notFoundHandler) + return r, nil } + +func newHTTPErrorHandler(filesystem embed.FS, statusCode int) (http.HandlerFunc, error) { + filePath := fmt.Sprintf("assets/html/HTTP%d.html", statusCode) + page, err := fs.ReadFile(filesystem, filePath) + if err != nil { + return nil, fmt.Errorf("failed to read error page %s: %w", filePath, err) + } + + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(statusCode) + if _, err := w.Write(page); err != nil { + slog.Error("failed to write error page", slog.Any("error", err)) + } + }, nil +} diff --git a/internal/service/booking/service.go b/internal/service/booking/service.go index 94b900d..46ee888 100644 --- a/internal/service/booking/service.go +++ b/internal/service/booking/service.go @@ -2,9 +2,12 @@ package booking import ( "context" + "errors" "log/slog" "time" + "gorm.io/gorm" + "github.com/rjNemo/rentease/internal/config" stripeclient "github.com/rjNemo/rentease/internal/driver/stripe" ) @@ -14,7 +17,7 @@ type Store interface { Search(value string) []*Line List(from, to time.Time) ([]*Line, error) CardTotal(from, to time.Time) (float64, error) - Get(id int) *Booking + Get(id int) (*Booking, error) Create(b *Booking) error Update(b *Booking) error Cancel(id int) error @@ -95,8 +98,15 @@ func (bs Service) Create(From time.Time, To time.Time, Name, PhoneNumber, Email, return b } -func (bs Service) One(id int) *Booking { - return bs.store.Get(id) +func (bs Service) One(id int) (*Booking, error) { + b, err := bs.store.Get(id) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, ErrBookingNotFound + } + return nil, err + } + return b, nil } // Update updates an existing booking with new data diff --git a/internal/service/booking/stripe_payment_link.go b/internal/service/booking/stripe_payment_link.go index db3c87a..05a758c 100644 --- a/internal/service/booking/stripe_payment_link.go +++ b/internal/service/booking/stripe_payment_link.go @@ -22,7 +22,7 @@ func (bs Service) CreateStripePaymentLink(ctx context.Context, bookingID int) (s return "", ErrStripeClientNotConfigured } - b := bs.store.Get(bookingID) + b, _ := bs.store.Get(bookingID) if b == nil || b.ID == 0 { return "", ErrBookingNotFound } diff --git a/internal/service/booking/stripe_sync_test.go b/internal/service/booking/stripe_sync_test.go index df21295..dc270ad 100644 --- a/internal/service/booking/stripe_sync_test.go +++ b/internal/service/booking/stripe_sync_test.go @@ -52,7 +52,7 @@ func (m *mockStore) All() []*Line { return ni func (m *mockStore) Search(string) []*Line { return nil } func (m *mockStore) List(time.Time, time.Time) ([]*Line, error) { return nil, nil } func (m *mockStore) CardTotal(time.Time, time.Time) (float64, error) { return 0, nil } -func (m *mockStore) Get(int) *Booking { return nil } +func (m *mockStore) Get(int) (*Booking, error) { return nil, nil } func (m *mockStore) Create(*Booking) error { return nil } func (m *mockStore) Update(*Booking) error { return nil } func (m *mockStore) Cancel(int) error { return nil }