Fixes to recent extension changes. (#568)

* Fixes to recent extension changes.

* Fixes issue where gin will continue calling the handler even if next() isn't called.

* Updated docs.

api/server/error_response.go

@@ -43,6 +43,12 @@ func HandleErrorResponse(ctx context.Context, w http.ResponseWriter, err error)
 		statuscode = http.StatusInternalServerError
 		err = ErrInternalServerError
 	}
+	WriteError(ctx, w, statuscode, err)
+}
+
+// WriteError easy way to do standard error response, but can set statuscode and error message easier than HandleErrorResponse
+func WriteError(ctx context.Context, w http.ResponseWriter, statuscode int, err error) {
+	log := common.Logger(ctx)
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	w.WriteHeader(statuscode)
 	err = json.NewEncoder(w).Encode(simpleError(err))

api/server/extensions.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"fmt"
 	"log"
 
 	"github.com/fnproject/fn/fnext"
@@ -10,15 +9,15 @@ import (
 // TODO: Move this into `github.com/fnproject/fn` package after main is moved out of root dir.
 var extensions = map[string]fnext.Extension{}
 
-// RegisterExtension registers the extension so it's available, but does not initialize it or anything
+// RegisterExtension registers the extension so it's available, but does not initialize it.
+// This is generally what third party extensions will use in their init() method.
 func RegisterExtension(ext fnext.Extension) {
 	extensions[ext.Name()] = ext
 }
 
-// AddExtensionByName This essentially just makes sure the extensions are ordered properly.
-// It could do some initialization if required too.
+// AddExtensionByName This essentially just makes sure the extensions are ordered properly and is
+// what the CLI uses for the `fn build-server` command. Probably not used by much else.
 func (s *Server) AddExtensionByName(name string) {
-	fmt.Printf("extensions: %+v\n", extensions)
 	e, ok := extensions[name]
 	if !ok {
 		log.Fatalf("Extension %v not registered.\n", name)
@@ -28,3 +27,10 @@ func (s *Server) AddExtensionByName(name string) {
 		log.Fatalf("Failed to add extension %v: %v\n", name, err)
 	}
 }
+
+// AddExtension both registers an extension and adds it. This is useful during extension development
+// or if you want to build a custom server without using `fn build-server`.
+func (s *Server) AddExtension(ext fnext.Extension) {
+	RegisterExtension(ext)
+	s.AddExtensionByName(ext.Name())
+}

api/server/middleware.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
 	"github.com/fnproject/fn/api/common"
@@ -37,12 +36,14 @@ func (c *middlewareController) FunctionCalled() bool {
 
 func (s *Server) apiMiddlewareWrapper() gin.HandlerFunc {
 	return func(c *gin.Context) {
+		// fmt.Println("api middleware")
 		s.runMiddleware(c, s.apiMiddlewares)
 	}
 }
 
 func (s *Server) rootMiddlewareWrapper() gin.HandlerFunc {
 	return func(c *gin.Context) {
+		// fmt.Println("ROOT MIDDLE")
 		s.runMiddleware(c, s.rootMiddlewares)
 	}
 }
@@ -50,7 +51,9 @@ func (s *Server) rootMiddlewareWrapper() gin.HandlerFunc {
 // This is basically a single gin middleware that runs a bunch of fn middleware.
 // The final handler will pass it back to gin for further processing.
 func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) {
+	// fmt.Println("runMiddleware")
 	if len(ms) == 0 {
+		// fmt.Println("ZERO MIDDLEWARE")
 		c.Next()
 		return
 	}
@@ -65,11 +68,11 @@ func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) {
 
 	ctx := context.WithValue(c.Request.Context(), fnext.MiddlewareControllerKey, s.newMiddlewareController(c))
 	last := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		fmt.Println("final function called")
+		// fmt.Println("final handler called")
 		// check for bypass
 		mctx := fnext.GetMiddlewareController(r.Context())
 		if mctx.FunctionCalled() {
-			fmt.Println("function already called, skipping")
+			// fmt.Println("func already called, skipping")
 			c.Abort()
 			return
 		}
@@ -77,6 +80,8 @@ func (s *Server) runMiddleware(c *gin.Context, ms []fnext.Middleware) {
 	})
 
 	chainAndServe(ms, c.Writer, c.Request.WithContext(ctx), last)
+
+	c.Abort() // we always abort here because the middleware decides to call next or not. If the `last` handler gets called, it will continue, otherwise we abort.
 }
 
 func (s *Server) newMiddlewareController(c *gin.Context) *middlewareController {
@@ -86,6 +91,14 @@ func (s *Server) newMiddlewareController(c *gin.Context) *middlewareController {
 	}
 }
 
+// TODO: I will remove this and other debug commented lines once I'm sure it's all right.
+func debugH(i int, m fnext.Middleware, h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// fmt.Println("handling", i, "m:", reflect.TypeOf(m), "h:", reflect.TypeOf(h))
+		h.ServeHTTP(w, r) // call original
+	})
+}
+
 // chainAndServe essentially makes a chain of middleware wrapped around each other, then calls ServerHTTP on the end result.
 // then each middleware also calls ServeHTTP within it
 func chainAndServe(ms []fnext.Middleware, w http.ResponseWriter, r *http.Request, last http.Handler) {
@@ -93,7 +106,7 @@ func chainAndServe(ms []fnext.Middleware, w http.ResponseWriter, r *http.Request
 	// These get chained in reverse order so they play out in the right order. Don't ask.
 	for i := len(ms) - 1; i >= 0; i-- {
 		m := ms[i]
-		h = m.Handle(h)
+		h = m.Handle(debugH(i, m, h))
 	}
 	h.ServeHTTP(w, r)
 }