Skip to content

Commit

Permalink
Never return 5xx errors on delivery failures (#39)
Browse files Browse the repository at this point in the history
  • Loading branch information
colinodell authored Jan 9, 2025
1 parent cac130a commit d807c5e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
12 changes: 7 additions & 5 deletions pkg/server/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,20 @@ func CreateEventHandler(s handler.Handler, n notifier.Notifier) http.HandlerFunc
id := notifications[0].Context().ID
slog.Debug("dispatching notifications", "id", id, "handler", s.Key(), "notifications", len(notifications))

var errs []error
errorCount := 0
for _, notification := range notifications {
err = n.Push(request.Context(), notification)
if err != nil {
errs = append(errs, err)
// We assume that the notifier will log the error details, as it will have more context about what went wrong
errorCount++
}
}

if len(errs) > 0 {
http.Error(writer, fmt.Sprintf("failed to send notifications: %v", errs), 500)
return
if errorCount > 0 {
slog.Warn("some notifications failed to send", "id", id, "handler", s.Key(), "sent", len(notifications)-errorCount, "failed", errorCount)
}

http.Error(writer, "notifications enqueued", http.StatusAccepted)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/server/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestHandler(t *testing.T) {
name: "happy path",
handler: handlerThatReturns(t, someNotifications, nil),
notifier: notifierThatReturns(t, nil),
wantStatusCode: 200,
wantStatusCode: 202,
},
{
name: "no notifications generated",
Expand All @@ -61,7 +61,7 @@ func TestHandler(t *testing.T) {
name: "notifier error",
handler: handlerThatReturns(t, someNotifications, nil),
notifier: notifierThatReturns(t, someError),
wantStatusCode: 500,
wantStatusCode: 202,
},
}

Expand Down

0 comments on commit d807c5e

Please sign in to comment.