From 419c6a416ea14cdec49c5d48c76e42771d4ee803 Mon Sep 17 00:00:00 2001 From: Josh Potts <9337140+jlpdeveloper@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:26:03 -0500 Subject: [PATCH 1/2] Refactor error handling: replace `Is` methods with utility functions for `NotFoundError` and `ValidationError`, and add corresponding tests --- internal/errors.go | 21 ++++++----- internal/errors_test.go | 77 +++++++++++++++++++++++++++++++++++++++ internal/response_test.go | 29 --------------- 3 files changed, 88 insertions(+), 39 deletions(-) create mode 100644 internal/errors_test.go diff --git a/internal/errors.go b/internal/errors.go index 79cebaa..39352a5 100644 --- a/internal/errors.go +++ b/internal/errors.go @@ -1,6 +1,7 @@ package internal import ( + "errors" "strconv" ) @@ -13,11 +14,6 @@ func (e NotFoundError) Error() string { return e.itemType + " not found with ID: " + strconv.Itoa(e.id) } -func (e NotFoundError) Is(target error) bool { - _, ok := target.(NotFoundError) - return ok -} - type ValidationError struct { msg string } @@ -26,11 +22,6 @@ func (e ValidationError) Error() string { return e.msg } -func (e ValidationError) Is(target error) bool { - _, ok := target.(ValidationError) - return ok -} - func NewValidationErr(message string) ValidationError { return ValidationError{ msg: message, @@ -40,3 +31,13 @@ func NewValidationErr(message string) ValidationError { func NewNotFoundError(id int, itemType string) NotFoundError { return NotFoundError{id: id, itemType: itemType} } + +func IsNotFoundError(err error) bool { + _, ok := errors.AsType[NotFoundError](err) + return ok +} + +func IsValidationError(err error) bool { + _, ok := errors.AsType[ValidationError](err) + return ok +} diff --git a/internal/errors_test.go b/internal/errors_test.go new file mode 100644 index 0000000..058963b --- /dev/null +++ b/internal/errors_test.go @@ -0,0 +1,77 @@ +package internal + +import ( + "errors" + "fmt" + "testing" +) + +func TestIsNotFoundError(t *testing.T) { + t.Run("returns true for NotFoundError", func(t *testing.T) { + err := NewNotFoundError(1, "Product") + if !IsNotFoundError(err) { + t.Errorf("IsNotFoundError(err) = false; want true") + } + }) + + t.Run("returns true for wrapped NotFoundError", func(t *testing.T) { + err := fmt.Errorf("wrapped: %w", NewNotFoundError(1, "Product")) + if !IsNotFoundError(err) { + t.Errorf("IsNotFoundError(err) = false; want true") + } + }) + + t.Run("returns false for other errors", func(t *testing.T) { + err := errors.New("some other error") + if IsNotFoundError(err) { + t.Errorf("IsNotFoundError(err) = true; want false") + } + }) + + t.Run("returns false for nil error", func(t *testing.T) { + if IsNotFoundError(nil) { + t.Errorf("IsNotFoundError(nil) = true; want false") + } + }) +} + +func TestIsValidationError(t *testing.T) { + t.Run("returns true for ValidationError", func(t *testing.T) { + err := NewValidationErr("invalid input") + if !IsValidationError(err) { + t.Errorf("IsValidationError(err) = false; want true") + } + }) + + t.Run("returns true for wrapped ValidationError", func(t *testing.T) { + err := fmt.Errorf("wrapped: %w", NewValidationErr("invalid input")) + if !IsValidationError(err) { + t.Errorf("IsValidationError(err) = false; want true") + } + }) + + t.Run("returns false for other errors", func(t *testing.T) { + err := errors.New("some other error") + if IsValidationError(err) { + t.Errorf("IsValidationError(err) = true; want false") + } + }) + + t.Run("returns false for nil error", func(t *testing.T) { + if IsValidationError(nil) { + t.Errorf("IsValidationError(nil) = true; want false") + } + }) +} + +func TestNotFoundError(t *testing.T) { + err := NewNotFoundError(123, "Product") + + t.Run("Error message", func(t *testing.T) { + expected := "Product not found with ID: 123" + if err.Error() != expected { + t.Errorf("expected %q, got %q", expected, err.Error()) + } + }) + +} diff --git a/internal/response_test.go b/internal/response_test.go index 9ae280c..4189488 100644 --- a/internal/response_test.go +++ b/internal/response_test.go @@ -2,8 +2,6 @@ package internal import ( "encoding/json" - "errors" - "fmt" "net/http" "net/http/httptest" "testing" @@ -91,30 +89,3 @@ func TestWriteJSONResponse(t *testing.T) { }) } } - -func TestNotFoundError(t *testing.T) { - err := NewNotFoundError(123, "Product") - - t.Run("Error message", func(t *testing.T) { - expected := "Product not found with ID: 123" - if err.Error() != expected { - t.Errorf("expected %q, got %q", expected, err.Error()) - } - }) - - t.Run("Is method", func(t *testing.T) { - if !errors.Is(err, NotFoundError{}) { - t.Error("expected errors.Is(err, NotFoundError{}) to be true") - } - - otherErr := errors.New("other error") - if errors.Is(otherErr, NotFoundError{}) { - t.Error("expected errors.Is(otherErr, NotFoundError{}) to be false") - } - - wrappedErr := fmt.Errorf("wrapped: %w", err) - if !errors.Is(wrappedErr, NotFoundError{}) { - t.Error("expected errors.Is(wrappedErr, NotFoundError{}) to be true") - } - }) -} From 6997c2427b22647c37b35578c51e5fd731c3aba6 Mon Sep 17 00:00:00 2001 From: Josh Potts <9337140+jlpdeveloper@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:26:14 -0500 Subject: [PATCH 2/2] Replace `errors.Is` with `IsNotFoundError` utility function across handlers and tests for improved readability and consistency. Remove redundant `errors` imports. --- internal/capability/handler.go | 19 +++++++++---------- .../capability/service_capability_test.go | 14 +++++++------- internal/flow/handler.go | 18 +++++++++--------- internal/platform/handler.go | 10 ++++------ internal/platform/service_test.go | 10 +++++----- internal/product/handler.go | 7 +++---- internal/product/service_test.go | 6 +++--- 7 files changed, 40 insertions(+), 44 deletions(-) diff --git a/internal/capability/handler.go b/internal/capability/handler.go index c69e49a..406cb25 100644 --- a/internal/capability/handler.go +++ b/internal/capability/handler.go @@ -3,7 +3,6 @@ package capability import ( "context" "encoding/json" - "errors" "log/slog" "net/http" "products/internal" @@ -51,7 +50,7 @@ func (h *handler) GetCapability(w http.ResponseWriter, r *http.Request) { defer cancel() capability, err := h.service.GetCapability(ctxWithTimeout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Capability not found"}, http.StatusNotFound) return } @@ -72,7 +71,7 @@ func (h *handler) GetCapabilitiesByFlow(w http.ResponseWriter, r *http.Request) defer cancel() capabilities, err := h.service.GetCapabilitiesByFlow(ctxWithTimeout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Flow not found"}, http.StatusNotFound) return } @@ -93,7 +92,7 @@ func (h *handler) GetCapabilitiesByProduct(w http.ResponseWriter, r *http.Reques defer cancel() capabilities, err := h.service.GetCapabilitiesByProduct(ctxWithTimeout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Product not found"}, http.StatusNotFound) return } @@ -124,7 +123,7 @@ func (h *handler) UpdateCapability(w http.ResponseWriter, r *http.Request) { defer cancel() capability, err := h.service.UpdateCapability(ctxWithTimeout, *req) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Capability not found"}, http.StatusNotFound) return } @@ -145,7 +144,7 @@ func (h *handler) DeleteCapability(w http.ResponseWriter, r *http.Request) { defer cancel() err := h.service.DeleteCapability(ctxWithTimout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Capability not found"}, http.StatusNotFound) return } @@ -171,10 +170,10 @@ func (h *handler) CreateCapabilityStep(w http.ResponseWriter, r *http.Request) { defer cancel() capStep, err := h.service.CreateCapabilityStep(ctxWithTimeout, *req) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error()}, http.StatusNotFound) return - } else if errors.Is(err, internal.ValidationError{}) { + } else if internal.IsValidationError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error()}, http.StatusBadRequest) return } @@ -195,7 +194,7 @@ func (h *handler) DeleteCapabilityStep(w http.ResponseWriter, r *http.Request) { defer cancel() err := h.service.DeleteCapabilityStep(ctxWithTimeout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Capability step not found", Instance: strconv.Itoa(id)}, http.StatusNotFound) return } @@ -216,7 +215,7 @@ func (h *handler) GetCapabilitySteps(w http.ResponseWriter, r *http.Request) { defer cancel() steps, err := h.service.GetCapabilitySteps(ctxWithTimeout, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Capability not found"}, http.StatusNotFound) return } diff --git a/internal/capability/service_capability_test.go b/internal/capability/service_capability_test.go index e84e131..532da3e 100644 --- a/internal/capability/service_capability_test.go +++ b/internal/capability/service_capability_test.go @@ -151,7 +151,7 @@ func TestService_CreateCapability(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -169,7 +169,7 @@ func TestService_CreateCapability(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -269,7 +269,7 @@ func TestService_GetCapability(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -366,7 +366,7 @@ func TestService_GetCapabilitiesByProduct(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -476,7 +476,7 @@ func TestService_GetCapabilitiesByFlow(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -579,7 +579,7 @@ func TestService_UpdateCapability(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, @@ -675,7 +675,7 @@ func TestService_DeleteCapability(t *testing.T) { } }, checkError: func(t *testing.T, err error) { - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %T", err) } }, diff --git a/internal/flow/handler.go b/internal/flow/handler.go index 572df79..36d02f2 100644 --- a/internal/flow/handler.go +++ b/internal/flow/handler.go @@ -41,7 +41,7 @@ func (h *handler) CreateFlow(w http.ResponseWriter, r *http.Request) { defer cancel() flow, err := h.flowService.CreateFlow(contextWithTimeOut, *req, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error()}, http.StatusNotFound) return } @@ -61,7 +61,7 @@ func (h *handler) GetFlowById(w http.ResponseWriter, r *http.Request) { defer cancel() flow, err := h.flowService.GetFlowById(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -81,7 +81,7 @@ func (h *handler) GetFlowsByProduct(w http.ResponseWriter, r *http.Request) { defer cancel() flows, err := h.flowService.GetFlowsByProduct(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -114,7 +114,7 @@ func (h *handler) UpdateFlow(w http.ResponseWriter, r *http.Request) { flow, err := h.flowService.UpdateFlow(contextWithTimeOut, *req, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -137,7 +137,7 @@ func (h *handler) DeleteFlow(w http.ResponseWriter, r *http.Request) { err := h.flowService.DeleteFlow(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -179,7 +179,7 @@ func (h *handler) CreateFlowStep(w http.ResponseWriter, r *http.Request) { defer cancel() flowStep, err := h.flowService.CreateFlowStep(contextWithTimeOut, *req) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -214,7 +214,7 @@ func (h *handler) DeleteFlowStep(w http.ResponseWriter, r *http.Request) { defer cancel() err := h.flowService.DeleteFlowStep(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -238,7 +238,7 @@ func (h *handler) GetFlowSteps(w http.ResponseWriter, r *http.Request) { defer cancel() flowSteps, err := h.flowService.GetFlowSteps(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } @@ -258,7 +258,7 @@ func (h *handler) GetFlowPath(w http.ResponseWriter, r *http.Request) { defer cancel() flowPath, err := h.flowService.GetFlowPath(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: err.Error(), Instance: r.URL.Path}, http.StatusNotFound) return } diff --git a/internal/platform/handler.go b/internal/platform/handler.go index e79ff50..1b67e70 100644 --- a/internal/platform/handler.go +++ b/internal/platform/handler.go @@ -3,7 +3,6 @@ package platform import ( "context" "encoding/json" - "errors" "net/http" "products/internal" "products/internal/platform/db" @@ -67,7 +66,7 @@ func (h *handler) UpdatePlatform(w http.ResponseWriter, r *http.Request) { defer cancel() _, err := h.service.UpdatePlatform(contextWithTimeOut, req, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Platform not found"}, http.StatusNotFound) return } @@ -88,12 +87,11 @@ func (h *handler) DeletePlatform(w http.ResponseWriter, r *http.Request) { defer cancel() _, err := h.service.DeletePlatform(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Platform not found"}, http.StatusNotFound) return } - logger := internal.LoggerFromContext(r.Context()) - logger.Error("Failed to delete platform", "error", err, "platform_id", id) + internal.LoggerFromContext(r.Context()).Error("Failed to delete platform", "error", err, "platform_id", id) internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Internal server error"}, http.StatusInternalServerError) return } @@ -125,7 +123,7 @@ func (h *handler) GetPlatform(w http.ResponseWriter, r *http.Request) { defer cancel() platform, err := h.service.GetPlatform(contextWithTimeOut, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Platform not found"}, http.StatusNotFound) return } diff --git a/internal/platform/service_test.go b/internal/platform/service_test.go index 22f528d..a0e61bc 100644 --- a/internal/platform/service_test.go +++ b/internal/platform/service_test.go @@ -95,7 +95,7 @@ func TestPostgresService_GetPlatform(t *testing.T) { } s := postgresService{db: m} _, err := s.GetPlatform(ctx, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -162,7 +162,7 @@ func TestPostgresService_UpdatePlatform(t *testing.T) { } s := postgresService{db: m} _, err := s.UpdatePlatform(ctx, req, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -175,7 +175,7 @@ func TestPostgresService_UpdatePlatform(t *testing.T) { } s := postgresService{db: m} _, err := s.UpdatePlatform(ctx, req, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -208,7 +208,7 @@ func TestPostgresService_DeletePlatform(t *testing.T) { } s := postgresService{db: m} _, err := s.DeletePlatform(ctx, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -221,7 +221,7 @@ func TestPostgresService_DeletePlatform(t *testing.T) { } s := postgresService{db: m} _, err := s.DeletePlatform(ctx, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) diff --git a/internal/product/handler.go b/internal/product/handler.go index b97e9c7..5392321 100644 --- a/internal/product/handler.go +++ b/internal/product/handler.go @@ -3,7 +3,6 @@ package product import ( "context" "encoding/json" - "errors" "net/http" "products/internal" "products/internal/product/db" @@ -54,7 +53,7 @@ func (h *handler) DeleteProduct(w http.ResponseWriter, r *http.Request) { contextWithTimeOut, cancel := context.WithTimeout(r.Context(), 5*time.Second) defer cancel() if _, err := h.service.DeleteProduct(contextWithTimeOut, id); err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Product not found"}, http.StatusNotFound) return } @@ -87,7 +86,7 @@ func (h *handler) UpdateProduct(w http.ResponseWriter, r *http.Request) { defer cancel() if _, err := h.service.UpdateProduct(ctx, req, id); err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Product not found"}, http.StatusNotFound) return } @@ -133,7 +132,7 @@ func (h *handler) GetProductById(w http.ResponseWriter, r *http.Request) { product, err := h.service.GetProductById(ctx, id) if err != nil { - if errors.Is(err, internal.NotFoundError{}) { + if internal.IsNotFoundError(err) { internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Product not found"}, http.StatusNotFound) return } diff --git a/internal/product/service_test.go b/internal/product/service_test.go index 146cecb..4414202 100644 --- a/internal/product/service_test.go +++ b/internal/product/service_test.go @@ -135,7 +135,7 @@ func TestPostgresService_GetProductById(t *testing.T) { } s := postgresService{queries: m} _, err := s.GetProductById(ctx, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -183,7 +183,7 @@ func TestPostgresService_UpdateProduct(t *testing.T) { } s := postgresService{queries: m} _, err := s.UpdateProduct(ctx, req, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } }) @@ -230,7 +230,7 @@ func TestPostgresService_DeleteProduct(t *testing.T) { } s := postgresService{queries: m} _, err := s.DeleteProduct(ctx, 1) - if !errors.Is(err, internal.NotFoundError{}) { + if !internal.IsNotFoundError(err) { t.Errorf("expected NotFoundError, got %v", err) } })