[query] Fix nil panic for error response for PromReadInstantHandler

src/query/api/v1/handler/prometheus/native/read_instantaneous.go

@@ -74,13 +74,13 @@ func (h *PromReadInstantHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 	}
 
 	if params.Debug {
-		logger.Info("Request params", zap.Any("params", params))
+		logger.Info("request params", zap.Any("params", params))
 	}
 
 	result, err := read(ctx, h.engine, h.tagOpts, w, params)
 	if err != nil {
 		logger.Error("unable to fetch data", zap.Error(err))
-		httperrors.ErrorWithReqInfo(w, r, http.StatusBadRequest, rErr)
+		httperrors.ErrorWithReqInfo(w, r, http.StatusInternalServerError, err)
 		return
 	}
 

src/query/api/v1/handler/prometheus/native/read_instantaneous_test.go

@@ -0,0 +1,172 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package native
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/m3db/m3/src/query/block"
+	"github.com/m3db/m3/src/query/test"
+	"github.com/m3db/m3/src/query/util/logging"
+	xtest "github.com/m3db/m3/src/x/test"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type vectorResult struct {
+	Data struct {
+		Result []struct {
+			Metric map[string]string  `json:"metric"`
+			Value  vectorResultValues `json:"value"`
+		} `json:"result"`
+	} `json:"data"`
+}
+
+type vectorResultValues []interface{}
+
+func (v vectorResultValues) parse() (time.Time, int, error) {
+	if len(v) != 2 {
+		return time.Time{}, 0,
+			fmt.Errorf("expected length 2: actual=%d", len(v))
+	}
+
+	t, ok := v[0].(float64)
+	if !ok {
+		return time.Time{}, 0,
+			fmt.Errorf("could not unmarshal time: %v", v[0])
+	}
+
+	str, ok := v[1].(string)
+	if !ok {
+		return time.Time{}, 0,
+			fmt.Errorf("could not unmarshal value: %v", v[1])
+	}
+
+	n, err := strconv.Atoi(str)
+	if err != nil {
+		return time.Time{}, 0,
+			fmt.Errorf("could not convert value to number: err=%v", err)
+	}
+
+	return time.Unix(int64(t), 0), n, nil
+}
+
+func TestPromReadInstantHandler(t *testing.T) {
+	logging.InitWithCores(nil)
+
+	values, bounds := test.GenerateValuesAndBounds(nil, nil)
+
+	setup := newTestSetup()
+	promReadInstant := setup.Handlers.InstantRead
+
+	b := test.NewBlockFromValues(bounds, values)
+	setup.Storage.SetFetchBlocksResult(block.Result{Blocks: []block.Block{b}}, nil)
+
+	req := httptest.NewRequest(PromReadInstantHTTPMethod, PromReadInstantURL, nil)
+
+	params := url.Values{}
+	params.Set(queryParam, "dummy0{}")
+
+	req.URL.RawQuery = params.Encode()
+
+	recorder := httptest.NewRecorder()
+	promReadInstant.ServeHTTP(recorder, req)
+
+	require.Equal(t, http.StatusOK, recorder.Result().StatusCode)
+
+	var result vectorResult
+	require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &result))
+	require.Equal(t, 2, len(result.Data.Result))
+
+	at0, value0, err := result.Data.Result[0].Value.parse()
+	require.NoError(t, err)
+	at1, value1, err := result.Data.Result[1].Value.parse()
+	require.NoError(t, err)
+
+	expected := mustPrettyJSON(t, fmt.Sprintf(`
+	{
+		"status": "success",
+		"data": {
+			"resultType": "vector",
+			"result": [
+				{
+					"metric": {
+						"__name__": "dummy0",
+						"dummy0": "dummy0"
+					},
+					"value": [
+						%d,
+						"%d"
+					]
+				},
+				{
+					"metric": {
+						"__name__": "dummy1",
+						"dummy1": "dummy1"
+					},
+					"value": [
+						%d,
+						"%d"
+					]
+				}
+			]
+		}
+	}
+	`, at0.Unix(), value0, at1.Unix(), value1))
+	actual := mustPrettyJSON(t, recorder.Body.String())
+	assert.Equal(t, expected, actual, xtest.Diff(expected, actual))
+}
+
+func TestPromReadInstantHandlerStorageError(t *testing.T) {
+	logging.InitWithCores(nil)
+
+	setup := newTestSetup()
+	promReadInstant := setup.Handlers.InstantRead
+
+	storageErr := fmt.Errorf("storage err")
+	setup.Storage.SetFetchBlocksResult(block.Result{}, storageErr)
+
+	req := httptest.NewRequest(PromReadInstantHTTPMethod, PromReadInstantURL, nil)
+
+	params := url.Values{}
+	params.Set(queryParam, "dummy0{}")
+
+	req.URL.RawQuery = params.Encode()
+
+	recorder := httptest.NewRecorder()
+	promReadInstant.ServeHTTP(recorder, req)
+
+	require.Equal(t, http.StatusInternalServerError, recorder.Result().StatusCode)
+
+	var errResp struct {
+		Error string `json:"error"`
+	}
+	resp := recorder.Body.Bytes()
+	require.NoError(t, json.Unmarshal(resp, &errResp))
+	require.Equal(t, storageErr.Error(), errResp.Error)
+}

src/query/api/v1/handler/prometheus/native/read_test.go

@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"github.com/m3db/m3/src/cmd/services/m3query/config"
+	"github.com/m3db/m3/src/query/api/v1/handler/prometheus"
 	"github.com/m3db/m3/src/query/block"
 	"github.com/m3db/m3/src/query/executor"
 	"github.com/m3db/m3/src/query/models"
@@ -49,7 +50,7 @@ func TestPromReadHandler_Read(t *testing.T) {
 	values, bounds := test.GenerateValuesAndBounds(nil, nil)
 
 	setup := newTestSetup()
-	promRead := setup.Handler
+	promRead := setup.Handlers.Read
 
 	b := test.NewBlockFromValues(bounds, values)
 	setup.Storage.SetFetchBlocksResult(block.Result{Blocks: []block.Block{b}}, nil)
@@ -84,7 +85,7 @@ func TestPromReadHandler_ReadM3QL(t *testing.T) {
 	values, bounds := test.GenerateValuesAndBounds(nil, nil)
 
 	setup := newTestSetup()
-	promRead := setup.Handler
+	promRead := setup.Handlers.Read
 
 	b := test.NewBlockFromValues(bounds, values)
 	setup.Storage.SetFetchBlocksResult(block.Result{Blocks: []block.Block{b}}, nil)
@@ -116,29 +117,45 @@ func newReadRequest(t *testing.T, params url.Values) *http.Request {
 }
 
 type testSetup struct {
-	Storage mock.Storage
-	Handler *PromReadHandler
+	Storage     mock.Storage
+	Handlers    testSetupHandlers
+	TimeoutOpts *prometheus.TimeoutOpts
+}
+
+type testSetupHandlers struct {
+	Read        *PromReadHandler
+	InstantRead *PromReadInstantHandler
 }
 
 func newTestSetup() *testSetup {
 	mockStorage := mock.NewMockStorage()
 
+	scope := tally.NoopScope
+	engine := executor.NewEngine(mockStorage, scope,
+		time.Minute, nil)
+	tagOpts := models.NewTagOptions()
+	limitsConfig := &config.LimitsConfiguration{}
+	keepNans := false
+
+	read := NewPromReadHandler(engine, tagOpts, limitsConfig,
+		scope, timeoutOpts, keepNans)
+
+	instantRead := NewPromReadInstantHandler(engine, tagOpts,
+		timeoutOpts)
+
 	return &testSetup{
 		Storage: mockStorage,
-		Handler: NewPromReadHandler(
-			executor.NewEngine(mockStorage, tally.NewTestScope("test", nil), time.Minute, nil),
-			models.NewTagOptions(),
-			&config.LimitsConfiguration{},
-			tally.NewTestScope("", nil),
-			timeoutOpts,
-			false,
-		),
+		Handlers: testSetupHandlers{
+			Read:        read,
+			InstantRead: instantRead,
+		},
+		TimeoutOpts: timeoutOpts,
 	}
 }
 
 func TestPromReadHandler_ServeHTTP_maxComputedDatapoints(t *testing.T) {
 	setup := newTestSetup()
-	setup.Handler.limitsCfg = &config.LimitsConfiguration{
+	setup.Handlers.Read.limitsCfg = &config.LimitsConfiguration{
 		PerQuery: config.PerQueryLimitsConfiguration{
 			PrivateMaxComputedDatapoints: 3599,
 		},
@@ -151,7 +168,7 @@ func TestPromReadHandler_ServeHTTP_maxComputedDatapoints(t *testing.T) {
 	req := newReadRequest(t, params)
 
 	recorder := httptest.NewRecorder()
-	setup.Handler.ServeHTTP(recorder, req)
+	setup.Handlers.Read.ServeHTTP(recorder, req)
 	resp := recorder.Result()
 
 	assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
@@ -249,13 +266,13 @@ func TestPromReadHandler_validateRequest(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.Name, func(t *testing.T) {
 			setup := newTestSetup()
-			setup.Handler.limitsCfg = &config.LimitsConfiguration{
+			setup.Handlers.Read.limitsCfg = &config.LimitsConfiguration{
 				PerQuery: config.PerQueryLimitsConfiguration{
 					PrivateMaxComputedDatapoints: tc.Max,
 				},
 			}
 
-			err := setup.Handler.validateRequest(tc.Params)
+			err := setup.Handlers.Read.validateRequest(tc.Params)
 
 			if tc.ErrorExpected {
 				require.Error(t, err)