huma icon indicating copy to clipboard operation
huma copied to clipboard

Get rid of exported global variables and instead make them a part of the "huma.API" struct

Open leonklingele opened this issue 8 months ago • 2 comments
trafficstars

About to implement a (more-strict) golangci-lint config for this repo, I noticed there are quite a few global variables inside the project. Just to name some of the more relevant ones:

adapters/humago/humago.go:19:5: MultipartMaxMemory is a global variable (gochecknoglobals)
var MultipartMaxMemory int64 = 8 * 1024

defaults.go:16:5: DefaultJSONFormat is a global variable (gochecknoglobals)
var DefaultJSONFormat = Format{

error.go:251:5: NewErrorWithContext is a global variable (gochecknoglobals)
var NewErrorWithContext = func(_ Context, status int, msg string, errs ...error) StatusError {

huma.go:1977:5: GenerateOperationID is a global variable (gochecknoglobals)
var GenerateOperationID = func(method, path string, response any) string {

schema.go:29:5: DefaultArrayNullable is a global variable (gochecknoglobals)
var DefaultArrayNullable = true

While it is definitely nice and helpful to provide a way to configure these (default) settings, they all depend on a single, shared global state.

In order to help mitigate problems which arise when using multiple Huma apps (huma.API contexts) in the same project, race conditions, etc., those settings should instead be a part of the huma.API context itself.

For example, I could imagine configuring the default Huma error handler (currently the global variable huma.NewErrorWithContext) as follows:

humago.New(
	mux,
	huma.DefaultConfig("API", "1.0.0").
		WithErrorHandler(func(ctx huma.Context, status int, msg string, errs ...error) huma.StatusError {
			...
		}),
)
diff --git a/api.go b/api.go
index e99bc43948bc7859a06b8ecf9dd5cd83300c65b9..e2c86418d2058cd3edcce88131c359a614a66ff0 100644
--- a/api.go
+++ b/api.go
@@ -203,8 +203,12 @@ type Config struct {
     // for example if you need access to the path settings that may be changed
     // by the user after the defaults have been set.
     CreateHooks []func(Config) Config
+
+    ErrorHandler ErrorHandler
 }

+type ErrorHandler func(ctx Context, status int, msg string, errs ...error) StatusError
+
 // API represents a Huma API wrapping a specific router.
 type API interface {
     // Adapter returns the router adapter for this API, providing a generic
@@ -265,6 +269,9 @@ type api struct {
     formatKeys   []string
     transformers []Transformer
     middlewares  Middlewares
+
+    // NOTE: Ensure errorHandler is an unexported property
+    errorHandler ErrorHandler
 }

 func (a *api) Adapter() Adapter {
@@ -400,6 +407,11 @@ func NewAPI(config Config, a Adapter) API {
         newAPI.formatKeys = append(newAPI.formatKeys, k)
     }

+    if config.ErrorHandler == nil {
+        config.ErrorHandler = defaultErrorHandler
+    }
+    newAPI.errorHandler = config.ErrorHandler
+
     if config.OpenAPIPath != "" {
         var specJSON []byte
         a.Handle(&Operation{

leonklingele avatar Mar 09 '25 09:03 leonklingele

Yes I think this would be nice, however we can't make major changes like this without breaking everyone. Perhaps it's time to start a list of things that would be nice to have for a v3, and once we get enough on there it might make sense to work on it.

danielgtaylor avatar Mar 10 '25 17:03 danielgtaylor

Right! The config options however can still be added in v2, falling back to the global variables (which stay until v3). This would allow for multiple Huma APIs, even in v2. What do you think?

leonklingele avatar Mar 26 '25 16:03 leonklingele