[Bugfix] Return 400 instead of 500 when query_range or query_instant requests have unparseable start/end parameters

CHANGELOG.md

@@ -46,6 +46,7 @@
 * [BUGFIX] metrics-generator: Fix active-series counter underflow in local series limiter when overflow series are deleted [#6568](https://github.com/grafana/tempo/pull/6568) (@carles-grafana)
 * [BUGFIX] fix: skip per-label limiter and sanitizer for target_info and host_info metrics in metrics-generator [#6660](https://github.com/grafana/tempo/pull/6660) (@electron0zero)
 * [BUGFIX] fix(traceql): err on division by zero [#6580](https://github.com/grafana/tempo/pull/6580) (@Proximyst)
+* [BUGFIX] Return 400 instead of 500 when query_range or query_instant requests have unparseable start/end parameters [#6694](https://github.com/grafana/tempo/pull/6694) (@ruslan-mikhailov)
 
 ### 3.0 Cleanup
 

pkg/api/http.go

@@ -304,7 +304,10 @@ func ParseQueryInstantRequest(r *http.Request) (*tempopb.QueryInstantRequest, er
 		req.Query = s
 	}
 
-	start, end, _ := bounds(vals)
+	start, end, err := bounds(vals)
+	if err != nil {
+		return nil, httpgrpc.Error(http.StatusBadRequest, err.Error())
+	}
 	req.Start = uint64(start.UnixNano())
 	req.End = uint64(end.UnixNano())
 
@@ -329,7 +332,10 @@ func ParseQueryRangeRequest(r *http.Request) (*tempopb.QueryRangeRequest, error)
 		req.QueryMode = s
 	}
 
-	start, end, _ := bounds(vals)
+	start, end, err := bounds(vals)
+	if err != nil {
+		return nil, httpgrpc.Error(http.StatusBadRequest, err.Error())
+	}
 	req.Start = uint64(start.UnixNano())
 	req.End = uint64(end.UnixNano())
 

pkg/api/http_test.go

@@ -696,6 +696,41 @@ func Test_parseTimestamp(t *testing.T) {
 	}
 }
 
+func TestParseQueryRequestInvalidBounds(t *testing.T) {
+	tests := []struct {
+		name   string
+		params map[string]string
+		errMsg string
+	}{
+		{
+			name:   "invalid start",
+			params: map[string]string{"q": "{} | rate()", "start": "abc", "end": "1700000000"},
+			errMsg: "could not parse 'start' parameter",
+		},
+		{
+			name:   "invalid end",
+			params: map[string]string{"q": "{} | rate()", "start": "1700000000", "end": "bca"},
+			errMsg: "could not parse 'end' parameter",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run("range/"+tt.name, func(t *testing.T) {
+			r := makeReq(tt.params)
+			_, err := ParseQueryRangeRequest(r)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), tt.errMsg)
+		})
+
+		t.Run("instant/"+tt.name, func(t *testing.T) {
+			r := makeReq(tt.params)
+			_, err := ParseQueryInstantRequest(r)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), tt.errMsg)
+		})
+	}
+}
+
 func TestQueryRangeRoundtripEmpty(t *testing.T) {
 	req := &tempopb.QueryRangeRequest{
 		Step: uint64(time.Second), // you can't actually roundtrip an empty query b/c Build/Parse will force a default step