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.
This commit is contained in:
Juan Castro committed 2026-10-24 16:07:59 -04:00
1 parent 3da39b7e35
commit f2ed00b5bc
5 files changed
+71 -27

No files matched your search

@@ -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('<token>');
expect(logged).toContain('invalid_token');
expect(logged).not.toContain('push.example.test');
expect(logged).not.toMatch(/bad-[0-9a-f]{8}-/);
});
+30 -21
View File
@@ -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<string>();
const codes: Partial<Record<PushFailureCode, number>> = {};
settled.forEach((outcome, index) => {
let results: Array<PushResult | undefined> = [];
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('<token>')
.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 = <T>(promise: Promise<T>): Promise<T> => {
let timer: ReturnType<typeof setTimeout> | undefined;
const timeout = new Promise<never>((_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';
+16 -1
View File
@@ -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' } },
+3 -1
View File
@@ -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
+13 -1
View File
@@ -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;