Skip to content

Commit 7db63e9

Browse files
alexluongalexluong
committed
chore: error handling
1 parent 0c437b5 commit 7db63e9

2 files changed

Lines changed: 113 additions & 113 deletions

File tree

‎internal/alert/monitor.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ func (m *alertMonitor) HandleAttempt(ctx context.Context, attempt DeliveryAttemp
182182
zap.String("destination_type", attempt.Destination.Type),
183183
)
184184

185-
// Send destination disabled alert
185+
// Send destination disabled alert (best-effort, don't fail on notification error)
186186
if m.notifier != nil {
187187
disabledAlert := NewDestinationDisabledAlert(DestinationDisabledData{
188188
TenantID: attempt.Destination.TenantID,
@@ -204,30 +204,27 @@ func (m *alertMonitor) HandleAttempt(ctx context.Context, attempt DeliveryAttemp
204204
zap.String("tenant_id", attempt.Destination.TenantID),
205205
zap.String("destination_id", attempt.Destination.ID),
206206
)
207-
return fmt.Errorf("failed to send destination disabled alert: %w", err)
208207
}
209208
}
210209
}
211210

212-
// Send alert if notifier is configured
211+
// Send alert if notifier is configured (best-effort, don't fail on notification error)
213212
if m.notifier != nil {
214213
if err := m.notifier.Notify(ctx, alert); err != nil {
215-
m.logger.Ctx(ctx).Error("failed to send alert",
214+
m.logger.Ctx(ctx).Error("failed to send consecutive failure alert",
216215
zap.Error(err),
217216
zap.String("event_id", attempt.DeliveryTask.Event.ID),
218217
zap.String("tenant_id", attempt.Destination.TenantID),
219218
zap.String("destination_id", attempt.Destination.ID),
219+
)
220+
} else {
221+
m.logger.Ctx(ctx).Audit("alert sent",
222+
zap.String("event_id", attempt.DeliveryTask.Event.ID),
223+
zap.String("tenant_id", attempt.Destination.TenantID),
224+
zap.String("destination_id", attempt.Destination.ID),
220225
zap.String("destination_type", attempt.Destination.Type),
221226
)
222-
return fmt.Errorf("failed to send alert: %w", err)
223227
}
224-
225-
m.logger.Ctx(ctx).Audit("alert sent",
226-
zap.String("event_id", attempt.DeliveryTask.Event.ID),
227-
zap.String("tenant_id", attempt.Destination.TenantID),
228-
zap.String("destination_id", attempt.Destination.ID),
229-
zap.String("destination_type", attempt.Destination.Type),
230-
)
231228
}
232229

233230
return nil

‎internal/alert/notifier_test.go‎

Lines changed: 104 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"net/http"
77
"net/http/httptest"
8+
"sync/atomic"
89
"testing"
910
"time"
1011

@@ -16,107 +17,109 @@ import (
1617
func TestAlertNotifier_Notify(t *testing.T) {
1718
t.Parallel()
1819

19-
tests := []struct {
20-
name string
21-
handler func(w http.ResponseWriter, r *http.Request)
22-
notifierOpts []alert.NotifierOption
23-
wantErr bool
24-
errContains string
25-
}{
26-
{
27-
name: "successful notification",
28-
handler: func(w http.ResponseWriter, r *http.Request) {
29-
// Verify request
30-
assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
31-
32-
// Read and verify request body
33-
var body map[string]interface{}
34-
err := json.NewDecoder(r.Body).Decode(&body)
35-
require.NoError(t, err)
36-
37-
assert.Equal(t, "alert.consecutive_failure", body["topic"])
38-
data := body["data"].(map[string]interface{})
39-
assert.Equal(t, float64(10), data["max_consecutive_failures"])
40-
assert.Equal(t, float64(5), data["consecutive_failures"])
41-
assert.Equal(t, true, data["will_disable"])
42-
43-
// Log raw JSON for debugging
44-
rawJSON, _ := json.Marshal(body)
45-
t.Logf("Raw JSON: %s", string(rawJSON))
46-
47-
w.WriteHeader(http.StatusOK)
48-
},
49-
},
50-
{
51-
name: "server error",
52-
handler: func(w http.ResponseWriter, r *http.Request) {
53-
w.WriteHeader(http.StatusInternalServerError)
54-
},
55-
wantErr: true,
56-
errContains: "alert callback failed with status 500",
57-
},
58-
{
59-
name: "invalid response status",
60-
handler: func(w http.ResponseWriter, r *http.Request) {
61-
w.WriteHeader(http.StatusBadRequest)
62-
},
63-
wantErr: true,
64-
errContains: "alert callback failed with status 400",
65-
},
66-
{
67-
name: "timeout exceeded",
68-
handler: func(w http.ResponseWriter, r *http.Request) {
69-
time.Sleep(100 * time.Millisecond)
70-
w.WriteHeader(http.StatusOK)
71-
},
72-
notifierOpts: []alert.NotifierOption{alert.NotifierWithTimeout(50 * time.Millisecond)},
73-
wantErr: true,
74-
errContains: "context deadline exceeded",
75-
},
76-
{
77-
name: "successful notification with bearer token",
78-
handler: func(w http.ResponseWriter, r *http.Request) {
79-
assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization"))
80-
w.WriteHeader(http.StatusOK)
20+
t.Run("successful notification", func(t *testing.T) {
21+
t.Parallel()
22+
var called atomic.Bool
23+
24+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
25+
called.Store(true)
26+
// Verify request
27+
assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
28+
29+
// Read and verify request body
30+
var body map[string]any
31+
err := json.NewDecoder(r.Body).Decode(&body)
32+
require.NoError(t, err)
33+
34+
assert.Equal(t, "alert.consecutive_failure", body["topic"])
35+
data := body["data"].(map[string]any)
36+
assert.Equal(t, float64(10), data["max_consecutive_failures"])
37+
assert.Equal(t, float64(5), data["consecutive_failures"])
38+
assert.Equal(t, true, data["will_disable"])
39+
40+
w.WriteHeader(http.StatusOK)
41+
}))
42+
defer ts.Close()
43+
44+
notifier := alert.NewHTTPAlertNotifier(ts.URL)
45+
dest := &alert.AlertDestination{ID: "dest_123", TenantID: "tenant_123"}
46+
testAlert := alert.NewConsecutiveFailureAlert(alert.ConsecutiveFailureData{
47+
MaxConsecutiveFailures: 10,
48+
ConsecutiveFailures: 5,
49+
WillDisable: true,
50+
Destination: dest,
51+
AttemptResponse: map[string]any{
52+
"status": "error",
53+
"data": map[string]any{"code": "ETIMEDOUT"},
8154
},
82-
notifierOpts: []alert.NotifierOption{alert.NotifierWithBearerToken("test-token")},
83-
},
84-
}
85-
86-
for _, tt := range tests {
87-
tt := tt
88-
t.Run(tt.name, func(t *testing.T) {
89-
t.Parallel()
90-
91-
// Create test server
92-
ts := httptest.NewServer(http.HandlerFunc(tt.handler))
93-
defer ts.Close()
94-
95-
// Create notifier
96-
notifier := alert.NewHTTPAlertNotifier(ts.URL, tt.notifierOpts...)
97-
98-
// Create test alert
99-
dest := &alert.AlertDestination{ID: "dest_123", TenantID: "tenant_123"}
100-
testAlert := alert.NewConsecutiveFailureAlert(alert.ConsecutiveFailureData{
101-
MaxConsecutiveFailures: 10,
102-
ConsecutiveFailures: 5,
103-
WillDisable: true,
104-
Destination: dest,
105-
AttemptResponse: map[string]interface{}{
106-
"status": "error",
107-
"data": map[string]any{"code": "ETIMEDOUT"},
108-
},
109-
})
110-
111-
// Send alert
112-
err := notifier.Notify(context.Background(), testAlert)
113-
114-
if tt.wantErr {
115-
require.Error(t, err)
116-
assert.ErrorContains(t, err, tt.errContains)
117-
} else {
118-
require.NoError(t, err)
119-
}
12055
})
121-
}
56+
57+
err := notifier.Notify(context.Background(), testAlert)
58+
require.NoError(t, err)
59+
assert.True(t, called.Load(), "handler should have been called")
60+
})
61+
62+
t.Run("successful notification with bearer token", func(t *testing.T) {
63+
t.Parallel()
64+
var called atomic.Bool
65+
66+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
67+
called.Store(true)
68+
assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization"))
69+
w.WriteHeader(http.StatusOK)
70+
}))
71+
defer ts.Close()
72+
73+
notifier := alert.NewHTTPAlertNotifier(ts.URL, alert.NotifierWithBearerToken("test-token"))
74+
dest := &alert.AlertDestination{ID: "dest_123", TenantID: "tenant_123"}
75+
testAlert := alert.NewConsecutiveFailureAlert(alert.ConsecutiveFailureData{
76+
MaxConsecutiveFailures: 10,
77+
ConsecutiveFailures: 5,
78+
WillDisable: true,
79+
Destination: dest,
80+
})
81+
82+
err := notifier.Notify(context.Background(), testAlert)
83+
require.NoError(t, err)
84+
assert.True(t, called.Load(), "handler should have been called")
85+
})
86+
87+
t.Run("server error returns error", func(t *testing.T) {
88+
t.Parallel()
89+
90+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
91+
w.WriteHeader(http.StatusInternalServerError)
92+
}))
93+
defer ts.Close()
94+
95+
notifier := alert.NewHTTPAlertNotifier(ts.URL)
96+
dest := &alert.AlertDestination{ID: "dest_123", TenantID: "tenant_123"}
97+
testAlert := alert.NewConsecutiveFailureAlert(alert.ConsecutiveFailureData{
98+
Destination: dest,
99+
})
100+
101+
err := notifier.Notify(context.Background(), testAlert)
102+
require.Error(t, err)
103+
assert.Contains(t, err.Error(), "status 500")
104+
})
105+
106+
t.Run("timeout returns error", func(t *testing.T) {
107+
t.Parallel()
108+
109+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
110+
time.Sleep(100 * time.Millisecond)
111+
w.WriteHeader(http.StatusOK)
112+
}))
113+
defer ts.Close()
114+
115+
notifier := alert.NewHTTPAlertNotifier(ts.URL, alert.NotifierWithTimeout(50*time.Millisecond))
116+
dest := &alert.AlertDestination{ID: "dest_123", TenantID: "tenant_123"}
117+
testAlert := alert.NewConsecutiveFailureAlert(alert.ConsecutiveFailureData{
118+
Destination: dest,
119+
})
120+
121+
err := notifier.Notify(context.Background(), testAlert)
122+
require.Error(t, err)
123+
assert.Contains(t, err.Error(), "failed to send alert")
124+
})
122125
}

0 commit comments

Comments
 (0)