fix(notifications): record a relay refusal at warn, not error

A 502 from a channel test reports the user's relay or mail setup, not a fault on this server, yet the status alone raised every press to an error-level event. The event builder gains a ceiling for the status-derived level, and the test routes cap it at warn on the 502 path. The 500 for an unnamed failure stays at error, and explicit elevations are unaffected.

The notifications guide notes that a reverse proxy which replaces 5xx pages hides the detail, so the card falls back to Test failed.

Refs #947
This commit is contained in:
Marc-André Bombeck committed 2026-09-13 14:50:32 +02:00
1 parent f89dda81a6
commit 6a663dfc27
5 files changed
+78 -5

No files matched your search

+7
View File
@@ -274,6 +274,13 @@ short, Gotify's own error text. A `401` means the token is wrong. A `400`
means Gotify could not read the body; check that the format is set to
Gotify. A Gotify on your own network also needs the operator grant below.
The detail reaches the card inside the server's `502` response. A reverse
proxy that replaces error pages for 5xx responses, such as Traefik's
`errors` middleware or nginx with `proxy_intercept_errors on`, swaps that
response for its own page, and the card can then only say "Test failed".
Exclude the `/api/` paths from the error-page rule to keep the detail. This
applies to the ntfy and email tests too.
## Notification targets on a private network
Every webhook and ntfy target is resolved at send time and refused when
@@ -52,7 +52,10 @@ vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
vi.mock("@/lib/auth/audit", () => ({
auditLog: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
const emitIfSampledMock = vi.fn();
vi.mock("@/lib/logging/transports", () => ({
emitIfSampled: (...args: unknown[]) => emitIfSampledMock(...args),
}));
vi.mock("@/lib/db-compat", () => ({
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
}));
@@ -374,6 +377,10 @@ describe("POST /api/settings/{webhook,ntfy}/test — the refusal names itself",
upstreamBody: '{"error":"Bad Request","errorCode":400}',
});
expect(json.error).toContain("HTTP 400");
// A user's relay refusing is not an operator error: the event is warn.
expect(emitIfSampledMock).toHaveBeenCalledWith(
expect.objectContaining({ level: "warn" }),
);
},
);
@@ -444,6 +451,9 @@ describe("POST /api/settings/{webhook,ntfy}/test — the refusal names itself",
expect(response.status).toBe(500);
const json = await response.json();
expect(json.meta).toBeUndefined();
expect(emitIfSampledMock).toHaveBeenCalledWith(
expect.objectContaining({ level: "error" }),
);
},
);
@@ -110,3 +110,33 @@ describe("WideEventBuilder.setHttp", () => {
expect(evt.toJSON().http).toBeUndefined();
});
});
describe("WideEventBuilder.capStatusLevel", () => {
it("derives error from a 5xx by default", () => {
const evt = new WideEventBuilder("http");
evt.finish(502);
expect(evt.toJSON().level).toBe("error");
});
it("keeps a capped 5xx at warn", () => {
const evt = new WideEventBuilder("http");
evt.capStatusLevel("warn");
evt.finish(502);
expect(evt.toJSON().level).toBe("warn");
});
it("does not lower an explicit elevation", () => {
const evt = new WideEventBuilder("http");
evt.capStatusLevel("warn");
evt.elevateLevel("error");
evt.finish(502);
expect(evt.toJSON().level).toBe("error");
});
it("leaves a 4xx at warn under the cap", () => {
const evt = new WideEventBuilder("http");
evt.capStatusLevel("warn");
evt.finish(422);
expect(evt.toJSON().level).toBe("warn");
});
});
+23 -2
View File
@@ -43,6 +43,7 @@ export class WideEventBuilder {
private event: Partial<WideEvent>;
private startTime: number;
private highestLevel: LogLevel = "info";
private statusLevelCeiling: LogLevel = "error";
constructor(kind: EventKind = "http") {
this.startTime = performance.now();
@@ -239,6 +240,18 @@ export class WideEventBuilder {
return this;
}
/**
* Ceiling for the level `finish()` derives from the HTTP status. A route
* whose 5xx reports someone else's refusal (a user's relay answering a
* test with 400) caps it at `warn`, so a user's configuration does not
* raise an operator-facing error. `elevateLevel` calls and thrown errors
* are unaffected.
*/
capStatusLevel(level: LogLevel): this {
this.statusLevelCeiling = level;
return this;
}
/** Request/Operation beenden, Duration berechnen */
finish(httpStatus?: number): this {
this.event.duration_ms = Math.round(performance.now() - this.startTime);
@@ -256,8 +269,16 @@ export class WideEventBuilder {
this.event.http.status = httpStatus;
}
if (httpStatus !== undefined) {
if (httpStatus >= 500) this.elevateLevel("error");
else if (httpStatus >= 400) this.elevateLevel("warn");
const derived: LogLevel | null =
httpStatus >= 500 ? "error" : httpStatus >= 400 ? "warn" : null;
if (derived) {
this.elevateLevel(
LOG_LEVEL_PRIORITY[derived] >
LOG_LEVEL_PRIORITY[this.statusLevelCeiling]
? this.statusLevelCeiling
: derived,
);
}
}
this.event.level = this.highestLevel;
return this;
@@ -1,5 +1,5 @@
import { apiError } from "@/lib/api-response";
import { annotate } from "@/lib/logging/context";
import { annotate, getEvent } from "@/lib/logging/context";
import type { SendOutcome } from "@/lib/notifications/retry-policy";
/**
@@ -78,7 +78,8 @@ export function testFailureSentence(
* The other end, or the way to it, failed: 502 with `meta.errorCode`, the
* upstream status or SMTP code, and what the relay said when it is short
* and holds no secret. A failure the sender could not name is an internal
* fault and stays a 500 with `fallbackMessage`.
* fault and stays a 500 with `fallbackMessage`. The 502 is recorded at
* `warn`, the 500 at `error`.
*/
export function answerTestDeliveryFailure(
result: SendOutcome,
@@ -105,5 +106,9 @@ export function answerTestDeliveryFailure(
if (!detail.errorCode) {
return apiError(opts.fallbackMessage, 500);
}
// 502 is the honest status: the other end failed, not this request. But
// the cause is the user's relay or mail setup, so the wide event stays at
// warn rather than raising an operator-facing error on every test press.
getEvent()?.capStatusLevel("warn");
return apiError(testFailureSentence(opts.label, detail), 502, { ...detail });
}