Skip to content

fix: validate handler constructors #4741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
turip wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: validate handler constructors #4741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9fcf35b
fix: validate handler constructors
turip Jul 17, 2026
647bbee
fix: return precondition failed when credits are disabled
turip Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions api/v3/handlers/currencies/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package currencies

import (
"context"
"errors"
"fmt"

"github.com/openmeterio/openmeter/openmeter/currencies"
"github.com/openmeterio/openmeter/pkg/framework/transport/httptransport"
Expand All @@ -24,10 +26,21 @@ func New(
resolveNamespace func(ctx context.Context) (string, error),
currencyService currencies.Service,
options ...httptransport.HandlerOption,
) Handler {
) (Handler, error) {
var errs []error
if resolveNamespace == nil {
errs = append(errs, errors.New("namespace resolver is required"))
}
if currencyService == nil {
errs = append(errs, errors.New("currency service is required"))
}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid currency handler config: %w", err)
}

return &handler{
resolveNamespace: resolveNamespace,
options: options,
service: currencyService,
}
}, nil
}
93 changes: 93 additions & 0 deletions api/v3/handlers/customers/credits/disabled.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package customerscredits

import (
"context"
"net/http"

"github.com/openmeterio/openmeter/api/v3/apierrors"
"github.com/openmeterio/openmeter/pkg/framework/commonhttp"
"github.com/openmeterio/openmeter/pkg/framework/transport/httptransport"
)

const creditsDisabledPrecondition = "credits is not enabled"

type disabledHandler struct {
options []httptransport.HandlerOption
}

func NewDisabled(options ...httptransport.HandlerOption) Handler {
return disabledHandler{
options: options,
}
}

func (h disabledHandler) GetCustomerCreditBalance() GetCustomerCreditBalanceHandler {
return disabledHandlerWithArgs[GetCustomerCreditBalanceRequest, GetCustomerCreditBalanceResponse, GetCustomerCreditBalanceParams](
"get-customer-credit-balance",
h.options,
)
}

func (h disabledHandler) ListCreditGrants() ListCreditGrantsHandler {
return disabledHandlerWithArgs[ListCreditGrantsRequest, ListCreditGrantsResponse, ListCreditGrantsParams](
"list-credit-grants",
h.options,
)
}

func (h disabledHandler) CreateCreditGrant() CreateCreditGrantHandler {
return disabledHandlerWithArgs[CreateCreditGrantRequest, CreateCreditGrantResponse, CreateCreditGrantParams](
"create-credit-grant",
h.options,
)
}

func (h disabledHandler) GetCreditGrant() GetCreditGrantHandler {
return disabledHandlerWithArgs[GetCreditGrantRequest, GetCreditGrantResponse, GetCreditGrantParams](
"get-credit-grant",
h.options,
)
}

func (h disabledHandler) VoidCreditGrant() VoidCreditGrantHandler {
return disabledHandlerWithArgs[VoidCreditGrantRequest, VoidCreditGrantResponse, VoidCreditGrantParams](
"void-credit-grant",
h.options,
)
}

func (h disabledHandler) UpdateCreditGrantExternalSettlement() UpdateCreditGrantExternalSettlementHandler {
return disabledHandlerWithArgs[UpdateCreditGrantExternalSettlementRequest, UpdateCreditGrantExternalSettlementResponse, UpdateCreditGrantExternalSettlementParams](
"update-credit-grant-external-settlement",
h.options,
)
}

func (h disabledHandler) ListCreditTransactions() ListCreditTransactionsHandler {
return disabledHandlerWithArgs[ListCreditTransactionsRequest, ListCreditTransactionsResponse, ListCreditTransactionsParams](
"list-credit-transactions",
h.options,
)
}

func disabledHandlerWithArgs[Request, Response, Params any](
operationName string,
options []httptransport.HandlerOption,
) httptransport.HandlerWithArgs[Request, Response, Params] {
return httptransport.NewHandlerWithArgs(
func(ctx context.Context, _ *http.Request, _ Params) (Request, error) {
var request Request
return request, apierrors.NewPreconditionFailedError(ctx, creditsDisabledPrecondition)
},
func(context.Context, Request) (Response, error) {
var response Response
return response, nil
},
commonhttp.JSONResponseEncoder[Response],
httptransport.AppendOptions(
options,
httptransport.WithOperationName(operationName),
httptransport.WithErrorEncoder(apierrors.GenericErrorEncoder()),
)...,
)
}
14 changes: 7 additions & 7 deletions api/v3/server/routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ func (s *Server) DeletePlanAddon(w http.ResponseWriter, r *http.Request, planId
var unimplemented = api.Unimplemented{}

func (s *Server) GetCustomerCreditBalance(w http.ResponseWriter, r *http.Request, customerId api.ULID, params api.GetCustomerCreditBalanceParams) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil {
if s.customersCreditsHandler == nil {
unimplemented.GetCustomerCreditBalance(w, r, customerId, params)
return
}
Expand All @@ -453,7 +453,7 @@ func (s *Server) GetCustomerCreditBalance(w http.ResponseWriter, r *http.Request
}

func (s *Server) ListCreditGrants(w http.ResponseWriter, r *http.Request, customerId api.ULID, params api.ListCreditGrantsParams) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.CreditGrantService == nil {
if s.customersCreditsHandler == nil {
unimplemented.ListCreditGrants(w, r, customerId, params)
return
}
Expand All @@ -465,7 +465,7 @@ func (s *Server) ListCreditGrants(w http.ResponseWriter, r *http.Request, custom
}

func (s *Server) CreateCreditGrant(w http.ResponseWriter, r *http.Request, customerId api.ULID) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.CreditGrantService == nil {
if s.customersCreditsHandler == nil {
unimplemented.CreateCreditGrant(w, r, customerId)
return
}
Expand All @@ -476,7 +476,7 @@ func (s *Server) CreateCreditGrant(w http.ResponseWriter, r *http.Request, custo
}

func (s *Server) GetCreditGrant(w http.ResponseWriter, r *http.Request, customerId api.ULID, creditGrantId api.ULID) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.CreditGrantService == nil {
if s.customersCreditsHandler == nil {
unimplemented.GetCreditGrant(w, r, customerId, creditGrantId)
return
}
Expand All @@ -492,7 +492,7 @@ func (s *Server) CreateCreditAdjustment(w http.ResponseWriter, r *http.Request,
}

func (s *Server) VoidCreditGrant(w http.ResponseWriter, r *http.Request, customerId api.ULID, creditGrantId api.ULID) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.CreditGrantService == nil {
if s.customersCreditsHandler == nil {
unimplemented.VoidCreditGrant(w, r, customerId, creditGrantId)
return
}
Expand All @@ -504,7 +504,7 @@ func (s *Server) VoidCreditGrant(w http.ResponseWriter, r *http.Request, custome
}

func (s *Server) UpdateCreditGrantExternalSettlement(w http.ResponseWriter, r *http.Request, customerId api.ULID, creditGrantId api.ULID) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.CreditGrantService == nil {
if s.customersCreditsHandler == nil {
unimplemented.UpdateCreditGrantExternalSettlement(w, r, customerId, creditGrantId)
return
}
Expand All @@ -516,7 +516,7 @@ func (s *Server) UpdateCreditGrantExternalSettlement(w http.ResponseWriter, r *h
}

func (s *Server) ListCreditTransactions(w http.ResponseWriter, r *http.Request, customerId api.ULID, params api.ListCreditTransactionsParams) {
if !s.Credits.Enabled || s.customersCreditsHandler == nil || s.Ledger == nil {
if s.customersCreditsHandler == nil {
unimplemented.ListCreditTransactions(w, r, customerId, params)
return
}
Expand Down
35 changes: 19 additions & 16 deletions api/v3/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import (
"github.com/openmeterio/openmeter/openmeter/ingest"
"github.com/openmeterio/openmeter/openmeter/ledger"
"github.com/openmeterio/openmeter/openmeter/ledger/customerbalance"
ledgernoop "github.com/openmeterio/openmeter/openmeter/ledger/noop"
"github.com/openmeterio/openmeter/openmeter/llmcost"
"github.com/openmeterio/openmeter/openmeter/meter"
"github.com/openmeterio/openmeter/openmeter/meterevent"
Expand Down Expand Up @@ -308,21 +307,22 @@ func NewServer(config *Config) (*Server, error) {
eventsHandler := eventshandler.New(resolveNamespace, config.IngestService, config.MeterEventService, httptransport.WithErrorHandler(config.ErrorHandler))
customersHandler := customershandler.New(resolveNamespace, config.CustomerService, httptransport.WithErrorHandler(config.ErrorHandler))
customersBillingHandler := customersbillinghandler.New(resolveNamespace, config.BillingService, config.CustomerService, config.StripeService, httptransport.WithErrorHandler(config.ErrorHandler))
customerBalanceFacade := config.CustomerBalanceFacade
creditGrantService := config.CreditGrantService
ledgerService := config.Ledger
accountResolver := config.AccountResolver
if !config.Credits.Enabled {
customerBalanceFacade, err = customerbalance.NewFacade(customerbalance.NewNoopService())
if err != nil {
return nil, fmt.Errorf("create noop customer balance facade: %w", err)
}

creditGrantService = creditgrant.NewNoopService()
ledgerService = ledgernoop.Ledger{}
accountResolver = ledgernoop.AccountResolver{}
var customersCreditsHandler customerscreditshandler.Handler
if config.Credits.Enabled {
customersCreditsHandler = customerscreditshandler.New(
resolveNamespace,
config.CustomerService,
config.CustomerBalanceFacade,
config.CreditGrantService,
config.Ledger,
config.AccountResolver,
httptransport.WithErrorHandler(config.ErrorHandler),
)
} else {
customersCreditsHandler = customerscreditshandler.NewDisabled(
httptransport.WithErrorHandler(config.ErrorHandler),
)
}
customersCreditsHandler := customerscreditshandler.New(resolveNamespace, config.CustomerService, customerBalanceFacade, creditGrantService, ledgerService, accountResolver, httptransport.WithErrorHandler(config.ErrorHandler))
customersEntitlementHandler := customersentitlementhandler.New(resolveNamespace, config.CustomerService, config.EntitlementService, httptransport.WithErrorHandler(config.ErrorHandler))
metersHandler := metershandler.New(resolveNamespace, config.MeterService, config.StreamingConnector, config.CustomerService, httptransport.WithErrorHandler(config.ErrorHandler))
subscriptionsHandler := subscriptionshandler.New(resolveNamespace, config.CustomerService, config.PlanService, config.PlanSubscriptionService, config.SubscriptionService, httptransport.WithErrorHandler(config.ErrorHandler))
Expand All @@ -332,7 +332,10 @@ func NewServer(config *Config) (*Server, error) {
plansHandler := planshandler.New(resolveNamespace, config.PlanService, config.UnitConfig.Enabled, httptransport.WithErrorHandler(config.ErrorHandler))
planAddonsHandler := planaddonshandler.New(resolveNamespace, config.PlanService, config.PlanAddonService, httptransport.WithErrorHandler(config.ErrorHandler))
taxcodesHandler := taxcodeshandler.New(resolveNamespace, config.TaxCodeService, httptransport.WithErrorHandler(config.ErrorHandler))
currenciesHandler := currencieshandler.New(resolveNamespace, config.CurrencyService, httptransport.WithErrorHandler(config.ErrorHandler))
currenciesHandler, err := currencieshandler.New(resolveNamespace, config.CurrencyService, httptransport.WithErrorHandler(config.ErrorHandler))
if err != nil {
return nil, fmt.Errorf("currency handler: %w", err)
}

var chargesH chargeshandler.Handler
if config.ChargeService != nil {
Expand Down
16 changes: 14 additions & 2 deletions openmeter/app/custominvoicing/httpdriver/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package httpdriver
import (
"context"
"errors"
"fmt"
"net/http"

appcustominvoicing "github.com/openmeterio/openmeter/openmeter/app/custominvoicing"
Expand Down Expand Up @@ -43,10 +44,21 @@ func New(
service appcustominvoicing.SyncService,
namespaceDecoder namespacedriver.NamespaceDecoder,
options ...httptransport.HandlerOption,
) Handler {
) (Handler, error) {
var errs []error
if service == nil {
errs = append(errs, errors.New("app custom invoicing service is required"))
}
if namespaceDecoder == nil {
errs = append(errs, errors.New("namespace decoder is required"))
}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid app custom invoicing handler config: %w", err)
}

return &handler{
service: service,
namespaceDecoder: namespaceDecoder,
options: options,
}
}, nil
}
28 changes: 26 additions & 2 deletions openmeter/app/httpdriver/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package httpdriver
import (
"context"
"errors"
"fmt"
"log/slog"
"net/http"

Expand Down Expand Up @@ -68,13 +69,36 @@ func New(
customerService customer.Service,

options ...httptransport.HandlerOption,
) Handler {
) (Handler, error) {
var errs []error
if logger == nil {
errs = append(errs, errors.New("logger is required"))
}
if namespaceDecoder == nil {
errs = append(errs, errors.New("namespace decoder is required"))
}
if appService == nil {
errs = append(errs, errors.New("app service is required"))
}
if appStripeService == nil {
errs = append(errs, errors.New("app stripe service is required"))
}
if billingService == nil {
errs = append(errs, errors.New("billing service is required"))
}
if customerService == nil {
errs = append(errs, errors.New("customer service is required"))
Comment on lines +77 to +90

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make constructor validation consistently reject typed-nil interfaces.

Direct interface comparisons with nil still permit nil-backed handlers, while the subscription configuration already handles this case correctly.

  • openmeter/app/httpdriver/handler.go#L77-L90: use typed-nil-aware checks for each interface dependency.
  • openmeter/credit/driver/grant.go#L51-L62: reject typed-nil connector, repository, decoder, and service values.
  • openmeter/entitlement/driver/entitlement.go#L57-L67: reject typed-nil entitlement dependencies.
  • openmeter/entitlement/driver/v2/handler.go#L45-L55: reject typed-nil v2 entitlement dependencies.
  • openmeter/productcatalog/planaddon/httpdriver/driver.go#L50-L55: reject typed-nil decoder and service values.
📍 Affects 5 files
  • openmeter/app/httpdriver/handler.go#L77-L90 (this comment)
  • openmeter/credit/driver/grant.go#L51-L62
  • openmeter/entitlement/driver/entitlement.go#L57-L67
  • openmeter/entitlement/driver/v2/handler.go#L45-L55
  • openmeter/productcatalog/planaddon/httpdriver/driver.go#L50-L55

}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid app handler config: %w", err)
}

return &handler{
service: appService,
namespaceDecoder: namespaceDecoder,
stripeAppService: appStripeService,
billingService: billingService,
customerService: customerService,
options: options,
}
}, nil
}
22 changes: 20 additions & 2 deletions openmeter/app/stripe/httpdriver/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package httpdriver
import (
"context"
"errors"
"fmt"
"net/http"

appstripe "github.com/openmeterio/openmeter/openmeter/app/stripe"
Expand Down Expand Up @@ -55,12 +56,29 @@ func New(
billingService billing.Service,
customerService customer.Service,
options ...httptransport.HandlerOption,
) Handler {
) (Handler, error) {
var errs []error
if namespaceDecoder == nil {
errs = append(errs, errors.New("namespace decoder is required"))
}
if service == nil {
errs = append(errs, errors.New("app stripe service is required"))
}
if billingService == nil {
errs = append(errs, errors.New("billing service is required"))
}
if customerService == nil {
errs = append(errs, errors.New("customer service is required"))
}
if err := errors.Join(errs...); err != nil {
return nil, fmt.Errorf("invalid app stripe handler config: %w", err)
}

return &handler{
service: service,
billingService: billingService,
customerService: customerService,
namespaceDecoder: namespaceDecoder,
options: options,
}
}, nil
}
Loading
Loading