From ccee1b3f96da67cdae687f0208f85cbf5a158fa4 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 11 Oct 2016 17:36:30 +0200 Subject: [PATCH] fix(alerting): fix bug with unhandled error closes #6239 --- pkg/services/alerting/extractor.go | 9 +++++++-- pkg/services/alerting/rule.go | 16 +++++++++++++--- pkg/services/alerting/rule_test.go | 13 +++++++++---- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index 6a8be4f6deb..8f79354898d 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -80,6 +80,11 @@ func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) { continue } + frequency, err := getTimeDurationStringToSeconds(jsonAlert.Get("frequency").MustString()) + if err != nil { + return nil, ValidationError{Reason: "Could not parse frequency"} + } + alert := &m.Alert{ DashboardId: e.Dash.Id, OrgId: e.OrgId, @@ -88,7 +93,7 @@ func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) { Name: jsonAlert.Get("name").MustString(), Handler: jsonAlert.Get("handler").MustInt64(), Message: jsonAlert.Get("message").MustString(), - Frequency: getTimeDurationStringToSeconds(jsonAlert.Get("frequency").MustString()), + Frequency: frequency, } for _, condition := range jsonAlert.Get("conditions").MustArray() { @@ -121,7 +126,7 @@ func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) { alert.Settings = jsonAlert // validate - _, err := NewRuleFromDBAlert(alert) + _, err = NewRuleFromDBAlert(alert) if err == nil && alert.ValidToSave() { alerts = append(alerts, alert) } else { diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 5f59b60b64f..bfbb28b99fb 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -43,17 +43,27 @@ var unitMultiplier = map[string]int{ "h": 3600, } -func getTimeDurationStringToSeconds(str string) int64 { +func getTimeDurationStringToSeconds(str string) (int64, error) { multiplier := 1 - value, _ := strconv.Atoi(ValueFormatRegex.FindAllString(str, 1)[0]) + matches := ValueFormatRegex.FindAllString(str, 1) + + if len(matches) <= 0 { + return 0, fmt.Errorf("Frequency could not be parsed") + } + + value, err := strconv.Atoi(matches[0]) + if err != nil { + return 0, err + } + unit := UnitFormatRegex.FindAllString(str, 1)[0] if val, ok := unitMultiplier[unit]; ok { multiplier = val } - return int64(value * multiplier) + return int64(value * multiplier), nil } func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index 622904ec3fc..6144c01d54d 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -20,25 +20,30 @@ func TestAlertRuleModel(t *testing.T) { }) Convey("Can parse seconds", func() { - seconds := getTimeDurationStringToSeconds("10s") + seconds, _ := getTimeDurationStringToSeconds("10s") So(seconds, ShouldEqual, 10) }) Convey("Can parse minutes", func() { - seconds := getTimeDurationStringToSeconds("10m") + seconds, _ := getTimeDurationStringToSeconds("10m") So(seconds, ShouldEqual, 600) }) Convey("Can parse hours", func() { - seconds := getTimeDurationStringToSeconds("1h") + seconds, _ := getTimeDurationStringToSeconds("1h") So(seconds, ShouldEqual, 3600) }) Convey("defaults to seconds", func() { - seconds := getTimeDurationStringToSeconds("1o") + seconds, _ := getTimeDurationStringToSeconds("1o") So(seconds, ShouldEqual, 1) }) + Convey("should return err for empty string", func() { + _, err := getTimeDurationStringToSeconds("") + So(err, ShouldNotBeNil) + }) + Convey("can construct alert rule model", func() { json := ` {