Major refactoring for better error handling

middleware/middleware.go

@@ -10,8 +10,32 @@ type (
 
 	// Middleware is the middle layer which represents the traditional
 	// idea of middleware: it is passed the next HandlerFunc in the chain
-	// and returns the inner layer, which is the actual HandlerFunc.
+	// and returns the inner layer, which is the actual Handler.
-	Middleware func(http.HandlerFunc) http.HandlerFunc
+	Middleware func(HandlerFunc) HandlerFunc
+
+	// HandlerFunc is like http.HandlerFunc except it returns a status code
+	// and an error. It is the inner-most layer which serves individual
+	// requests. The status code is for the client's benefit, the error
+	// value is for the server's benefit. The status code will be sent to
+	// the client while the error value will be logged privately. Sometimes,
+	// an error status code (4xx or 5xx) may be returned with a nil error
+	// when there is no reason to log the error on the server.
+	//
+	// If a HandlerFunc returns an error (status < 400), it should not write
+	// to the response. This philosophy is what makes middleware.HandlerFunc
+	// different from http.HandlerFunc. The error handling should happen
+	// at the application layer or in a dedicated error-handling middleware
+	// rather than an "every middleware for itself" paradigm. The error handling
+	// logic should make sure that the client is properly responded to according
+	// to the status code, but should probably not reveal the error message. The
+	// error message should be logged instead, for example.
+	HandlerFunc func(http.ResponseWriter, *http.Request) (int, error)
+
+	// Handler is like http.Handler except ServeHTTP returns a status code
+	// and an error. See HandlerFunc documentation for more information.
+	Handler interface {
+		ServeHTTP(http.ResponseWriter, *http.Request) (int, error)
+	}
 
 	// A Control provides structured access to tokens from a configuration file
 	// and also to properties of the server being configured. Middleware generators

server/server.go

@@ -26,10 +26,8 @@ var servers = make(map[string]*Server)
 // static content at a particular address (host and port).
 type Server struct {
 	config     config.Config
-	reqlog     *log.Logger
+	fileServer middleware.Handler
-	errlog     *log.Logger
+	stack      middleware.HandlerFunc
-	fileServer http.Handler
-	stack      http.HandlerFunc
 }
 
 // New creates a new Server and registers it with the list
@@ -60,12 +58,21 @@ func New(conf config.Config) (*Server, error) {
 
 // Serve starts the server. It blocks until the server quits.
 func (s *Server) Serve() error {
+	// Execute startup functions
+	for _, start := range s.config.Startup {
+		err := start()
+		if err != nil {
+			return err
+		}
+	}
+
+	// Build middleware stack
 	err := s.buildStack()
 	if err != nil {
 		return err
 	}
 
-	// use highest value across all configurations
+	// Use highest procs value across all configurations
 	if s.config.MaxCPU > 0 && s.config.MaxCPU > runtime.GOMAXPROCS(0) {
 		runtime.GOMAXPROCS(s.config.MaxCPU)
 	}
@@ -75,7 +82,8 @@ func (s *Server) Serve() error {
 		Handler: s,
 	}
 
-	http2.ConfigureServer(server, nil) // TODO: This may not be necessary after HTTP/2 merged into std lib
+	// TODO: This call may not be necessary after HTTP/2 is merged into std lib
+	http2.ConfigureServer(server, nil)
 
 	// Execute shutdown commands on exit
 	go func() {
@@ -98,40 +106,25 @@ func (s *Server) Serve() error {
 	}
 }
 
-// ServeHTTP is the entry point for each request to s.
+// ServeHTTP is the entry point for every request to s.
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	defer func() {
+		// In case the user doesn't enable error middleware, we still
+		// need to make sure that we stay alive up here
 		if rec := recover(); rec != nil {
-			s.Log("[PANIC] '%s': %s", r.URL.String(), rec)
+			http.Error(w, http.StatusText(http.StatusInternalServerError),
+				http.StatusInternalServerError)
 		}
 	}()
 	s.stack(w, r)
 }
 
-// Log writes a message to the server's configured error log,
-// if there is one, or if there isn't, to the default stderr log.
-func (s *Server) Log(v ...interface{}) {
-	if s.errlog != nil {
-		s.errlog.Println(v)
-	} else {
-		log.Println(v)
-	}
-}
-
 // buildStack builds the server's middleware stack based
 // on its config. This method should be called last before
 // ListenAndServe begins.
 func (s *Server) buildStack() error {
 	s.fileServer = FileServer(http.Dir(s.config.Root))
 
-	// Execute startup functions
-	for _, start := range s.config.Startup {
-		err := start()
-		if err != nil {
-			return err
-		}
-	}
-
 	// TODO: We only compile middleware for the "/" scope.
 	// Partial support for multiple location contexts already
 	// exists at the parser and config levels, but until full
@@ -141,11 +134,11 @@ func (s *Server) buildStack() error {
 	return nil
 }
 
-// compile is an elegant alternative to nesting middleware generator
+// compile is an elegant alternative to nesting middleware function
-// function calls like handler1(handler2(handler3(finalHandler))).
+// calls like handler1(handler2(handler3(finalHandler))).
 func (s *Server) compile(layers []middleware.Middleware) {
 	s.stack = s.fileServer.ServeHTTP // core app layer
-	for _, layer := range layers {
+	for i := len(layers) - 1; i >= 0; i-- {
-		s.stack = layer(s.stack)
+		s.stack = layers[i](s.stack)
 	}
 }