Skip to content

Commit

Permalink
fix(api): 500 on incorrect requests (#806)
Browse files Browse the repository at this point in the history
Api returned 500 on create trigger requests with incorrect json. Now it returns 400
  • Loading branch information
Tetrergeru authored Dec 5, 2022
1 parent 2ec1ec9 commit 0e97cc7
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 0 deletions.
3 changes: 3 additions & 0 deletions api/handler/triggers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handler

import (
"encoding/json"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -93,6 +94,8 @@ func getTriggerFromRequest(request *http.Request) (*dto.Trigger, *api.ErrorRespo
response := api.ErrorRemoteServerUnavailable(err)
middleware.GetLoggerEntry(request).Error("%s : %s : %s", response.StatusText, response.ErrorText, err)
return nil, response
case *json.UnmarshalTypeError:
return nil, api.ErrorInvalidRequest(fmt.Errorf("invalid payload: %s", err.Error()))
default:
return nil, api.ErrorInternalServer(err)
}
Expand Down
91 changes: 91 additions & 0 deletions api/handler/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"time"

"github.com/golang/mock/gomock"
"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
metricSource "github.com/moira-alert/moira/metric_source"
mock_metric_source "github.com/moira-alert/moira/mock/metric_source"

Expand Down Expand Up @@ -42,6 +44,95 @@ func TestGetSearchRequestString(t *testing.T) {
})
}

func TestGetTriggerFromRequest(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

localSource := mock_metric_source.NewMockMetricSource(mockCtrl)
remoteSource := mock_metric_source.NewMockMetricSource(mockCtrl)
fetchResult := mock_metric_source.NewMockFetchResult(mockCtrl)
sourceProvider := metricSource.CreateMetricSourceProvider(localSource, remoteSource)

localSource.EXPECT().IsConfigured().Return(true, nil).AnyTimes()
localSource.EXPECT().GetMetricsTTLSeconds().Return(int64(3600)).AnyTimes()
localSource.EXPECT().Fetch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(fetchResult, nil).AnyTimes()
fetchResult.EXPECT().GetPatterns().Return(make([]string, 0), nil).AnyTimes()
fetchResult.EXPECT().GetMetricsData().Return([]metricSource.MetricData{*metricSource.MakeMetricData("", []float64{}, 0, 0)}).AnyTimes()

Convey("Given a correct payload", t, func() {
triggerWarnValue := 0.0
triggerErrorValue := 1.0
ttlState := moira.TTLState("NODATA")
triggerDTO := dto.Trigger{
TriggerModel: dto.TriggerModel{
ID: "test_id",
Name: "Test trigger",
Desc: new(string),
Targets: []string{"foo.bar"},
WarnValue: &triggerWarnValue,
ErrorValue: &triggerErrorValue,
TriggerType: "rising",
Tags: []string{"Normal", "DevOps", "DevOpsGraphite-duty"},
TTLState: &ttlState,
TTL: 0,
Schedule: &moira.ScheduleData{},
Expression: "",
Patterns: []string{},
IsRemote: false,
MuteNewMetrics: false,
AloneMetrics: map[string]bool{},
CreatedAt: &time.Time{},
UpdatedAt: &time.Time{},
},
}
body, _ := json.Marshal(triggerDTO)

request := httptest.NewRequest(http.MethodPut, "/trigger", bytes.NewReader(body))
request.Header.Add("content-type", "application/json")
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))

Convey("It should be parsed successfully", func() {
trigger, err := getTriggerFromRequest(request)
So(err, ShouldBeNil)
So(trigger, ShouldResemble, &triggerDTO)
})
})

Convey("Given an incorrect payload", t, func() {
body := `{
"name": "test",
"desc": "",
"targets": ["foo.bar"],
"tags": ["test"],
"patterns": [],
"expression": "",
"ttl": 600,
"ttl_state": "NODATA",
"sched": {
"startOffset": 0,
"endOffset": 1439,
"tzOffset": -240,
"days": null
},
"is_remote": false,
"error_value": 1,
"warn_value": 0,
"trigger_type": "rising",
"mute_new_metrics": false,
"alone_metrics": "beliberda"
}`

request := httptest.NewRequest(http.MethodPut, "/trigger", strings.NewReader(body))
request.Header.Add("content-type", "application/json")
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))

Convey("Parser should return en error", func() {
_, err := getTriggerFromRequest(request)
So(err, ShouldHaveSameTypeAs, api.ErrorInvalidRequest(fmt.Errorf("")))
})
})
}

func TestGetMetricTTLByTrigger(t *testing.T) {
request := httptest.NewRequest(http.MethodPut, "/trigger/new", strings.NewReader(""))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "localMetricTTL", to.Duration("65m")))
Expand Down

0 comments on commit 0e97cc7

Please sign in to comment.