Use ErrorPage to render proxy error page

oauthproxy.go

@@ -122,8 +122,15 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 	if err != nil {
 		return nil, fmt.Errorf("error loading templates: %v", err)
 	}
-	proxyErrorHandler := upstream.NewProxyErrorHandler(templates.Lookup("error.html"), opts.ProxyPrefix)
+
-	upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), proxyErrorHandler)
+	errorPage := &app.ErrorPage{
+		Template:    templates.Lookup("error.html"),
+		ProxyPrefix: opts.ProxyPrefix,
+		Footer:      opts.Templates.Footer,
+		Version:     VERSION,
+	}
+
+	upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), errorPage.ProxyErrorHandler)
 	if err != nil {
 		return nil, fmt.Errorf("error initialising upstream proxy: %v", err)
 	}
@@ -225,12 +232,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		sessionChain:        sessionChain,
 		headersChain:        headersChain,
 		preAuthChain:        preAuthChain,
-		errorPage: &app.ErrorPage{
+		errorPage:           errorPage,
-			Template:    templates.Lookup("error.html"),
-			ProxyPrefix: opts.ProxyPrefix,
-			Footer:      opts.Templates.Footer,
-			Version:     VERSION,
-		},
 	}, nil
 }
 

pkg/app/error_page.go

@@ -54,3 +54,11 @@ func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL strin
 		http.Error(rw, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 	}
 }
+
+// ProxyErrorHandler is used by the upstream ReverseProxy to render error pages
+// when there are issues with upstream servers.
+// It is expected to always render a bad gateway error.
+func (e *ErrorPage) ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error) {
+	logger.Errorf("Error proxying to upstream server: %v", proxyErr)
+	e.Render(rw, http.StatusBadGateway, "", "Error proxying to upstream server")
+}

pkg/app/error_page_test.go

@@ -1,6 +1,7 @@
 package app
 
 import (
+	"errors"
 	"html/template"
 	"io/ioutil"
 	"net/http/httptest"
@@ -10,19 +11,22 @@ import (
 )
 
 var _ = Describe("Error Page", func() {
+	var errorPage *ErrorPage
+
+	BeforeEach(func() {
+		tmpl, err := template.New("").Parse("{{.Title}} {{.Message}} {{.ProxyPrefix}} {{.StatusCode}} {{.Redirect}} {{.Footer}} {{.Version}}")
+		Expect(err).ToNot(HaveOccurred())
+
+		errorPage = &ErrorPage{
+			Template:    tmpl,
+			ProxyPrefix: "/prefix/",
+			Footer:      "Custom Footer Text",
+			Version:     "v0.0.0-test",
+		}
+	})
 
 	Context("Render", func() {
 		It("Writes the template to the response writer", func() {
-			tmpl, err := template.New("").Parse("{{.Title}} {{.Message}} {{.ProxyPrefix}} {{.StatusCode}} {{.Redirect}} {{.Footer}} {{.Version}}")
-			Expect(err).ToNot(HaveOccurred())
-
-			errorPage := &ErrorPage{
-				Template:    tmpl,
-				ProxyPrefix: "/prefix/",
-				Footer:      "Custom Footer Text",
-				Version:     "v0.0.0-test",
-			}
-
 			recorder := httptest.NewRecorder()
 			errorPage.Render(recorder, 403, "/redirect", "Access Denied")
 
@@ -32,4 +36,15 @@ var _ = Describe("Error Page", func() {
 		})
 	})
 
+	Context("ProxyErrorHandler", func() {
+		It("Writes a bad gateway error the response writer", func() {
+			req := httptest.NewRequest("", "/bad-gateway", nil)
+			recorder := httptest.NewRecorder()
+			errorPage.ProxyErrorHandler(recorder, req, errors.New("some upstream error"))
+
+			body, err := ioutil.ReadAll(recorder.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(body)).To(Equal("Bad Gateway Error proxying to upstream server /prefix/ 502  Custom Footer Text v0.0.0-test"))
+		})
+	})
 })

pkg/app/templates.go

@@ -79,6 +79,7 @@ const (
   		</div>
       {{ end }}
 
+      {{ if .Redirect }}
       <hr>
 
       <div class="columns">
@@ -94,6 +95,7 @@ const (
           </form>
         </div>
       </div>
+      {{ end }}
 
     </div>
   </section>

pkg/upstream/proxy.go

@@ -2,7 +2,6 @@ package upstream
 
 import (
 	"fmt"
-	"html/template"
 	"net/http"
 	"net/url"
 
@@ -71,24 +70,3 @@ func (m *multiUpstreamProxy) registerHTTPUpstreamProxy(upstream options.Upstream
 	logger.Printf("mapping path %q => upstream %q", upstream.Path, upstream.URI)
 	m.serveMux.Handle(upstream.Path, newHTTPUpstreamProxy(upstream, u, sigData, errorHandler))
 }
-
-// NewProxyErrorHandler creates a ProxyErrorHandler using the template given.
-func NewProxyErrorHandler(errorTemplate *template.Template, proxyPrefix string) ProxyErrorHandler {
-	return func(rw http.ResponseWriter, req *http.Request, proxyErr error) {
-		logger.Errorf("Error proxying to upstream server: %v", proxyErr)
-		rw.WriteHeader(http.StatusBadGateway)
-		data := struct {
-			Title       string
-			Message     string
-			ProxyPrefix string
-		}{
-			Title:       "Bad Gateway",
-			Message:     "Error proxying to upstream server",
-			ProxyPrefix: proxyPrefix,
-		}
-		err := errorTemplate.Execute(rw, data)
-		if err != nil {
-			http.Error(rw, "Internal Server Error", http.StatusInternalServerError)
-		}
-	}
-}

pkg/upstream/proxy_test.go

@@ -4,7 +4,6 @@ import (
 	"crypto"
 	"encoding/json"
 	"fmt"
-	"html/template"
 	"net/http"
 	"net/http/httptest"
 
@@ -20,9 +19,10 @@ var _ = Describe("Proxy Suite", func() {
 	BeforeEach(func() {
 		sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
 
-		tmpl, err := template.New("").Parse("{{ .Title }}\n{{ .Message }}\n{{ .ProxyPrefix }}")
+		errorHandler := func(rw http.ResponseWriter, _ *http.Request, _ error) {
-		Expect(err).ToNot(HaveOccurred())
+			rw.WriteHeader(502)
-		errorHandler := NewProxyErrorHandler(tmpl, "prefix")
+			rw.Write([]byte("Proxy Error"))
+		}
 
 		ok := http.StatusOK
 
@@ -56,6 +56,7 @@ var _ = Describe("Proxy Suite", func() {
 			},
 		}
 
+		var err error
 		upstreamServer, err = NewProxy(upstreams, sigData, errorHandler)
 		Expect(err).ToNot(HaveOccurred())
 	})
@@ -143,7 +144,7 @@ var _ = Describe("Proxy Suite", func() {
 					gapUpstream: {"bad-http-backend"},
 				},
 				// This tests the error handler
-				raw: "Bad Gateway\nError proxying to upstream server\nprefix",
+				raw: "Proxy Error",
 			},
 		}),
 		Entry("with a request to the to an unregistered path", &proxyTableInput{