From 9378f38371153c6f2bc2006afca8184c6eddb894 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 28 Mar 2015 16:37:37 -0600 Subject: [PATCH] Major refactoring for better error handling --- middleware/middleware.go | 28 +++++++++++++++++++-- server/server.go | 53 +++++++++++++++++----------------------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/middleware/middleware.go b/middleware/middleware.go index 2aceda136..ca570441b 100644 --- a/middleware/middleware.go +++ b/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. - Middleware func(http.HandlerFunc) http.HandlerFunc + // and returns the inner layer, which is the actual Handler. + 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 diff --git a/server/server.go b/server/server.go index 9a1fe894e..4bc594898 100644 --- a/server/server.go +++ b/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 - errlog *log.Logger - fileServer http.Handler - stack http.HandlerFunc + fileServer middleware.Handler + stack middleware.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 -// function calls like handler1(handler2(handler3(finalHandler))). +// compile is an elegant alternative to nesting middleware function +// 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 { - s.stack = layer(s.stack) + for i := len(layers) - 1; i >= 0; i-- { + s.stack = layers[i](s.stack) } }