Skip to content

Commit 5c9f16d

Browse files
ADI-ROXXyurishkuro
andauthored
Return 400 instead of 500 on incorrect OTLP payload (#6599)
## Which problem is this PR solving? - Resolves #6598 ## Description of the changes - Changed StatusInternalServerError to StatusBadRequest because otlp2traces doing the unmarshaling will go into error if there is error in the payload - Added a new unit test for the change ## How was this change tested? - make lint test => pass ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: cs-308-2023 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
1 parent ca3763f commit 5c9f16d

File tree

2 files changed

+23
-2
lines changed

2 files changed

+23
-2
lines changed

cmd/query/app/http_handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) {
183183
}
184184

185185
traces, err := otlp2traces(body)
186-
if aH.handleError(w, err, http.StatusInternalServerError) {
186+
if aH.handleError(w, err, http.StatusBadRequest) {
187187
return
188188
}
189189

cmd/query/app/http_handler_test.go

+22-1
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,15 @@ func TestTransformOTLPBadPayload(t *testing.T) {
769769
}, querysvc.QueryServiceOptions{})
770770
}
771771

772+
func TestBadOTLPReturns400(t *testing.T) {
773+
withTestServer(t, func(ts *testServer) {
774+
err := postJSON(ts.server.URL+"/api/transform", "Bad Payload", nil)
775+
var httpErr *HTTPError
776+
require.ErrorAs(t, err, &httpErr)
777+
require.Equal(t, 400, httpErr.StatusCode)
778+
}, querysvc.QueryServiceOptions{})
779+
}
780+
772781
func TestGetMetricsSuccess(t *testing.T) {
773782
mr := &metricsmocks.Reader{}
774783
apiHandlerOptions := []HandlerOption{
@@ -980,6 +989,15 @@ func postJSON(url string, req any, out any) error {
980989
return execJSON(r, make(map[string]string), out)
981990
}
982991

992+
type HTTPError struct {
993+
StatusCode int
994+
Body string
995+
}
996+
997+
func (e *HTTPError) Error() string {
998+
return fmt.Sprintf("%d error from server: %s", e.StatusCode, e.Body)
999+
}
1000+
9831001
// execJSON executes an http request against a server and parses response as JSON
9841002
func execJSON(req *http.Request, additionalHeaders map[string]string, out any) error {
9851003
req.Header.Add("Accept", "application/json")
@@ -999,7 +1017,10 @@ func execJSON(req *http.Request, additionalHeaders map[string]string, out any) e
9991017
if err != nil {
10001018
return err
10011019
}
1002-
return fmt.Errorf("%d error from server: %s", resp.StatusCode, body)
1020+
return &HTTPError{
1021+
StatusCode: resp.StatusCode,
1022+
Body: string(body),
1023+
}
10031024
}
10041025

10051026
if out == nil {

0 commit comments

Comments
 (0)