From edc19b86147c5f633d588489d9a36627a2f9faff Mon Sep 17 00:00:00 2001
From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com>
Date: Thu, 27 Jun 2024 13:49:58 +0200
Subject: [PATCH] refactor: unify error handling for entitlements (#1076)

---
 .../entitlement/httpdriver/entitlement.go     | 54 ++------------
 internal/entitlement/httpdriver/errors.go     | 70 ++++++++++++++++++
 internal/entitlement/httpdriver/metered.go    | 72 ++-----------------
 3 files changed, 78 insertions(+), 118 deletions(-)
 create mode 100644 internal/entitlement/httpdriver/errors.go

diff --git a/internal/entitlement/httpdriver/entitlement.go b/internal/entitlement/httpdriver/entitlement.go
index 31af0556e..fbe8d99b1 100644
--- a/internal/entitlement/httpdriver/entitlement.go
+++ b/internal/entitlement/httpdriver/entitlement.go
@@ -12,7 +12,6 @@ import (
 	meteredentitlement "github.com/openmeterio/openmeter/internal/entitlement/metered"
 	staticentitlement "github.com/openmeterio/openmeter/internal/entitlement/static"
 	"github.com/openmeterio/openmeter/internal/namespace/namespacedriver"
-	"github.com/openmeterio/openmeter/internal/productcatalog"
 	"github.com/openmeterio/openmeter/pkg/convert"
 	"github.com/openmeterio/openmeter/pkg/defaultx"
 	"github.com/openmeterio/openmeter/pkg/framework/commonhttp"
@@ -138,38 +137,7 @@ func (h *entitlementHandler) CreateEntitlement() CreateEntitlementHandler {
 		httptransport.AppendOptions(
 			h.options,
 			httptransport.WithOperationName("createEntitlement"),
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*productcatalog.FeatureNotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if err, ok := err.(*entitlement.AlreadyExistsError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusConflict,
-						err,
-						commonhttp.ExtendProblem("conflictingEntityId", err.EntitlementID),
-					).EncodeError(ctx, w)
-					return true
-				}
-				if err, ok := err.(*entitlement.InvalidValueError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusBadRequest,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -235,23 +203,7 @@ func (h *entitlementHandler) GetEntitlementValue() GetEntitlementValueHandler {
 		httptransport.AppendOptions(
 			h.options,
 			httptransport.WithOperationName("getEntitlementValue"),
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*productcatalog.FeatureNotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -299,6 +251,7 @@ func (h *entitlementHandler) GetEntitlementsOfSubjectHandler() GetEntitlementsOf
 		httptransport.AppendOptions(
 			h.options,
 			httptransport.WithOperationName("getEntitlementsOfSubject"),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -355,6 +308,7 @@ func (h *entitlementHandler) ListEntitlements() ListEntitlementsHandler {
 		httptransport.AppendOptions(
 			h.options,
 			httptransport.WithOperationName("listEntitlements"),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
diff --git a/internal/entitlement/httpdriver/errors.go b/internal/entitlement/httpdriver/errors.go
new file mode 100644
index 000000000..5d4bb4cb9
--- /dev/null
+++ b/internal/entitlement/httpdriver/errors.go
@@ -0,0 +1,70 @@
+package httpdriver
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/openmeterio/openmeter/internal/entitlement"
+	"github.com/openmeterio/openmeter/internal/productcatalog"
+	"github.com/openmeterio/openmeter/pkg/framework/commonhttp"
+	"github.com/openmeterio/openmeter/pkg/framework/transport/httptransport"
+	"github.com/openmeterio/openmeter/pkg/models"
+)
+
+func getErrorEncoder() httptransport.ErrorEncoder {
+	return func(ctx context.Context, err error, w http.ResponseWriter) bool {
+		// user errors
+		if _, ok := err.(*productcatalog.FeatureNotFoundError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusNotFound,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		if _, ok := err.(*entitlement.NotFoundError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusNotFound,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		if _, ok := err.(*models.GenericUserError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusBadRequest,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		if err, ok := err.(*entitlement.AlreadyExistsError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusConflict,
+				err,
+				commonhttp.ExtendProblem("conflictingEntityId", err.EntitlementID),
+			).EncodeError(ctx, w)
+			return true
+		}
+		if err, ok := err.(*entitlement.InvalidValueError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusBadRequest,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		if err, ok := err.(*entitlement.InvalidFeatureError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusBadRequest,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		// system errors (naming known errors for transparency)
+		if _, ok := err.(*entitlement.WrongTypeError); ok {
+			commonhttp.NewHTTPError(
+				http.StatusInternalServerError,
+				err,
+			).EncodeError(ctx, w)
+			return true
+		}
+		return false
+	}
+}
diff --git a/internal/entitlement/httpdriver/metered.go b/internal/entitlement/httpdriver/metered.go
index c523912f5..f9be7b2c8 100644
--- a/internal/entitlement/httpdriver/metered.go
+++ b/internal/entitlement/httpdriver/metered.go
@@ -126,23 +126,7 @@ func (h *meteredEntitlementHandler) CreateGrant() CreateGrantHandler {
 		commonhttp.JSONResponseEncoderWithStatus[api.EntitlementGrant](http.StatusCreated),
 		httptransport.AppendOptions(
 			h.options,
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*models.GenericUserError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusBadRequest,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -194,23 +178,7 @@ func (h *meteredEntitlementHandler) ListEntitlementGrants() ListEntitlementGrant
 		commonhttp.JSONResponseEncoder[[]api.EntitlementGrant],
 		httptransport.AppendOptions(
 			h.options,
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*models.GenericUserError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusBadRequest,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -264,23 +232,7 @@ func (h *meteredEntitlementHandler) ResetEntitlementUsage() ResetEntitlementUsag
 		commonhttp.EmptyResponseEncoder[interface{}](http.StatusNoContent),
 		httptransport.AppendOptions(
 			h.options,
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*models.GenericUserError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusBadRequest,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }
@@ -378,23 +330,7 @@ func (h *meteredEntitlementHandler) GetEntitlementBalanceHistory() GetEntitlemen
 		commonhttp.JSONResponseEncoder[api.WindowedBalanceHistory],
 		httptransport.AppendOptions(
 			h.options,
-			httptransport.WithErrorEncoder(func(ctx context.Context, err error, w http.ResponseWriter) bool {
-				if _, ok := err.(*entitlement.NotFoundError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusNotFound,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				if _, ok := err.(*models.GenericUserError); ok {
-					commonhttp.NewHTTPError(
-						http.StatusBadRequest,
-						err,
-					).EncodeError(ctx, w)
-					return true
-				}
-				return false
-			}),
+			httptransport.WithErrorEncoder(getErrorEncoder()),
 		)...,
 	)
 }