Merge pull request #1041 from lucamzanon/fix/web-push-recovery

fix(push): keep notification previews truthful when a lookup fails

app/api/push/preview/route.ts

@@ -161,10 +161,16 @@ export async function GET(request: NextRequest) {
     };
 
     const inboxBody = inboxData.methodResponses.find(
-      ([method]) => method === 'Mailbox/query',
+      ([method, , callId]) => method === 'Mailbox/query' && callId === 'mb',
     )?.[1] as { ids?: string[] } | undefined;
 
-    const inboxId = inboxBody?.ids?.[0];
+    // A method-level JMAP error still has HTTP 200. Do not turn it into
+    // an empty Inbox: the service worker would silently discard the push.
+    if (!Array.isArray(inboxBody?.ids)) {
+      return NextResponse.json({ error: 'JMAP mailbox query failed' }, { status: 502 });
+    }
+
+    const inboxId = inboxBody.ids[0];
     const emailProperties = ['id', 'threadId', 'from', 'subject', 'preview', 'receivedAt'];
 
     if (!inboxId && !requestedEmailId) {
@@ -239,6 +245,19 @@ export async function GET(request: NextRequest) {
       methodResponses: [string, Record<string, unknown>, string][];
     };
 
+    // Missing/failed method responses are preview failures, not proof that
+    // there is no unread mail. A non-2xx response lets the SW show its fallback.
+    for (const call of methodCalls as [string, unknown, string][]) {
+      const response = data.methodResponses.find(([method, , callId]) =>
+        method === call[0] && callId === call[2]);
+      const result = response?.[1];
+      if (!result || (call[0] === 'Email/query'
+        ? !Array.isArray(result.ids) || typeof result.total !== 'number'
+        : !Array.isArray(result.list))) {
+        return NextResponse.json({ error: 'JMAP email query failed' }, { status: 502 });
+      }
+    }
+
     type EmailLite = {
       id: string;
       threadId: string;
@@ -270,6 +289,13 @@ export async function GET(request: NextRequest) {
         unreadTotal = Math.max(unreadTotal, 1);
       }
       email = delivered;
+    } else if (requestedEmailId) {
+      // The server named the message that arrived and it could not be read -
+      // deleted since, or filed where this login cannot see it. "Newest unread
+      // in the Inbox" is then some other message, and announcing it would be
+      // confidently wrong. Name no message: the unread total still comes back,
+      // so the SW posts its generic toast instead of staying silent.
+      email = null;
     }
 
     // Some servers build the preview from HTML without skipping <style>; a

lib/__tests__/push-preview-route.test.ts

@@ -65,10 +65,10 @@ function mockJmap() {
   });
 }
 
-async function callRoute(accountId: string) {
+async function callRoute(accountId: string, emailId: string | null = null) {
   const { GET } = await import('@/app/api/push/preview/route');
   const request = {
-    nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : null) } },
+    nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : k === 'emailId' ? emailId : null) } },
   };
   const res = (await GET(request as unknown as Parameters<typeof GET>[0])) as unknown as {
     status: number;
@@ -116,3 +116,76 @@ describe('push preview route account resolution', () => {
     expect(status).toBe(401);
   });
 });
+
+
+describe('push preview JMAP failures', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockJmap();
+  });
+
+  it.each([
+    ['error', { type: 'unsupportedFilter' }, 'mb'],
+    ['Mailbox/query', {}, 'mb'],
+  ])('does not silence a push when mailbox lookup fails (%s)', async (method, body, id) => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [[method, body, id]] }));
+    expect((await callRoute('a')).status).toBe(502);
+  });
+
+  it.each([
+    [['error', { type: 'serverFail' }, 'eq'], ['error', { type: 'resultReference' }, 'eg']],
+    [['Email/query', { ids: ['e1'], total: 1 }, 'eq'], ['error', { type: 'serverFail' }, 'eg']],
+    [['Email/query', { ids: [] }, 'eq'], ['Email/get', { list: [] }, 'eg']],
+  ])('does not report zero unread on an email method failure', async (query, get) => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [query, get] }));
+    expect((await callRoute('a')).status).toBe(502);
+  });
+
+  it('still reports genuinely empty unread results', async () => {
+    fetchJmapServer.mockReset()
+      .mockResolvedValueOnce(jsonResponse(SESSION))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+      .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+        ['Email/query', { ids: [], total: 0 }, 'eq'],
+        ['Email/get', { list: [] }, 'eg'],
+      ] }));
+    expect(await callRoute('a')).toEqual({ status: 200, body: { email: null, unreadTotal: 0, slot: 0 } });
+  });
+});
+
+
+it('previews a delivered message outside an empty Inbox', async () => {
+  fetchJmapServer.mockReset()
+    .mockResolvedValueOnce(jsonResponse(SESSION))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+      ['Email/query', { ids: [], total: 0 }, 'eq'],
+      ['Email/get', { list: [] }, 'eg'],
+      ['Email/get', { list: [{ id: 'filed', threadId: 't2' }] }, 'delivered'],
+    ] }));
+  expect(await callRoute('a', 'filed')).toEqual({
+    status: 200, body: { email: { id: 'filed', threadId: 't2' }, unreadTotal: 1, slot: 0 },
+  });
+});
+
+it('names no message when the delivered one cannot be read, rather than another', async () => {
+  // The push named a message - deleted since, or filed where this login
+  // cannot see it - and the Inbox happens to hold an older unread one. That
+  // older message is not what arrived.
+  fetchJmapServer.mockReset()
+    .mockResolvedValueOnce(jsonResponse(SESSION))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [['Mailbox/query', { ids: ['inbox'] }, 'mb']] }))
+    .mockResolvedValueOnce(jsonResponse({ methodResponses: [
+      ['Email/query', { ids: ['stale'], total: 3 }, 'eq'],
+      ['Email/get', { list: [{ id: 'stale', threadId: 't9' }] }, 'eg'],
+      ['Email/get', { list: [] }, 'delivered'],
+    ] }));
+  expect(await callRoute('a', 'gone')).toEqual({
+    status: 200, body: { email: null, unreadTotal: 3, slot: 0 },
+  });
+});