fix: return provider response during fnc listing errors

Return the original upstream response body when the the list request
returns an error. In general, the provider is returning useful and
actionable error messages for the user, the previous code hid this in
the logs and this is easy for user to overlook.

Additionally, remove an early return from error case after fetching
metrics. This looked like a bug and could result in empty api responses
if there was a prometheus error.

Signed-off-by: Lucas Roesler <[email protected]>

Author: LucasRoesler

gateway/metrics/add_metrics.go

@@ -34,15 +34,13 @@ func AddMetricsHandler(handler http.HandlerFunc, prometheusQuery PrometheusQuery
 			log.Printf("List functions responded with code %d, body: %s",
 				recorder.Code,
 				string(upstreamBody))
-
-			http.Error(w, "Metrics handler: unexpected status code from provider listing functions", http.StatusInternalServerError)
+			http.Error(w, string(upstreamBody), recorder.Code)
 			return
 		}
 
 		var functions []types.FunctionStatus
 
 		err := json.Unmarshal(upstreamBody, &functions)
-
 		if err != nil {
 			log.Printf("Metrics upstream error: %s, value: %s", err, string(upstreamBody))
 
@@ -63,8 +61,8 @@ func AddMetricsHandler(handler http.HandlerFunc, prometheusQuery PrometheusQuery
 
 			results, err := prometheusQuery.Fetch(url.QueryEscape(q))
 			if err != nil {
+				// log the error but continue, the mixIn will correctly handle the empty results.
 				log.Printf("Error querying Prometheus: %s\n", err.Error())
-				return
 			}
 			mixIn(&functions, results)
 		}

gateway/metrics/add_metrics_test.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	types "github.com/openfaas/faas-provider/types"
@@ -55,6 +56,34 @@ func Test_PrometheusMetrics_MixedInto_Services(t *testing.T) {
 	}
 }
 
+func Test_MetricHandler_ForwardsErrors(t *testing.T) {
+	functionsHandler := func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusConflict)
+		w.Write([]byte("test error case"))
+	}
+	// explicitly set the query fetcher to nil because it should
+	// not be called when a non-200 response is returned from the
+	// functions handler, if it is called then the test will panic
+	handler := AddMetricsHandler(functionsHandler, nil)
+
+	rr := httptest.NewRecorder()
+	request, _ := http.NewRequest(http.MethodGet, "/system/functions", nil)
+	handler.ServeHTTP(rr, request)
+
+	if status := rr.Code; status != http.StatusConflict {
+		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusConflict)
+	}
+
+	if rr.Header().Get("Content-Type") != "text/plain; charset=utf-8" {
+		t.Errorf("Want 'text/plain; charset=utf-8' content-type, got: %s", rr.Header().Get("Content-Type"))
+	}
+	body := strings.TrimSpace(rr.Body.String())
+	if body != "test error case" {
+		t.Errorf("Want 'test error case', got: %q", body)
+	}
+
+}
+
 func Test_FunctionsHandler_ReturnsJSONAndOneFunction(t *testing.T) {
 	functionsHandler := makeFunctionsHandler()