diff --git a/docs/self-hosting/notifications.md b/docs/self-hosting/notifications.md index e0416e86c..65b7e1fa1 100644 --- a/docs/self-hosting/notifications.md +++ b/docs/self-hosting/notifications.md @@ -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 diff --git a/src/app/api/settings/__tests__/private-origin-routes.test.ts b/src/app/api/settings/__tests__/private-origin-routes.test.ts index 7825c97b3..edcc96bd4 100644 --- a/src/app/api/settings/__tests__/private-origin-routes.test.ts +++ b/src/app/api/settings/__tests__/private-origin-routes.test.ts @@ -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" }), + ); }, ); diff --git a/src/lib/logging/__tests__/event-builder.test.ts b/src/lib/logging/__tests__/event-builder.test.ts index ae12c5227..2ff65ce65 100644 --- a/src/lib/logging/__tests__/event-builder.test.ts +++ b/src/lib/logging/__tests__/event-builder.test.ts @@ -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"); + }); +}); diff --git a/src/lib/logging/event-builder.ts b/src/lib/logging/event-builder.ts index 439d65b5b..9a89a6dd4 100644 --- a/src/lib/logging/event-builder.ts +++ b/src/lib/logging/event-builder.ts @@ -43,6 +43,7 @@ export class WideEventBuilder { private event: Partial; 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; diff --git a/src/lib/notifications/test-delivery-failure.ts b/src/lib/notifications/test-delivery-failure.ts index 1728f1678..4612f1d72 100644 --- a/src/lib/notifications/test-delivery-failure.ts +++ b/src/lib/notifications/test-delivery-failure.ts @@ -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 }); }