mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 22:26:13 +00:00
fix(notifications): rest a channel that had nobody to send to
A send with no browser subscribed, no iPhone paired or a transport the operator has not set up no longer counts as a failure, but nothing else replaced it. The mood reminder and the daily briefing retry every minute until delivered, so such a channel wrote a skipped row each minute, for good; the dispatch still counted as an attempt, so the watchdog and status alerts took it as announced; and the status screen said "active" with no hint why nothing arrived. The channel now rests for an hour after such a send, with the reason on its status, and the send does not count as an attempt. Subscribing a browser or pairing an iPhone ends the rest at once.
This commit is contained in:
1 parent
95e2ba5477
commit
4ff3e4e1b7
7 files changed
+97
-7
No files matched your search
@@ -17,6 +17,7 @@ vi.mock("@/lib/db", () => ({
|
||||
$transaction: vi.fn(),
|
||||
notificationChannel: {
|
||||
upsert: vi.fn(),
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
auditLog: {
|
||||
create: vi.fn(),
|
||||
@@ -89,6 +90,9 @@ beforeEach(() => {
|
||||
vi.mocked(prisma.notificationChannel.upsert).mockResolvedValue({
|
||||
id: "ch-1",
|
||||
} as never);
|
||||
vi.mocked(prisma.notificationChannel.updateMany).mockResolvedValue({
|
||||
count: 0,
|
||||
});
|
||||
vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never);
|
||||
// `resetAllMocks` strips the top-level `.mockResolvedValue(...)`
|
||||
// declaration too, so re-arm the rate-limit success path here.
|
||||
@@ -219,6 +223,29 @@ describe("POST /api/devices", () => {
|
||||
expect(args.data.apnsEnvironment).toBe("sandbox");
|
||||
});
|
||||
|
||||
it("ends a pause that was waiting for a paired iPhone", async () => {
|
||||
// A reminder with no iPhone paired rests the APNs channel for an hour;
|
||||
// pairing one must not leave the next reminder waiting it out.
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
const res = await POST(
|
||||
req({
|
||||
token: "abcd1234efgh5678",
|
||||
bundleId: "io.healthlog.app",
|
||||
apnsToken: "deadbeef".repeat(8),
|
||||
apnsEnvironment: "sandbox",
|
||||
}),
|
||||
);
|
||||
expect(res.status).toBe(201);
|
||||
expect(prisma.notificationChannel.updateMany).toHaveBeenCalledWith({
|
||||
where: {
|
||||
userId: "user-1",
|
||||
type: "APNS",
|
||||
lastFailureReason: "apns_no_devices",
|
||||
},
|
||||
data: { nextRetryAt: null, lastFailureReason: null },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects re-registration of an apnsToken owned by another user with 409", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(prisma.device.findFirst).mockResolvedValue({
|
||||
|
||||
@@ -272,6 +272,15 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
},
|
||||
update: {},
|
||||
});
|
||||
// The iPhone a no-device pause was waiting for: deliver at once.
|
||||
await tx.notificationChannel.updateMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
type: "APNS",
|
||||
lastFailureReason: "apns_no_devices",
|
||||
},
|
||||
data: { nextRetryAt: null, lastFailureReason: null },
|
||||
});
|
||||
}
|
||||
|
||||
return { id: device.id };
|
||||
|
||||
@@ -211,13 +211,15 @@ describe("dispatchNotification — nobody to send to", () => {
|
||||
// Turning push off on the last browser deletes its subscription and leaves
|
||||
// the channel on. Counted as failures, the next five reminders switched the
|
||||
// channel off for good, and subscribing again later did not bring it back.
|
||||
// Not counted either, the jobs that retry until delivered asked again every
|
||||
// minute, and a watchdog read the attempt as an announced alert.
|
||||
it.each([
|
||||
["WEB_PUSH", "web_push_no_subscriptions"],
|
||||
["WEB_PUSH", "web_push_vapid_not_configured"],
|
||||
["APNS", "apns_no_devices"],
|
||||
["APNS", "apns_not_configured"],
|
||||
] as const)(
|
||||
"%s answering %s leaves the channel state alone",
|
||||
"%s answering %s rests the channel without counting a failure or an attempt",
|
||||
async (type, reason) => {
|
||||
vi.mocked(prisma.notificationChannel.findMany).mockResolvedValueOnce([
|
||||
{
|
||||
@@ -241,8 +243,25 @@ describe("dispatchNotification — nobody to send to", () => {
|
||||
});
|
||||
|
||||
expect(sender).toHaveBeenCalledTimes(1);
|
||||
expect(outcome.dispatched).toBe(false);
|
||||
expect(prisma.notificationChannel.update).not.toHaveBeenCalled();
|
||||
expect(outcome).toEqual({
|
||||
dispatched: false,
|
||||
channelsAttempted: 0,
|
||||
channelsSucceeded: 0,
|
||||
});
|
||||
expect(prisma.notificationChannel.update).toHaveBeenCalledTimes(1);
|
||||
const { data } = vi.mocked(prisma.notificationChannel.update).mock
|
||||
.calls[0][0] as { data: Record<string, unknown> };
|
||||
// The reason for the status screen and an hour's rest; the counter and
|
||||
// the switch stay as they were.
|
||||
expect(data).toEqual({
|
||||
lastFailureAt: expect.any(Date),
|
||||
lastFailureReason: reason,
|
||||
nextRetryAt: expect.any(Date),
|
||||
});
|
||||
expect(
|
||||
(data.nextRetryAt as Date).getTime() -
|
||||
(data.lastFailureAt as Date).getTime(),
|
||||
).toBe(60 * 60 * 1000);
|
||||
expect(auditLog).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import {
|
||||
NO_RECIPIENT_COOLDOWN_MS,
|
||||
nextRetryAt,
|
||||
shouldAutoDisableAfterTransient,
|
||||
type SendOutcome,
|
||||
@@ -129,6 +130,26 @@ export async function recordChannelTransientFailure(
|
||||
return { autoDisabled: false, nextRetryAt: retryAt };
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a send that had nobody to go to. Not a failure: the counter stays
|
||||
* where it is. The reason is kept, so the status screen says why nothing
|
||||
* arrives, and the channel rests for `NO_RECIPIENT_COOLDOWN_MS`.
|
||||
*/
|
||||
export async function recordChannelNoRecipient(
|
||||
channel: ChannelRef,
|
||||
reason: string,
|
||||
now: Date = new Date(),
|
||||
): Promise<void> {
|
||||
await prisma.notificationChannel.update({
|
||||
where: { id: channel.id },
|
||||
data: {
|
||||
lastFailureAt: now,
|
||||
lastFailureReason: reason,
|
||||
nextRetryAt: new Date(now.getTime() + NO_RECIPIENT_COOLDOWN_MS),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-enable a channel that was previously auto-disabled. Clears the
|
||||
* disabled-reason + counter and the cooldown so the next dispatch
|
||||
|
||||
@@ -20,6 +20,7 @@ import type { SendOutcome } from "@/lib/notifications/retry-policy";
|
||||
import {
|
||||
isChannelInCooldown,
|
||||
recordChannelHardReject,
|
||||
recordChannelNoRecipient,
|
||||
recordChannelSuccess,
|
||||
recordChannelTransientFailure,
|
||||
} from "@/lib/notifications/channel-state";
|
||||
@@ -49,7 +50,8 @@ import {
|
||||
* not set up. The sender writes a `skipped` ledger row. The channel has not
|
||||
* failed, so these leave its failure counter alone; counted as failures,
|
||||
* turning push off on the last device would switch the channel off for good
|
||||
* after five reminders.
|
||||
* after five reminders. The channel rests for a while instead
|
||||
* (`recordChannelNoRecipient`), and the send does not count as an attempt.
|
||||
*/
|
||||
const NO_RECIPIENT_REASONS: ReadonlySet<string> = new Set([
|
||||
"web_push_no_subscriptions",
|
||||
@@ -74,8 +76,8 @@ const NO_RECIPIENT_REASONS: ReadonlySet<string> = new Set([
|
||||
* 6. Call the appropriate sender — which now returns a `SendOutcome` so
|
||||
* the dispatcher can classify hard rejects (410, blocked-by-user) vs
|
||||
* soft errors (5xx, 429, network) and update channel state accordingly.
|
||||
* A send that had nobody to go to (`NO_RECIPIENT_REASONS`) changes no
|
||||
* channel state.
|
||||
* A send that had nobody to go to (`NO_RECIPIENT_REASONS`) pauses the
|
||||
* channel without counting a failure.
|
||||
*
|
||||
* Also handles legacy Telegram config stored on the User model by
|
||||
* auto-migrating it to a NotificationChannel record on first dispatch.
|
||||
@@ -432,6 +434,10 @@ export async function dispatchNotification(
|
||||
}
|
||||
|
||||
if (outcome.reason && NO_RECIPIENT_REASONS.has(outcome.reason)) {
|
||||
// Nothing went anywhere: a caller that reads an attempt as "the
|
||||
// alert was announced" must not read this one so.
|
||||
channelsAttempted -= 1;
|
||||
await recordChannelNoRecipient(ref, outcome.reason, now);
|
||||
getEvent()?.addMeta(
|
||||
`notification_skip_${channel.type.toLowerCase()}_no_recipient`,
|
||||
outcome.reason,
|
||||
|
||||
@@ -76,6 +76,14 @@ export const BACKOFF_SCHEDULE_MS: readonly number[] = Object.freeze([
|
||||
|
||||
export const MAX_CONSECUTIVE_FAILURES = 5;
|
||||
|
||||
/**
|
||||
* How long a channel rests after a send that had nobody to go to (no
|
||||
* browser subscribed, no iPhone paired, a transport the operator has not set
|
||||
* up). The jobs that retry until delivered would otherwise ask again every
|
||||
* minute. Subscribing or pairing ends the rest at once.
|
||||
*/
|
||||
export const NO_RECIPIENT_COOLDOWN_MS = 60 * 60 * 1_000;
|
||||
|
||||
/**
|
||||
* Given the new `consecutiveFailures` value (1, 2, …) AFTER incrementing,
|
||||
* return the wall-clock time when a retry is allowed. Returns null once the
|
||||
|
||||
@@ -105,7 +105,7 @@ const channelDeliveryStatus = z
|
||||
state: z
|
||||
.enum(["active", "auto_disabled", "manually_disabled", "sending_paused"])
|
||||
.describe(
|
||||
"Server-derived. `auto_disabled` — the dispatcher turned it off after repeated hard rejects (`disabledReason` says why). `manually_disabled` — the user turned it off. `sending_paused` — still enabled but inside a retry cooldown until `nextRetryAt`. `active` — enabled and sending.",
|
||||
"Server-derived. `auto_disabled` — the dispatcher turned it off after repeated hard rejects (`disabledReason` says why). `manually_disabled` — the user turned it off. `sending_paused` — still enabled but inside a retry cooldown until `nextRetryAt`, after a transient failure or after a send that had nobody to go to (no browser subscribed, no iPhone paired, a transport the operator has not set up; `lastFailureReason` names it, `consecutiveFailures` is not raised). `active` — enabled and sending.",
|
||||
),
|
||||
disabledReason: z
|
||||
.string()
|
||||
|
||||
Reference in new issue
Block a user