From 7be94ce96216f60459eaf1fb85190b3c6a20822b Mon Sep 17 00:00:00 2001
From: Fabian Reinartz <fab.reinartz@gmail.com>
Date: Tue, 9 Jun 2015 13:44:49 +0200
Subject: [PATCH] web/api: improve errors, add tests

---
 web/api/v1/api.go      | 24 +++++++----
 web/api/v1/api_test.go | 94 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 101 insertions(+), 17 deletions(-)

diff --git a/web/api/v1/api.go b/web/api/v1/api.go
index 7a430b732..8f97e345e 100644
--- a/web/api/v1/api.go
+++ b/web/api/v1/api.go
@@ -112,7 +112,13 @@ func (api *API) query(r *http.Request) (interface{}, *apiError) {
 
 	res := qry.Exec()
 	if res.Err != nil {
-		return nil, &apiError{errorBadData, res.Err}
+		switch res.Err.(type) {
+		case promql.ErrQueryCanceled:
+			return nil, &apiError{errorCanceled, res.Err}
+		case promql.ErrQueryTimeout:
+			return nil, &apiError{errorTimeout, res.Err}
+		}
+		return nil, &apiError{errorExec, res.Err}
 	}
 	return &queryData{
 		ResultType: res.Value.Type(),
@@ -143,18 +149,18 @@ func (api *API) queryRange(r *http.Request) (interface{}, *apiError) {
 
 	qry, err := api.QueryEngine.NewRangeQuery(r.FormValue("query"), start, end, step)
 	if err != nil {
-		switch err.(type) {
-		case promql.ErrQueryCanceled:
-			return nil, &apiError{errorCanceled, err}
-		case promql.ErrQueryTimeout:
-			return nil, &apiError{errorTimeout, err}
-		}
-		return nil, &apiError{errorExec, err}
+		return nil, &apiError{errorBadData, err}
 	}
 
 	res := qry.Exec()
 	if res.Err != nil {
-		return nil, &apiError{errorBadData, err}
+		switch res.Err.(type) {
+		case promql.ErrQueryCanceled:
+			return nil, &apiError{errorCanceled, res.Err}
+		case promql.ErrQueryTimeout:
+			return nil, &apiError{errorTimeout, res.Err}
+		}
+		return nil, &apiError{errorExec, res.Err}
 	}
 	return &queryData{
 		ResultType: res.Value.Type(),
diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
index 4c808634f..103f1d0c8 100644
--- a/web/api/v1/api_test.go
+++ b/web/api/v1/api_test.go
@@ -16,6 +16,7 @@ import (
 	clientmodel "github.com/prometheus/client_golang/model"
 
 	"github.com/prometheus/prometheus/promql"
+	"github.com/prometheus/prometheus/storage/metric"
 	"github.com/prometheus/prometheus/util/route"
 )
 
@@ -90,6 +91,82 @@ func TestEndpoints(t *testing.T) {
 				},
 			},
 		},
+		{
+			endpoint: api.queryRange,
+			query: url.Values{
+				"query": []string{"time()"},
+				"start": []string{"0"},
+				"end":   []string{"2"},
+				"step":  []string{"1"},
+			},
+			response: &queryData{
+				ResultType: promql.ExprMatrix,
+				Result: promql.Matrix{
+					&promql.SampleStream{
+						Values: metric.Values{
+							{Value: 0, Timestamp: start},
+							{Value: 1, Timestamp: start.Add(1 * time.Second)},
+							{Value: 2, Timestamp: start.Add(2 * time.Second)},
+						},
+					},
+				},
+			},
+		},
+		// Missing query params in range queries.
+		{
+			endpoint: api.queryRange,
+			query: url.Values{
+				"query": []string{"time()"},
+				"end":   []string{"2"},
+				"step":  []string{"1"},
+			},
+			errType: errorBadData,
+		},
+		{
+			endpoint: api.queryRange,
+			query: url.Values{
+				"query": []string{"time()"},
+				"start": []string{"0"},
+				"step":  []string{"1"},
+			},
+			errType: errorBadData,
+		},
+		{
+			endpoint: api.queryRange,
+			query: url.Values{
+				"query": []string{"time()"},
+				"start": []string{"0"},
+				"end":   []string{"2"},
+			},
+			errType: errorBadData,
+		},
+		// Missing evaluation time.
+		{
+			endpoint: api.query,
+			query: url.Values{
+				"query": []string{"0.333"},
+			},
+			errType: errorBadData,
+		},
+		// Bad query expression.
+		{
+			endpoint: api.query,
+			query: url.Values{
+				"query": []string{"invalid][query"},
+				"time":  []string{"1970-01-01T01:02:03+01:00"},
+			},
+			errType: errorBadData,
+		},
+		{
+			endpoint: api.queryRange,
+			query: url.Values{
+				"query": []string{"invalid][query"},
+				"start": []string{"0"},
+				"end":   []string{"100"},
+				"step":  []string{"1"},
+			},
+			errType: errorBadData,
+		},
 		{
 			endpoint: api.labelValues,
 			params: map[string]string{
@@ -100,13 +177,6 @@ func TestEndpoints(t *testing.T) {
 				"test_metric2",
 			},
 		},
-		{
-			endpoint: api.labelValues,
-			params: map[string]string{
-				"name": "not!!!allowed",
-			},
-			errType: errorBadData,
-		},
 		{
 			endpoint: api.labelValues,
 			params: map[string]string{
@@ -117,6 +187,14 @@ func TestEndpoints(t *testing.T) {
 				"boo",
 			},
 		},
+		// Bad name parameter.
+		{
+			endpoint: api.labelValues,
+			params: map[string]string{
+				"name": "not!!!allowed",
+			},
+			errType: errorBadData,
+		},
 	}
 
 	for _, test := range tests {
@@ -147,7 +225,7 @@ func TestEndpoints(t *testing.T) {
 			t.Fatalf("Expected error of type %q but got none", test.errType)
 		}
 		if !reflect.DeepEqual(resp, test.response) {
-			t.Fatalf("Response does not match, expected:\n%v\ngot:\n%v", test.response, resp)
+			t.Fatalf("Response does not match, expected:\n%#v\ngot:\n%#v", test.response, resp)
 		}
 	}
 }