Files
notifier/internal/service/service_retry_test.go
T
igodwin 21990f2533 fix(service,queue): eliminate data races, add retry backoff, tenant scoping
- Copy discipline for notifications: the store, the queue, workers, and
  API callers each own clones; no notification object is shared across
  goroutines (races previously flagged by -race between workers mutating
  Status/RetryCount and handlers JSON-encoding the same pointer).
- Retry progress derives from QueueMessage.Attempt so it survives
  requeues; exponential backoff (1s base, 30s cap) honors the documented
  queue.retry_backoff setting instead of hammering failing providers in a
  tight loop; shutdown abandons pending backoff waits cleanly.
- Stop() cancels a service-lifetime context so idle workers blocked in
  Dequeue exit immediately instead of waiting out their poll timeout.
- LocalQueue no longer holds its mutex while sending on the queue
  channel (Enqueue/Nack) — with a full buffer this deadlocked the entire
  worker pool, since draining requires the same mutex.
- Tenant scoping: notifications are stamped with the caller's ClientID;
  non-admin clients can only read/cancel/retry their own (cross-tenant
  access reports not-found to avoid leaking existence).
- Sentinel errors ErrNotificationNotFound/ErrNotificationAlreadySent.
- New race, backoff, and tenant-scoping test suites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 09:15:57 -07:00

248 lines
7.5 KiB
Go

package service
import (
"context"
"sync"
"testing"
"time"
"github.com/igodwin/notifier/internal/domain"
"github.com/igodwin/notifier/internal/logging"
"github.com/igodwin/notifier/internal/notifier"
"github.com/igodwin/notifier/internal/queue"
)
// alwaysFailNotifier is a fake domain.Notifier that always reports failure
// while recording the wall-clock time of each Send call, so tests can assert
// on the spacing between retry attempts.
type alwaysFailNotifier struct {
mu sync.Mutex
calls []time.Time
}
func (n *alwaysFailNotifier) Send(ctx context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
n.mu.Lock()
n.calls = append(n.calls, time.Now())
n.mu.Unlock()
return &domain.NotificationResult{
NotificationID: notification.ID,
Success: false,
Error: "simulated failure",
SentAt: time.Now(),
}, nil
}
func (n *alwaysFailNotifier) Type() domain.NotificationType { return domain.TypeStdout }
func (n *alwaysFailNotifier) Validate(notification *domain.Notification) error { return nil }
func (n *alwaysFailNotifier) Close() error { return nil }
func (n *alwaysFailNotifier) callTimes() []time.Time {
n.mu.Lock()
defer n.mu.Unlock()
return append([]time.Time(nil), n.calls...)
}
// createFailingTestService builds a NotificationService wired to the given
// (always failing) notifier instead of the stdout notifier used by
// createTestService.
func createFailingTestService(t *testing.T, fail domain.Notifier) *NotificationService {
t.Helper()
factory := notifier.NewFactory()
if err := factory.RegisterNotifier(domain.TypeStdout, "", fail); err != nil {
t.Fatalf("Failed to register notifier: %v", err)
}
q, err := queue.NewLocalQueue(&domain.LocalQueueConfig{BufferSize: 100})
if err != nil {
t.Fatalf("Failed to create queue: %v", err)
}
logger, err := logging.NewFromConfig("error", "stdout")
if err != nil {
t.Fatalf("Failed to create logger: %v", err)
}
return NewNotificationService(factory, q, 2, nil, nil, logger)
}
// waitForStatus polls GetNotification until it observes the notification in
// the given status, or fails the test after timeout.
func waitForStatus(t *testing.T, svc *NotificationService, ctx context.Context, id string, status domain.NotificationStatus, timeout time.Duration) *domain.Notification {
t.Helper()
deadline := time.Now().Add(timeout)
for time.Now().Before(deadline) {
n, err := svc.GetNotification(ctx, id)
if err == nil && n.Status == status {
return n
}
time.Sleep(10 * time.Millisecond)
}
t.Fatalf("notification %s did not reach status %s within %v", id, status, timeout)
return nil
}
// TestRetryBackoffExponentialDelaysRequeue verifies that, with the default
// (exponential) backoff mode, the delay between retry attempts roughly
// doubles each time rather than hammering the failing notifier in a tight
// loop.
func TestRetryBackoffExponentialDelaysRequeue(t *testing.T) {
fail := &alwaysFailNotifier{}
svc := createFailingTestService(t, fail)
svc.retryBaseDelay = 40 * time.Millisecond // tiny base delay for a fast test
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := svc.Start(ctx); err != nil {
t.Fatalf("Failed to start service: %v", err)
}
defer svc.Stop()
notification := &domain.Notification{
ID: "backoff-exponential-1",
Type: domain.TypeStdout,
Status: domain.StatusPending,
Recipients: []string{"test@example.com"},
MaxRetries: 3,
}
if _, err := svc.Send(ctx, notification); err != nil {
t.Fatalf("Send failed: %v", err)
}
waitForStatus(t, svc, ctx, notification.ID, domain.StatusFailed, 5*time.Second)
calls := fail.callTimes()
if len(calls) != 3 {
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
}
gap1 := calls[1].Sub(calls[0])
gap2 := calls[2].Sub(calls[1])
// Expected gaps are ~40ms then ~80ms; use generous tolerances to absorb
// scheduler jitter while still proving a real, growing delay was applied.
if gap1 < 25*time.Millisecond {
t.Errorf("expected delay before 2nd attempt >= ~40ms, got %v", gap1)
}
if gap2 < gap1 {
t.Errorf("expected delay before 3rd attempt (%v) to be larger than before 2nd (%v)", gap2, gap1)
}
t.Logf("attempt gaps: %v, %v", gap1, gap2)
}
// TestRetryBackoffNoneIsImmediate verifies that retryBackoff "none" requeues
// immediately, ignoring the configured base delay entirely.
func TestRetryBackoffNoneIsImmediate(t *testing.T) {
fail := &alwaysFailNotifier{}
svc := createFailingTestService(t, fail)
svc.WithRetryBackoff("none")
svc.retryBaseDelay = 5 * time.Second // large, to prove "none" ignores it
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := svc.Start(ctx); err != nil {
t.Fatalf("Failed to start service: %v", err)
}
defer svc.Stop()
notification := &domain.Notification{
ID: "backoff-none-1",
Type: domain.TypeStdout,
Status: domain.StatusPending,
Recipients: []string{"test@example.com"},
MaxRetries: 3,
}
start := time.Now()
if _, err := svc.Send(ctx, notification); err != nil {
t.Fatalf("Send failed: %v", err)
}
waitForStatus(t, svc, ctx, notification.ID, domain.StatusFailed, 2*time.Second)
elapsed := time.Since(start)
if elapsed > 1*time.Second {
t.Errorf("expected immediate retries with backoff=none, took %v", elapsed)
}
calls := fail.callTimes()
if len(calls) != 3 {
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
}
}
// TestRetryBackoffShutdownAbandonsCleanly verifies that Stop() does not block
// on (or leak) a goroutine that is waiting out a backoff delay: it should
// abandon the pending retry and mark the notification Failed instead.
func TestRetryBackoffShutdownAbandonsCleanly(t *testing.T) {
fail := &alwaysFailNotifier{}
svc := createFailingTestService(t, fail)
svc.retryBaseDelay = 5 * time.Second // long enough that shutdown lands mid-wait
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := svc.Start(ctx); err != nil {
t.Fatalf("Failed to start service: %v", err)
}
notification := &domain.Notification{
ID: "backoff-shutdown-1",
Type: domain.TypeStdout,
Status: domain.StatusPending,
Recipients: []string{"test@example.com"},
MaxRetries: 3,
}
if _, err := svc.Send(ctx, notification); err != nil {
t.Fatalf("Send failed: %v", err)
}
// Give the worker time to make its first (failing) attempt and enter the
// backoff wait.
deadline := time.Now().Add(2 * time.Second)
for time.Now().Before(deadline) && len(fail.callTimes()) == 0 {
time.Sleep(10 * time.Millisecond)
}
if len(fail.callTimes()) == 0 {
t.Fatal("notifier was never called; cannot test shutdown mid-backoff")
}
stopped := make(chan error, 1)
go func() {
stopped <- svc.Stop()
}()
select {
case err := <-stopped:
if err != nil {
t.Errorf("Stop returned error: %v", err)
}
case <-time.After(3 * time.Second):
t.Fatal("Stop did not return promptly - possible goroutine leak in retry backoff")
}
n, err := svc.GetNotification(context.Background(), notification.ID)
if err != nil {
t.Fatalf("GetNotification failed after shutdown: %v", err)
}
if n.Status != domain.StatusFailed {
t.Errorf("expected notification to be marked Failed after shutdown abandon, got %s", n.Status)
}
// Only the one attempt made before shutdown should have happened; the
// pending retry must not have fired.
if calls := len(fail.callTimes()); calls != 1 {
t.Errorf("expected exactly 1 send attempt before shutdown abandoned the retry, got %d", calls)
}
}