mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Merge remote-tracking branch 'origin/main' into claude/telemetry-test-version-pings
# Conflicts: # docs/release-control/v6/internal/status.json
This commit is contained in:
Richard Courtman
@@ -10129,9 +10129,47 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "notification-delivery-terminal-failure-diagnosis",
|
||||
"summary": "Terminal notification failures are 36% of resolved delivery outcomes fleet-wide (126,337 dead-lettered against 224,692 delivered, week to 2026-09-03, 6,668 clean installs), and the failure breakdown could not explain why. Two causes, one fixed here. First, the delivery failure class was inferred from Go error prose, so any failure whose text carried no recognised token fell to unknown (31,218, the modal bucket) and the destination's own response body could steer the class, letting a 500 whose body says 'rate limit' be recorded as rate_limited; senders now declare the class and SMTP reply codes, DNS, x509 and timeout errors classify structurally. Second, the shape is concentration, not breadth: only 400 of 6,668 installs report any failure, the top 10 carry 73,276 (58%), the median failing install has 13, and 72 installs that delivered nothing at all in seven days account for 62,615 failures (50%). Pulse retries a permanently broken destination three times per notification forever and never tells the operator the destination has never once succeeded, so the worst install burned 20,312 dead letters against 36 deliveries. Still open: back off or disable a destination with no successful delivery, surface that state on the destinations surface, and add server_error guidance to the delivery health card, which currently falls through to the unknown copy for a class the client already emits.",
|
||||
"owner": "project-owner",
|
||||
"status": "triaged",
|
||||
"recorded_at": "2026-09-03",
|
||||
"lane_ids": [
|
||||
"L6"
|
||||
],
|
||||
"subsystem_ids": [
|
||||
"alerts",
|
||||
"notifications"
|
||||
],
|
||||
"proposed_resolution": "lane-expansion",
|
||||
"coverage_impact": 3,
|
||||
"evidence": [
|
||||
{
|
||||
"repo": "pulse",
|
||||
"path": "frontend-modern/src/utils/alertDestinationsPresentation.ts",
|
||||
"kind": "file"
|
||||
},
|
||||
{
|
||||
"repo": "pulse",
|
||||
"path": "internal/notifications/failure_class.go",
|
||||
"kind": "file"
|
||||
},
|
||||
{
|
||||
"repo": "pulse",
|
||||
"path": "internal/notifications/queue.go",
|
||||
"kind": "file"
|
||||
},
|
||||
{
|
||||
"repo": "pulse-pro",
|
||||
"path": "docs/TELEMETRY_SIGNALS.md",
|
||||
"kind": "file"
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "telemetry-test-binary-production-pings",
|
||||
"summary": "Go test binaries reported themselves to the production telemetry receiver as live installations. pkg/server tests boot the real server through Run() with the version literal \"test-version\", which internal/updates normalizes to 0.0.0-test-version, and each test runs against its own t.TempDir(), so every run minted a fresh install ID. The startup ping waits two minutes and never fired in a short test, but the service-health failure reporter added on 2026-08-29 sends synchronously from a deferred handler as soon as Run() returns an error, so every CI shard containing pkg/server posted one ping. The receiver recorded 317 single-ping installs between 2026-08-29 and 2026-09-03, 311 from linux/amd64 CI runners and 3 from a maintainer workstation, still arriving at roughly 60 a day. The canonical clean denominator excludes single-ping installs and was unaffected, but raw install counts and the operator-evidence Patrol blocked-cause read counted them as real installations. Resolved by refusing production-endpoint sends from a test binary in internal/telemetry, opting the server tests and the CI test job out of telemetry, and excluding development builds from the operator-evidence read.",
|
||||
"summary": "Go test binaries reported themselves to the production telemetry receiver as live installations. pkg/server tests boot the real server through Run() with the version literal \"test-version\", which internal/updates normalizes to 0.0.0-test-version, and each test runs against its own t.TempDir(), so every run minted a fresh install ID. The startup ping waits two minutes and never fired in a short test, but the service-health failure reporter added on 2026-08-29 sends synchronously from a deferred handler as soon as Run() returns an error, so every test run that exercised a startup failure posted one ping. The receiver recorded 317 single-ping installs between 2026-08-29 and 2026-09-03, 311 from linux/amd64 hosts (dominated by the autonomous maintainer fleet running ad-hoc go test, not GitHub Actions, which ran twice in the final 24h) and 3 from a maintainer workstation. The canonical clean denominator excludes single-ping installs and was unaffected, but raw install counts and the operator-evidence Patrol blocked-cause read counted them as real installations. Resolved by refusing production-endpoint sends from a test binary in internal/telemetry, opting the server tests out of telemetry, and putting the operator-evidence read on the production-ping basis. Note the receiver cannot filter these on version_is_development: the emitter sets that flag only for git build metadata or a prerelease of exactly dev or dev.*, so 0.0.0-test-version arrives with it clear and only version_is_published_release excludes it.",
|
||||
"owner": "project-owner",
|
||||
"status": "triaged",
|
||||
"recorded_at": "2026-09-03",
|
||||
|
||||
@@ -52,6 +52,7 @@ or displayed a notification.
|
||||
9. `internal/notifications/tag_routing.go`
|
||||
10. `internal/notifications/delivery_health.go`
|
||||
11. `internal/notifications/deadman_config.go`
|
||||
12. `internal/notifications/failure_class.go`
|
||||
|
||||
## Shared Boundaries
|
||||
|
||||
@@ -525,3 +526,29 @@ content and destination identity never leave Pulse.
|
||||
|
||||
`internal/notifications/queue_test.go` pins the local classification order and
|
||||
the terminal-only retry/dead-letter accounting boundary.
|
||||
|
||||
### Delivery failure class is declared by the sender, never read from a response
|
||||
|
||||
The failure class is authoritative where the sender knows it and heuristic only
|
||||
where nobody does. `internal/notifications/failure_class.go` owns that order:
|
||||
a class declared by the sender through `NotificationFailureError` wins, then
|
||||
Go's own error types decide (`*textproto.Error` carries the SMTP reply code,
|
||||
x509 and TLS errors mean `tls`, `net.DNSError` and timeouts mean
|
||||
`connectivity`), and only then is the error message text consulted.
|
||||
|
||||
Two rules follow and are not optional. A destination's response body must never
|
||||
determine its own failure class: every site that builds an error from an HTTP
|
||||
status classifies from the status code, so the body is retained for the
|
||||
operator's audit row but cannot make a 500 that says "rate limit" record as
|
||||
`rate_limited`. And an SMTP reply code is not read like an HTTP status: a 5xx
|
||||
reply is the destination refusing the message (`rejected`, or `authentication`
|
||||
for 530/534/535/538 and `configuration` for the 500-504 syntax replies), while
|
||||
only the transient 4xx replies are `server_error`.
|
||||
|
||||
`RecordAuditError` is the canonical audit entry point because it is the one
|
||||
that preserves a declared class; `RecordAudit` re-derives from text and is
|
||||
retained only for callers that never had the error value.
|
||||
|
||||
`internal/notifications/failure_class_test.go` pins the precedence order, the
|
||||
SMTP reply-code mapping, and the rule that response-body text cannot steer the
|
||||
recorded class.
|
||||
|
||||
@@ -55,7 +55,7 @@ func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) {
|
||||
case "password:":
|
||||
return []byte(a.password), nil
|
||||
default:
|
||||
return nil, fmt.Errorf("unexpected LOGIN prompt: %s", fromServer)
|
||||
return nil, FailfWithClass(NotificationFailureAuthentication, "unexpected LOGIN prompt: %s", fromServer)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,7 +75,7 @@ func (a *plainAuth) Start(server *smtp.ServerInfo) (string, []byte, error) {
|
||||
|
||||
func (a *plainAuth) Next(fromServer []byte, more bool) ([]byte, error) {
|
||||
if more {
|
||||
return nil, fmt.Errorf("unexpected server challenge")
|
||||
return nil, FailfWithClass(NotificationFailureAuthentication, "unexpected server challenge")
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
@@ -95,14 +95,14 @@ type resolvedEmailAddresses struct {
|
||||
func resolveEmailAddress(fieldName, value string) (*mail.Address, error) {
|
||||
addr, err := mail.ParseAddress(strings.TrimSpace(value))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("invalid %s address %q: %w", fieldName, value, err)
|
||||
return nil, FailfWithClass(NotificationFailureConfiguration, "invalid %s address %q: %w", fieldName, value, err)
|
||||
}
|
||||
return addr, nil
|
||||
}
|
||||
|
||||
func resolveRecipientAddresses(values []string) ([]*mail.Address, error) {
|
||||
if len(values) == 0 {
|
||||
return nil, fmt.Errorf("at least one recipient address is required")
|
||||
return nil, FailfWithClass(NotificationFailureConfiguration, "at least one recipient address is required")
|
||||
}
|
||||
|
||||
resolved := make([]*mail.Address, 0, len(values))
|
||||
|
||||
@@ -0,0 +1,172 @@
|
||||
package notifications
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"crypto/x509"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/textproto"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
// Delivery failure classification is authoritative where the sender knows the
|
||||
// answer and heuristic only where it does not.
|
||||
//
|
||||
// The original classifier read the class out of the error prose. That is wrong
|
||||
// in two ways that both showed up in fleet telemetry. It cannot see failures
|
||||
// whose text carries no recognised token, which is why `unknown` became the
|
||||
// modal bucket; and the prose it reads includes the destination's own response
|
||||
// body, so a third party can steer the reason code by putting "rate limit" or
|
||||
// "unauthorized" in the body of a 500. A declared class removes both: the
|
||||
// sender states what happened, and the text is only consulted when nobody did.
|
||||
|
||||
// NotificationFailureError carries the delivery failure class its sender
|
||||
// already knew, so the class never has to be inferred from the message.
|
||||
type NotificationFailureError struct {
|
||||
Class NotificationFailureClass
|
||||
Err error
|
||||
}
|
||||
|
||||
func (e *NotificationFailureError) Error() string {
|
||||
if e == nil {
|
||||
return ""
|
||||
}
|
||||
if e.Err == nil {
|
||||
return string(e.Class)
|
||||
}
|
||||
return e.Err.Error()
|
||||
}
|
||||
|
||||
func (e *NotificationFailureError) Unwrap() error {
|
||||
if e == nil {
|
||||
return nil
|
||||
}
|
||||
return e.Err
|
||||
}
|
||||
|
||||
// FailWithClass labels err with an authoritative delivery failure class.
|
||||
// A nil err yields nil so callers can wrap unconditionally.
|
||||
func FailWithClass(class NotificationFailureClass, err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
return &NotificationFailureError{Class: class, Err: err}
|
||||
}
|
||||
|
||||
// FailfWithClass builds a labelled failure from a format string. The format
|
||||
// arguments may include destination response text; that text is preserved for
|
||||
// the operator-facing audit row but can no longer influence the class.
|
||||
func FailfWithClass(class NotificationFailureClass, format string, args ...any) error {
|
||||
return &NotificationFailureError{Class: class, Err: fmt.Errorf(format, args...)}
|
||||
}
|
||||
|
||||
// ClassFromHTTPStatus maps an HTTP response status to its delivery class.
|
||||
// Status codes are the destination's own structured verdict, so they outrank
|
||||
// anything the response body happens to say.
|
||||
func ClassFromHTTPStatus(status int) NotificationFailureClass {
|
||||
switch status {
|
||||
case 401, 403, 407:
|
||||
return NotificationFailureAuthentication
|
||||
case 402:
|
||||
return NotificationFailureConfiguration
|
||||
case 408:
|
||||
return NotificationFailureConnectivity
|
||||
case 429:
|
||||
return NotificationFailureRateLimited
|
||||
}
|
||||
switch {
|
||||
case status >= 500 && status <= 599:
|
||||
return NotificationFailureServerError
|
||||
case status >= 400 && status <= 499:
|
||||
return NotificationFailureRejected
|
||||
default:
|
||||
return NotificationFailureUnknown
|
||||
}
|
||||
}
|
||||
|
||||
// ClassFromSMTPCode maps an SMTP reply code to its delivery class.
|
||||
//
|
||||
// SMTP 5xx is not the HTTP 5xx meaning: a 550 is the destination refusing the
|
||||
// message, not the destination breaking. Only the transient 4xx replies are a
|
||||
// server-side fault, and the 5xx range splits between our credentials, our
|
||||
// command, and the recipient.
|
||||
func ClassFromSMTPCode(code int) NotificationFailureClass {
|
||||
switch code {
|
||||
case 530, 534, 535, 538:
|
||||
return NotificationFailureAuthentication
|
||||
case 500, 501, 502, 503, 504:
|
||||
// Syntax, bad sequence, or an unimplemented command: the session Pulse
|
||||
// built does not match what this server accepts.
|
||||
return NotificationFailureConfiguration
|
||||
}
|
||||
switch {
|
||||
case code >= 400 && code <= 499:
|
||||
// Transient negative completion. The server responded and asked for
|
||||
// this to be tried again later.
|
||||
return NotificationFailureServerError
|
||||
case code >= 500 && code <= 599:
|
||||
return NotificationFailureRejected
|
||||
default:
|
||||
return NotificationFailureUnknown
|
||||
}
|
||||
}
|
||||
|
||||
// ClassifyNotificationFailureError determines the delivery failure class for a
|
||||
// send error. It prefers what the sender declared, then what Go's own error
|
||||
// types prove, and only then falls back to reading the message text.
|
||||
func ClassifyNotificationFailureError(err error) NotificationFailureClass {
|
||||
if err == nil {
|
||||
return NotificationFailureUnknown
|
||||
}
|
||||
|
||||
var declared *NotificationFailureError
|
||||
if errors.As(err, &declared) && declared.Class != "" {
|
||||
return declared.Class
|
||||
}
|
||||
|
||||
// An SMTP reply code is the mail server's structured verdict. net/smtp
|
||||
// surfaces it as *textproto.Error however deeply it is wrapped.
|
||||
var protoErr *textproto.Error
|
||||
if errors.As(err, &protoErr) {
|
||||
return ClassFromSMTPCode(protoErr.Code)
|
||||
}
|
||||
|
||||
// Certificate and handshake failures, before the generic network checks:
|
||||
// a verification failure is also reachable through a dial.
|
||||
var certVerifyErr *tls.CertificateVerificationError
|
||||
var unknownAuthorityErr x509.UnknownAuthorityError
|
||||
var hostnameErr x509.HostnameError
|
||||
var certInvalidErr x509.CertificateInvalidError
|
||||
var recordHeaderErr tls.RecordHeaderError
|
||||
if errors.As(err, &certVerifyErr) ||
|
||||
errors.As(err, &unknownAuthorityErr) ||
|
||||
errors.As(err, &hostnameErr) ||
|
||||
errors.As(err, &certInvalidErr) ||
|
||||
errors.As(err, &recordHeaderErr) {
|
||||
return NotificationFailureTLS
|
||||
}
|
||||
|
||||
var dnsErr *net.DNSError
|
||||
if errors.As(err, &dnsErr) {
|
||||
return NotificationFailureConnectivity
|
||||
}
|
||||
|
||||
if errors.Is(err, context.DeadlineExceeded) ||
|
||||
errors.Is(err, syscall.ECONNREFUSED) ||
|
||||
errors.Is(err, syscall.ECONNRESET) ||
|
||||
errors.Is(err, syscall.EHOSTUNREACH) ||
|
||||
errors.Is(err, syscall.ENETUNREACH) ||
|
||||
errors.Is(err, syscall.ETIMEDOUT) ||
|
||||
errors.Is(err, syscall.EPIPE) {
|
||||
return NotificationFailureConnectivity
|
||||
}
|
||||
|
||||
var netErr net.Error
|
||||
if errors.As(err, &netErr) && netErr.Timeout() {
|
||||
return NotificationFailureConnectivity
|
||||
}
|
||||
|
||||
return ClassifyNotificationFailure(err.Error())
|
||||
}
|
||||
@@ -0,0 +1,242 @@
|
||||
package notifications
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/x509"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/textproto"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestClassFromHTTPStatus(t *testing.T) {
|
||||
cases := map[int]NotificationFailureClass{
|
||||
401: NotificationFailureAuthentication,
|
||||
403: NotificationFailureAuthentication,
|
||||
407: NotificationFailureAuthentication,
|
||||
402: NotificationFailureConfiguration,
|
||||
408: NotificationFailureConnectivity,
|
||||
429: NotificationFailureRateLimited,
|
||||
400: NotificationFailureRejected,
|
||||
404: NotificationFailureRejected,
|
||||
422: NotificationFailureRejected,
|
||||
500: NotificationFailureServerError,
|
||||
502: NotificationFailureServerError,
|
||||
503: NotificationFailureServerError,
|
||||
200: NotificationFailureUnknown,
|
||||
}
|
||||
for status, want := range cases {
|
||||
if got := ClassFromHTTPStatus(status); got != want {
|
||||
t.Errorf("ClassFromHTTPStatus(%d) = %q, want %q", status, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassFromSMTPCode(t *testing.T) {
|
||||
// SMTP 5xx is a refusal, not a server fault: only the transient 4xx range
|
||||
// means the destination broke.
|
||||
cases := map[int]NotificationFailureClass{
|
||||
421: NotificationFailureServerError,
|
||||
450: NotificationFailureServerError,
|
||||
451: NotificationFailureServerError,
|
||||
452: NotificationFailureServerError,
|
||||
530: NotificationFailureAuthentication,
|
||||
535: NotificationFailureAuthentication,
|
||||
538: NotificationFailureAuthentication,
|
||||
500: NotificationFailureConfiguration,
|
||||
501: NotificationFailureConfiguration,
|
||||
503: NotificationFailureConfiguration,
|
||||
550: NotificationFailureRejected,
|
||||
552: NotificationFailureRejected,
|
||||
554: NotificationFailureRejected,
|
||||
250: NotificationFailureUnknown,
|
||||
}
|
||||
for code, want := range cases {
|
||||
if got := ClassFromSMTPCode(code); got != want {
|
||||
t.Errorf("ClassFromSMTPCode(%d) = %q, want %q", code, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A destination controls its own response body. It must not be able to choose
|
||||
// the reason code Pulse records and shows the operator.
|
||||
func TestClassifyNotificationFailureError_ResponseBodyCannotSteerClass(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
status int
|
||||
body string
|
||||
want NotificationFailureClass
|
||||
}{
|
||||
{"server error body claiming rate limit", 500, `{"error":"rate limit exceeded"}`, NotificationFailureServerError},
|
||||
{"server error body claiming unauthorized", 503, `unauthorized`, NotificationFailureServerError},
|
||||
{"rejection body mentioning certificate", 404, `no certificate route found`, NotificationFailureRejected},
|
||||
{"rejection body mentioning connection refused", 400, `upstream connection refused`, NotificationFailureRejected},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := FailfWithClass(
|
||||
ClassFromHTTPStatus(tc.status),
|
||||
"webhook returned HTTP %d: %s", tc.status, tc.body,
|
||||
)
|
||||
if got := ClassifyNotificationFailureError(err); got != tc.want {
|
||||
t.Errorf("class = %q, want %q", got, tc.want)
|
||||
}
|
||||
// The prose classifier is the one that gets this wrong; that is
|
||||
// precisely why the declared class has to win.
|
||||
if prose := ClassifyNotificationFailure(err.Error()); prose == tc.want {
|
||||
t.Logf("prose classifier happened to agree for %q", tc.name)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyNotificationFailureError_DeclaredClassSurvivesWrapping(t *testing.T) {
|
||||
base := FailWithClass(NotificationFailureConfiguration, errors.New("no Apprise targets configured for CLI delivery"))
|
||||
wrapped := fmt.Errorf("apprise CLI send failed: %w", base)
|
||||
if got := ClassifyNotificationFailureError(wrapped); got != NotificationFailureConfiguration {
|
||||
t.Fatalf("class = %q, want %q", got, NotificationFailureConfiguration)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyNotificationFailureError_SMTPReplyCode(t *testing.T) {
|
||||
// net/smtp surfaces the server's reply as *textproto.Error. Before this
|
||||
// path existed every one of these landed in the unknown bucket.
|
||||
cases := []struct {
|
||||
code int
|
||||
msg string
|
||||
want NotificationFailureClass
|
||||
}{
|
||||
{550, "5.7.1 Relay access denied", NotificationFailureRejected},
|
||||
{535, "5.7.8 Authentication credentials invalid", NotificationFailureAuthentication},
|
||||
{451, "4.3.0 Temporary local problem", NotificationFailureServerError},
|
||||
{501, "5.5.4 Syntax error in parameters", NotificationFailureConfiguration},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
err := fmt.Errorf("failed to send email: %w", &textproto.Error{Code: tc.code, Msg: tc.msg})
|
||||
if got := ClassifyNotificationFailureError(err); got != tc.want {
|
||||
t.Errorf("SMTP %d: class = %q, want %q", tc.code, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyNotificationFailureError_StructuralNetworkErrors(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
want NotificationFailureClass
|
||||
}{
|
||||
{
|
||||
name: "dns failure",
|
||||
err: fmt.Errorf("post webhook: %w", &net.DNSError{Err: "server misbehaving", Name: "hooks.example"}),
|
||||
want: NotificationFailureConnectivity,
|
||||
},
|
||||
{
|
||||
name: "context deadline",
|
||||
err: fmt.Errorf("post webhook: %w", context.DeadlineExceeded),
|
||||
want: NotificationFailureConnectivity,
|
||||
},
|
||||
{
|
||||
name: "unknown certificate authority",
|
||||
err: fmt.Errorf("post webhook: %w", x509.UnknownAuthorityError{}),
|
||||
want: NotificationFailureTLS,
|
||||
},
|
||||
{
|
||||
name: "certificate hostname mismatch",
|
||||
err: fmt.Errorf("post webhook: %w", x509.HostnameError{Host: "hooks.example"}),
|
||||
want: NotificationFailureTLS,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := ClassifyNotificationFailureError(tc.err); got != tc.want {
|
||||
t.Errorf("class = %q, want %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyNotificationFailureError_FallsBackToProse(t *testing.T) {
|
||||
err := errors.New("webhook returned HTTP 429 Too Many Requests")
|
||||
if got := ClassifyNotificationFailureError(err); got != NotificationFailureRateLimited {
|
||||
t.Fatalf("class = %q, want %q", got, NotificationFailureRateLimited)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyNotificationFailureError_NilIsUnknown(t *testing.T) {
|
||||
if got := ClassifyNotificationFailureError(nil); got != NotificationFailureUnknown {
|
||||
t.Fatalf("class = %q, want %q", got, NotificationFailureUnknown)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFailWithClassPreservesMessageAndUnwrap(t *testing.T) {
|
||||
cause := errors.New("dial tcp 10.0.0.1:465: connect: connection refused")
|
||||
err := FailWithClass(NotificationFailureConnectivity, cause)
|
||||
if err.Error() != cause.Error() {
|
||||
t.Errorf("Error() = %q, want %q", err.Error(), cause.Error())
|
||||
}
|
||||
if !errors.Is(err, cause) {
|
||||
t.Error("wrapped error does not unwrap to its cause")
|
||||
}
|
||||
if FailWithClass(NotificationFailureConnectivity, nil) != nil {
|
||||
t.Error("FailWithClass(nil) should be nil")
|
||||
}
|
||||
}
|
||||
|
||||
// The declared class has to survive all the way into the audit row that feeds
|
||||
// both the operator's delivery health card and the telemetry counters.
|
||||
func TestRecordAuditErrorPersistsDeclaredClass(t *testing.T) {
|
||||
nq, err := NewNotificationQueue(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatalf("NewNotificationQueue: %v", err)
|
||||
}
|
||||
defer func() { _ = nq.Stop() }()
|
||||
|
||||
now := time.Now().UTC()
|
||||
entries := []*QueuedNotification{
|
||||
{ID: "body-says-rate-limit", Type: "webhook", Status: QueueStatusDLQ, Attempts: 3, Config: []byte(`{}`), CreatedAt: now},
|
||||
{ID: "smtp-refusal", Type: "email", Status: QueueStatusDLQ, Attempts: 3, Config: []byte(`{}`), CreatedAt: now},
|
||||
}
|
||||
for _, entry := range entries {
|
||||
status := entry.Status
|
||||
entry.Status = QueueStatusPending
|
||||
if err := nq.Enqueue(entry); err != nil {
|
||||
t.Fatalf("enqueue %s: %v", entry.ID, err)
|
||||
}
|
||||
entry.Status = status
|
||||
}
|
||||
|
||||
// A 500 whose body says "rate limit" is a server error, not rate limiting.
|
||||
bodySteered := FailfWithClass(
|
||||
ClassFromHTTPStatus(500),
|
||||
"webhook returned HTTP %d: %s", 500, `{"error":"rate limit exceeded"}`,
|
||||
)
|
||||
if err := nq.RecordAuditError(entries[0], false, bodySteered); err != nil {
|
||||
t.Fatalf("record body-steered audit: %v", err)
|
||||
}
|
||||
smtpRefusal := fmt.Errorf("failed to send email: %w", &textproto.Error{Code: 550, Msg: "5.7.1 Relay access denied"})
|
||||
if err := nq.RecordAuditError(entries[1], false, smtpRefusal); err != nil {
|
||||
t.Fatalf("record smtp audit: %v", err)
|
||||
}
|
||||
|
||||
stats, err := nq.GetTelemetryStats(now.Add(-time.Hour))
|
||||
if err != nil {
|
||||
t.Fatalf("GetTelemetryStats: %v", err)
|
||||
}
|
||||
if stats.Failures != 2 {
|
||||
t.Fatalf("failures = %d, want 2", stats.Failures)
|
||||
}
|
||||
if stats.FailureClasses.ServerError != 1 {
|
||||
t.Errorf("server_error = %d, want 1", stats.FailureClasses.ServerError)
|
||||
}
|
||||
if stats.FailureClasses.Rejected != 1 {
|
||||
t.Errorf("rejected = %d, want 1 (SMTP 550 is a refusal)", stats.FailureClasses.Rejected)
|
||||
}
|
||||
if stats.FailureClasses.RateLimited != 0 {
|
||||
t.Errorf("rate_limited = %d, want 0: the response body must not set the class", stats.FailureClasses.RateLimited)
|
||||
}
|
||||
if stats.FailureClasses.Unknown != 0 {
|
||||
t.Errorf("unknown = %d, want 0", stats.FailureClasses.Unknown)
|
||||
}
|
||||
}
|
||||
@@ -1695,7 +1695,7 @@ func configJSONForNotificationDeliveryJob(job notificationDeliveryJob) ([]byte,
|
||||
return json.Marshal(*job.WebhookConfig)
|
||||
case "apprise":
|
||||
if job.AppriseConfig == nil {
|
||||
return nil, fmt.Errorf("missing apprise config")
|
||||
return nil, FailfWithClass(NotificationFailureConfiguration, "missing apprise config")
|
||||
}
|
||||
return json.Marshal(*job.AppriseConfig)
|
||||
default:
|
||||
@@ -1853,7 +1853,7 @@ func (n *NotificationManager) deliverNotificationJob(job notificationDeliveryJob
|
||||
return n.sendGroupedWebhook(*job.WebhookConfig, job.Alerts)
|
||||
case "apprise":
|
||||
if job.AppriseConfig == nil {
|
||||
return fmt.Errorf("missing apprise config")
|
||||
return FailfWithClass(NotificationFailureConfiguration, "missing apprise config")
|
||||
}
|
||||
if job.Event == eventResolved {
|
||||
return n.sendResolvedApprise(*job.AppriseConfig, job.Alerts, job.ResolvedAt)
|
||||
@@ -1919,7 +1919,7 @@ func (n *NotificationManager) sendGroupedApprise(config AppriseConfig, alertList
|
||||
|
||||
cfg := NormalizeAppriseConfig(config)
|
||||
if !cfg.Enabled {
|
||||
return fmt.Errorf("apprise not enabled")
|
||||
return FailfWithClass(NotificationFailureConfiguration, "apprise not enabled")
|
||||
}
|
||||
|
||||
title, body, notifyType := buildApprisePayload(alertList, n.publicURL)
|
||||
@@ -2089,7 +2089,7 @@ func resolveAppriseNotificationType(alertList []*alerts.Alert) string {
|
||||
|
||||
func (n *NotificationManager) sendAppriseViaCLI(cfg AppriseConfig, title, body string) error {
|
||||
if len(cfg.Targets) == 0 {
|
||||
return fmt.Errorf("no Apprise targets configured for CLI delivery")
|
||||
return FailfWithClass(NotificationFailureConfiguration, "no Apprise targets configured for CLI delivery")
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(cfg.TimeoutSeconds)*time.Second)
|
||||
@@ -2216,9 +2216,9 @@ func (n *NotificationManager) sendAppriseViaHTTP(cfg AppriseConfig, title, body,
|
||||
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
if len(respBody) > 0 {
|
||||
return fmt.Errorf("apprise server returned HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(respBody)))
|
||||
return FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "apprise server returned HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(respBody)))
|
||||
}
|
||||
return fmt.Errorf("apprise server returned HTTP %d", resp.StatusCode)
|
||||
return FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "apprise server returned HTTP %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
if len(respBody) > 0 {
|
||||
@@ -2239,7 +2239,7 @@ func (n *NotificationManager) sendResolvedApprise(config AppriseConfig, alertLis
|
||||
|
||||
cfg := NormalizeAppriseConfig(config)
|
||||
if !cfg.Enabled {
|
||||
return fmt.Errorf("apprise not enabled")
|
||||
return FailfWithClass(NotificationFailureConfiguration, "apprise not enabled")
|
||||
}
|
||||
|
||||
title, _, body := buildResolvedNotificationContent(alertList, resolvedAt, n.publicURL)
|
||||
@@ -2813,7 +2813,7 @@ func (n *NotificationManager) sendResolvedWebhookNtfy(webhook WebhookConfig, ale
|
||||
Int("status", resp.StatusCode).
|
||||
Str("response", respBody.String()).
|
||||
Msg("resolved ntfy webhook returned non-success status")
|
||||
return fmt.Errorf("ntfy webhook returned HTTP %d: %s", resp.StatusCode, respBody.String())
|
||||
return FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "ntfy webhook returned HTTP %d: %s", resp.StatusCode, respBody.String())
|
||||
}
|
||||
|
||||
// checkWebhookRateLimit checks if a webhook can be sent based on rate limits
|
||||
@@ -2981,7 +2981,7 @@ func (n *NotificationManager) executeWebhookRequest(webhook WebhookConfig, paylo
|
||||
}
|
||||
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
return result, fmt.Errorf("webhook returned HTTP %d: %s", resp.StatusCode, result.body)
|
||||
return result, FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "webhook returned HTTP %d: %s", resp.StatusCode, result.body)
|
||||
}
|
||||
|
||||
return result, nil
|
||||
@@ -3049,7 +3049,7 @@ func (n *NotificationManager) sendWebhookRequest(webhook WebhookConfig, jsonData
|
||||
Int("status", result.statusCode).
|
||||
Str("response", result.body).
|
||||
Msg("webhook returned non-success status")
|
||||
return fmt.Errorf("webhook returned HTTP %d: %s", result.statusCode, result.body)
|
||||
return FailfWithClass(ClassFromHTTPStatus(result.statusCode), "webhook returned HTTP %d: %s", result.statusCode, result.body)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1456,8 +1456,34 @@ func (nq *NotificationQueue) notifyDeliveryHealthChanged() {
|
||||
}
|
||||
}
|
||||
|
||||
// RecordAudit records a notification delivery attempt in the audit log
|
||||
// RecordAudit records a notification delivery attempt in the audit log,
|
||||
// deriving the failure class from the error text. Prefer RecordAuditError,
|
||||
// which keeps the class the sender declared.
|
||||
func (nq *NotificationQueue) RecordAudit(notif *QueuedNotification, success bool, errorMsg string) error {
|
||||
failureClass := NotificationFailureClass("")
|
||||
if !success {
|
||||
failureClass = ClassifyNotificationFailure(errorMsg)
|
||||
}
|
||||
return nq.recordAudit(notif, success, errorMsg, failureClass)
|
||||
}
|
||||
|
||||
// RecordAuditError records a delivery attempt from the send error itself, so a
|
||||
// class the sender declared reaches the audit row instead of being re-derived
|
||||
// from the message. This is what keeps a destination's response body out of the
|
||||
// classification decision.
|
||||
func (nq *NotificationQueue) RecordAuditError(notif *QueuedNotification, success bool, sendErr error) error {
|
||||
errorMsg := ""
|
||||
failureClass := NotificationFailureClass("")
|
||||
if !success {
|
||||
if sendErr != nil {
|
||||
errorMsg = sendErr.Error()
|
||||
}
|
||||
failureClass = ClassifyNotificationFailureError(sendErr)
|
||||
}
|
||||
return nq.recordAudit(notif, success, errorMsg, failureClass)
|
||||
}
|
||||
|
||||
func (nq *NotificationQueue) recordAudit(notif *QueuedNotification, success bool, errorMsg string, failureClass NotificationFailureClass) error {
|
||||
nq.mu.Lock()
|
||||
defer nq.mu.Unlock()
|
||||
|
||||
@@ -1477,10 +1503,6 @@ func (nq *NotificationQueue) RecordAudit(notif *QueuedNotification, success bool
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`
|
||||
|
||||
failureClass := ""
|
||||
if !success {
|
||||
failureClass = string(ClassifyNotificationFailure(errorMsg))
|
||||
}
|
||||
_, err = nq.db.Exec(query,
|
||||
notif.ID,
|
||||
notif.Type,
|
||||
@@ -1492,7 +1514,7 @@ func (nq *NotificationQueue) RecordAudit(notif *QueuedNotification, success bool
|
||||
notif.Attempts,
|
||||
success,
|
||||
errorMsg,
|
||||
failureClass,
|
||||
string(failureClass),
|
||||
strings.TrimSpace(notif.DestinationID),
|
||||
notif.PayloadBytes,
|
||||
time.Now().Unix(),
|
||||
@@ -1853,7 +1875,7 @@ func (nq *NotificationQueue) processNotification(notif *QueuedNotification) {
|
||||
} else {
|
||||
notif.Links = persistedLinks
|
||||
}
|
||||
if auditErr := nq.RecordAudit(notif, success, errorMsg); auditErr != nil {
|
||||
if auditErr := nq.RecordAuditError(notif, success, err); auditErr != nil {
|
||||
log.Error().
|
||||
Err(auditErr).
|
||||
Str("component", "notification_queue").
|
||||
|
||||
Reference in New Issue
Block a user