From f2ed00b5bcf5d03ae3563cff8bf4bf2e738f038f Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Fri, 23 Oct 2026 15:28:35 -0400 Subject: [PATCH] fix(push): scope app groups to the app and log fixed failure codes A failed result carries an optional code from a fixed set; logs carry only status, code and counts. --- .../push/PushService.delivery.test.ts | 12 +++-- src/backend/services/push/PushService.ts | 51 +++++++++++-------- src/backend/services/push/pushMessage.test.ts | 17 ++++++- src/backend/services/push/pushMessage.ts | 4 +- src/backend/services/push/types.ts | 14 ++++- 5 files changed, 71 insertions(+), 27 deletions(-) diff --git a/src/backend/services/push/PushService.delivery.test.ts b/src/backend/services/push/PushService.delivery.test.ts index d5b56aa21..9e67f329c 100644 --- a/src/backend/services/push/PushService.delivery.test.ts +++ b/src/backend/services/push/PushService.delivery.test.ts @@ -53,7 +53,12 @@ class FakeProvider implements PushProvider { return batch.tokens.map((token) => { if (token.includes('gone')) return { status: 'unregistered' }; if (token.includes('bad')) { - return { status: 'failed', reason: `rejected ${token}` }; + // A provider's own text never reaches the log, only the code. + return { + status: 'failed', + code: 'invalid_token', + reason: `rejected https://push.example.test/${token}`, + } as PushResult; } return { status: 'delivered' }; }); @@ -247,7 +252,7 @@ describe('PushService delivery', () => { uid, title: 'New reply', body: 'see you at 5', - group: 'room-7', + group: `${app.uid}:room-7`, target: { app: app.uid }, }); }); @@ -429,7 +434,8 @@ describe('PushService delivery', () => { const logged = JSON.stringify(warn.mock.calls); warn.mockRestore(); - expect(logged).toContain(''); + expect(logged).toContain('invalid_token'); + expect(logged).not.toContain('push.example.test'); expect(logged).not.toMatch(/bad-[0-9a-f]{8}-/); }); diff --git a/src/backend/services/push/PushService.ts b/src/backend/services/push/PushService.ts index 89d66f8ce..30c3cafca 100644 --- a/src/backend/services/push/PushService.ts +++ b/src/backend/services/push/PushService.ts @@ -33,11 +33,13 @@ import { PuterService } from '../types.js'; import { hasPushConsent } from './pushConsent.js'; import { buildPushMessage, fitPushMessage } from './pushMessage.js'; import { isUserConnected } from './pushPresence.js'; -import type { - PushMessage, - PushPlatform, - PushProvider, - PushResult, +import { + PUSH_FAILURE_CODES, + type PushFailureCode, + type PushMessage, + type PushPlatform, + type PushProvider, + type PushResult, } from './types.js'; /** Most tokens one provider call takes. */ @@ -250,11 +252,15 @@ export class PushService extends PuterService { const delivered: string[] = []; const failed: string[] = []; const unregistered: string[] = []; - const reasons = new Set(); + const codes: Partial> = {}; settled.forEach((outcome, index) => { let results: Array = []; + let batchCode: PushFailureCode = 'unknown'; if (outcome.status === 'rejected') { - reasons.add(errorName(outcome.reason)); + batchCode = + outcome.reason instanceof PushTimeoutError + ? 'timeout' + : 'provider_error'; } else if (Array.isArray(outcome.value)) { results = outcome.value; } @@ -266,14 +272,8 @@ export class PushService extends PuterService { unregistered.push(hashPushToken(device.token)); } else { failed.push(device.uid); - if (result?.status === 'failed' && result.reason) { - reasons.add( - String(result.reason) - .split(device.token) - .join('') - .slice(0, 120), - ); - } + const code = failureCode(result, batchCode); + codes[code] = (codes[code] ?? 0) + 1; } }); }); @@ -281,8 +281,9 @@ export class PushService extends PuterService { if (failed.length > 0) { console.warn('[push] not delivered to some devices', { uid: message.uid, + status: 'failed', devices: failed.length, - reasons: [...reasons].slice(0, 5), + codes, }); } await Promise.all([ @@ -331,18 +332,26 @@ export class PushService extends PuterService { } } +class PushTimeoutError extends Error {} + +/** A provider's own code when it is a known one, never its text. */ +const failureCode = ( + result: PushResult | undefined, + fallback: PushFailureCode, +): PushFailureCode => { + if (result?.status !== 'failed') return fallback; + const code = result.code; + return code && PUSH_FAILURE_CODES.includes(code) ? code : 'unknown'; +}; + const withTimeout = (promise: Promise): Promise => { let timer: ReturnType | undefined; const timeout = new Promise((_resolve, reject) => { timer = setTimeout( - () => reject(new Error('push provider timed out')), + () => reject(new PushTimeoutError('push provider timed out')), SEND_TIMEOUT_MS, ); timer.unref?.(); }); return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); }; - -/** Only the error's name: a provider's message may carry tokens. */ -const errorName = (err: unknown): string => - err instanceof Error ? err.name || 'Error' : 'error'; diff --git a/src/backend/services/push/pushMessage.test.ts b/src/backend/services/push/pushMessage.test.ts index 3458ed63b..af4b75a71 100644 --- a/src/backend/services/push/pushMessage.test.ts +++ b/src/backend/services/push/pushMessage.test.ts @@ -122,7 +122,7 @@ describe('buildPushMessage', () => { title: 'New reply', body: 'see you at 5', icon: 'https://chat.example/icon.png', - group: 'room-7', + group: 'app-uid-1:room-7', target: { app: 'app-uid-1', path: '/rooms/7' }, data: { room: 7 }, ttl: 120, @@ -130,6 +130,21 @@ describe('buildPushMessage', () => { }); }); + it('scopes an app’s group to the app', () => { + const build = (appUid: string, group: string) => + buildPushMessage( + { + uid: 'n9', + type: 'app.message', + value: { title: 't', group }, + }, + { uid: appUid, name: 'A' }, + )?.group; + expect(build('app-a', 'room')).not.toBe(build('app-b', 'room')); + // A group naming another notification's uid does not match its tag. + expect(build('app-a', 'n1')).not.toBe('n1'); + }); + it('groups an app message by its uid without a group', () => { const message = buildPushMessage( { uid: 'n5', type: 'app.message', value: { title: 'hi' } }, diff --git a/src/backend/services/push/pushMessage.ts b/src/backend/services/push/pushMessage.ts index 4e852691c..bb1b72a06 100644 --- a/src/backend/services/push/pushMessage.ts +++ b/src/backend/services/push/pushMessage.ts @@ -91,9 +91,11 @@ const appMessage = ( const title = str(value.title); if (!title) return null; const ttl = Number(value.ttl); + const group = str(value.group); const base = { uid: source.uid, - group: str(value.group) ?? source.uid, + // Scoped to the app, so it can only ever replace its own pushes. + group: group ? `${app.uid}:${group}` : source.uid, ttl: Number.isInteger(ttl) && ttl > 0 ? ttl diff --git a/src/backend/services/push/types.ts b/src/backend/services/push/types.ts index 04d9a981c..11e66ff77 100644 --- a/src/backend/services/push/types.ts +++ b/src/backend/services/push/types.ts @@ -50,7 +50,19 @@ export type PushResult = | { status: 'delivered' } /** The provider says the token no longer names a device. */ | { status: 'unregistered' } - | { status: 'failed'; reason?: string }; + | { status: 'failed'; code?: PushFailureCode }; + +/** Why a push failed. Only these are logged; providers map their own errors. */ +export const PUSH_FAILURE_CODES = [ + 'invalid_token', + 'rate_limited', + 'payload_too_large', + 'provider_error', + 'timeout', + 'unknown', +] as const; + +export type PushFailureCode = (typeof PUSH_FAILURE_CODES)[number]; export interface PushBatch { platform: PushPlatform;