Merge pull request #46 from calvinmclean/feature/32/validate-at-start

Add error handling for modifiers

babyapi.go

@@ -3,6 +3,7 @@ package babyapi
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log"
 	"log/slog"
 	"net/http"
@@ -79,6 +80,8 @@ type API[T Resource] struct {
 	rootAPI bool
 
 	readOnly sync.Mutex
+
+	errors []error
 }
 
 // NewAPI initializes an API using the provided name, base URL path, and function to create a new instance of
@@ -115,6 +118,7 @@ func NewAPI[T Resource](name, base string, instance func() T) *API[T] {
 		nil,
 		false,
 		sync.Mutex{},
+		nil,
 	}
 
 	api.GetAll = api.defaultGetAll()
@@ -250,7 +254,8 @@ func (a *API[T]) AddCustomRootRoute(route chi.Route) *API[T] {
 	a.panicIfReadOnly()
 
 	if a.parent != nil {
-		panic("cannot be applied to child APIs")
+		a.errors = append(a.errors, fmt.Errorf("AddCustomRootRoute: cannot be applied to child APIs"))
+		return a
 	}
 	a.rootRoutes = append(a.rootRoutes, route)
 	return a
@@ -270,7 +275,8 @@ func (a *API[T]) AddCustomIDRoute(route chi.Route) *API[T] {
 	a.panicIfReadOnly()
 
 	if a.rootAPI {
-		panic("ID routes cannot be used with a root API")
+		a.errors = append(a.errors, fmt.Errorf("AddCustomIDRoute: ID routes cannot be used with a root API"))
+		return a
 	}
 	a.customIDRoutes = append(a.customIDRoutes, route)
 	return a
@@ -289,19 +295,24 @@ func (a *API[T]) AddIDMiddleware(m func(http.Handler) http.Handler) *API[T] {
 	a.panicIfReadOnly()
 
 	if a.rootAPI {
-		panic("ID middleware cannot be used with a root API")
+		a.errors = append(a.errors, fmt.Errorf("AddIDMiddleware: ID middleware cannot be used with a root API"))
+		return a
 	}
 	a.idMiddlewares = append(a.idMiddlewares, m)
 	return a
 }
 
 // Serve will serve the API on the given port
-func (a *API[T]) Serve(address string) {
+func (a *API[T]) Serve(address string) error {
 	if address == "" {
 		address = ":8080"
 	}
 
-	a.server = &http.Server{Addr: address, Handler: a.Router()}
+	router, err := a.Router()
+	if err != nil {
+		return fmt.Errorf("error creating router: %w", err)
+	}
+	a.server = &http.Server{Addr: address, Handler: router}
 
 	var serverStopCtx context.CancelFunc
 	a.serverCtx, serverStopCtx = context.WithCancel(context.Background())
@@ -330,12 +341,14 @@ func (a *API[T]) Serve(address string) {
 	}()
 
 	slog.Info("starting server", "address", address, "api", a.name)
-	err := a.server.ListenAndServe()
+	err = a.server.ListenAndServe()
 	if err != nil && err != http.ErrServerClosed {
-		slog.Error("error starting the server", "error", err)
+		return fmt.Errorf("error starting the server: %w", err)
 	}
 
 	<-a.serverCtx.Done()
+
+	return nil
 }
 
 // Stop will stop the API

babyapi_test.go

@@ -75,7 +75,10 @@ func TestBabyAPI(t *testing.T) {
 
 	album1 := &Album{Title: "Album1"}
 
-	go api.Serve("localhost:8080")
+	go func() {
+		err := api.Serve("localhost:8080")
+		require.NoError(t, err)
+	}()
 	serverURL := "http://localhost:8080"
 
 	waitForAPI(serverURL)
@@ -980,19 +983,24 @@ func TestAPIModifierErrors(t *testing.T) {
 }
 
 func TestRootAPIWithMiddlewareAndCustomHandlers(t *testing.T) {
-	api := babyapi.NewRootAPI("root", "/")
-
-	t.Run("CustomizationsForIDsPanics", func(t *testing.T) {
-		require.Panics(t, func() {
-			api.AddCustomIDRoute(chi.Route{})
-		})
-		require.Panics(t, func() {
-			api.AddIDMiddleware(func(h http.Handler) http.Handler {
-				return nil
-			})
+	t.Run("CustomizationsForIDsCauseError", func(t *testing.T) {
+		api := babyapi.NewRootAPI("root", "/")
+		api.AddCustomIDRoute(chi.Route{})
+		api.AddIDMiddleware(func(h http.Handler) http.Handler {
+			return nil
 		})
+
+		err := api.RunWithArgs(os.Stdout, []string{"serve"}, "", "", false, nil, "")
+		require.Error(t, err)
+		require.ErrorAs(t, err, &babyapi.BuilderError{})
+		require.Equal(t, `error creating router: encountered 2 errors constructing API:
+- AddCustomIDRoute: ID routes cannot be used with a root API
+- AddIDMiddleware: ID middleware cannot be used with a root API
+`, err.Error())
 	})
 
+	api := babyapi.NewRootAPI("root", "/")
+
 	api.Get = func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(201)
 	}
@@ -1345,11 +1353,17 @@ func TestRootAPICLI(t *testing.T) {
 func TestReadOnlyPanicAfterStart(t *testing.T) {
 	api := babyapi.NewAPI[*Album]("Albums", "/albums", func() *Album { return &Album{} })
 
-	_ = api.Router()
+	_, err := api.Router()
+	require.NoError(t, err)
 
 	require.PanicsWithError(t, "API cannot be modified after starting", func() {
 		api.SetOnCreateOrUpdate(func(_ *http.Request, _ *Album) *babyapi.ErrResponse {
 			return nil
 		})
 	})
 }
+
+// func TestInvalidUseOfModifiersReturnsErrorAtStart(t *testing.T) {
+// 	api := babyapi.NewRootAPI[*Album]("Albums", "/albums", func() *Album { return &Album{} })
+// 	api
+// }

cli.go

@@ -55,8 +55,7 @@ func (a *API[T]) RunWithArgs(out io.Writer, args []string, bindAddress string, a
 	}
 
 	if args[0] == "serve" {
-		a.Serve(bindAddress)
-		return nil
+		return a.Serve(bindAddress)
 	}
 
 	return a.runClientCLI(out, args, address, pretty, headers, query)

extensions.go

@@ -14,9 +14,8 @@ func (a *API[T]) ApplyExtension(e Extension[T]) *API[T] {
 
 	err := e.Apply(a)
 	if err != nil {
-		// TODO: when #32 is implemented, extensions will be added to a list and applied in the function
-		// that finalizes the API and returns an error
-		panic(fmt.Sprintf("error applying extension: %v", err))
+		a.errors = append(a.errors, fmt.Errorf("ApplyExtension: error applying extension: %w", err))
+		return a
 	}
 	return a
 }

related_apis.go

@@ -10,8 +10,8 @@ import (
 // RelatedAPI declares a subset of methods from the API struct that are required to enable
 // nested/parent-child API relationships
 type RelatedAPI interface {
-	Router() chi.Router
-	Route(chi.Router)
+	Router() (chi.Router, error)
+	Route(chi.Router) error
 	Base() string
 	Name() string
 	GetIDParam(*http.Request) string
@@ -43,7 +43,8 @@ func (a *API[T]) AddNestedAPI(childAPI RelatedAPI) *API[T] {
 
 	relAPI, ok := childAPI.(relatedAPI)
 	if !ok {
-		panic(fmt.Sprintf("incompatible type for child API: %T", childAPI))
+		a.errors = append(a.errors, fmt.Errorf("AddNestedAPI: incompatible type for child API: %T", childAPI))
+		return a
 	}
 
 	a.subAPIs[childAPI.Name()] = relAPI

router.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 
 	"github.com/go-chi/chi/v5"
@@ -22,16 +23,38 @@ func defaultResponseCodes() map[string]int {
 	}
 }
 
+// BuilderError is used for combining errors that may occur when constructing a new API
+type BuilderError struct {
+	errors []error
+}
+
+func (e BuilderError) Error() string {
+	var sb strings.Builder
+	sb.WriteString(fmt.Sprintf("encountered %d errors constructing API:\n", len(e.errors)))
+
+	for _, err := range e.errors {
+		sb.WriteString(fmt.Sprintf("- %v\n", err))
+	}
+
+	return sb.String()
+}
+
+var _ error = BuilderError{}
+
 // HTMLer allows for easily represending reponses as HTML strings when accepted content
 // type is text/html
 type HTMLer interface {
 	HTML(*http.Request) string
 }
 
 // Create API routes on the given router
-func (a *API[T]) Route(r chi.Router) {
+func (a *API[T]) Route(r chi.Router) error {
 	a.readOnly.TryLock()
 
+	if len(a.errors) > 0 {
+		return BuilderError{a.errors}
+	}
+
 	respondOnce.Do(func() {
 		render.Respond = func(w http.ResponseWriter, r *http.Request, v interface{}) {
 			if render.GetAcceptedContentType(r) == render.ContentTypeHTML {
@@ -54,14 +77,15 @@ func (a *API[T]) Route(r chi.Router) {
 		a.doCustomRoutes(r, a.rootRoutes)
 	}
 
+	var returnErr error
 	r.Route(a.base, func(r chi.Router) {
 		// Only set these middleware for root-level API
 		if a.parent == nil {
 			a.DefaultMiddleware(r)
 		}
 
 		if a.rootAPI {
-			a.rootAPIRoutes(r)
+			returnErr = a.rootAPIRoutes(r)
 			return
 		}
 
@@ -79,38 +103,51 @@ func (a *API[T]) Route(r chi.Router) {
 			routeIfNotNil(r.With(a.requestBodyMiddleware).Patch, "/", a.Patch)
 
 			for _, subAPI := range a.subAPIs {
-				subAPI.Route(r)
+				err := subAPI.Route(r)
+				if err != nil {
+					returnErr = fmt.Errorf("error creating routes for %q: %w", subAPI.Name(), err)
+					return
+				}
 			}
 
 			a.doCustomRoutes(r, a.customIDRoutes)
 		})
+		if returnErr != nil {
+			return
+		}
 
 		a.doCustomRoutes(r, a.customRoutes)
 	})
+
+	return returnErr
 }
 
 // rootAPIRoutes creates different routes for a root API that doesn't deal with any resources
-func (a *API[T]) rootAPIRoutes(r chi.Router) {
+func (a *API[T]) rootAPIRoutes(r chi.Router) error {
 	routeIfNotNil(r.Post, "/", a.Post)
 	routeIfNotNil(r.Get, "/", a.Get)
 	routeIfNotNil(r.Delete, "/", a.Delete)
 	routeIfNotNil(r.Put, "/", a.Put)
 	routeIfNotNil(r.Patch, "/", a.Patch)
 
 	for _, subAPI := range a.subAPIs {
-		subAPI.Route(r)
+		err := subAPI.Route(r)
+		if err != nil {
+			return fmt.Errorf("error creating routes for %q: %w", subAPI.Name(), err)
+		}
 	}
 
 	a.doCustomRoutes(r, a.rootRoutes)
 	a.doCustomRoutes(r, a.customRoutes)
+
+	return nil
 }
 
 // Create a new router with API routes
-func (a *API[T]) Router() chi.Router {
+func (a *API[T]) Router() (chi.Router, error) {
 	r := chi.NewRouter()
-	a.Route(r)
-
-	return r
+	err := a.Route(r)
+	return r, err
 }
 
 func (a *API[T]) doCustomRoutes(r chi.Router, routes []chi.Route) {

test/testing_helpers.go

@@ -9,20 +9,26 @@ import (
 
 	"github.com/calvinmclean/babyapi"
 	"github.com/go-chi/chi/v5"
+	"github.com/stretchr/testify/require"
 )
 
 // Test is meant to be used in external tests to automatically handle setting up routes and using httptest
 func TestServe[T babyapi.Resource](t *testing.T, api *babyapi.API[T]) (string, func()) {
-	server := httptest.NewServer(api.Router())
+	router, err := api.Router()
+	require.NoError(t, err)
+	server := httptest.NewServer(router)
 	return server.URL, server.Close
 }
 
 // TestRequest is meant to be used in external tests to automatically handle setting up routes and using httptest
 func TestRequest[T babyapi.Resource](t *testing.T, api *babyapi.API[T], r *http.Request) *httptest.ResponseRecorder {
 	w := httptest.NewRecorder()
 
+	apiRouter, err := api.Router()
+	require.NoError(t, err)
+
 	router := chi.NewRouter()
-	router.Mount("/", api.Router())
+	router.Mount("/", apiRouter)
 	router.ServeHTTP(w, r)
 
 	return w
@@ -36,10 +42,13 @@ func TestWithParentRoute[T, P babyapi.Resource](t *testing.T, api *babyapi.API[T
 	parentAPI := babyapi.NewAPI[P](parentName, parentBasePath, func() P { return parent })
 	parentAPI.AddNestedAPI(api)
 
+	apiRouter, err := api.Router()
+	require.NoError(t, err)
+
 	router := chi.NewRouter()
 	api.DefaultMiddleware(router)
 	router.Route(fmt.Sprintf("%s/{%s}", parentBasePath, babyapi.IDParamKey(parentName)), func(r chi.Router) {
-		r.Mount("/", api.Router())
+		r.Mount("/", apiRouter)
 	})
 
 	router.ServeHTTP(w, r.WithContext(context.WithValue(context.Background(), babyapi.ContextKey(parentName), parent)))