From f5ca1b0c22d256a9e2d1d8c775eb67f598b0231e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 15:57:20 +0000 Subject: [PATCH 1/2] fix(announcements): suppress global error toast for fail-open announcement requests The ack POST and feed GET already swallow their own errors by design (fail-open dismissal), but the global error interceptor still toasted them. On mobile, a dropped ack (status 0) surfaced as "An error occurred. Please try again." for something the user cannot act on. Opt both requests out via SUPPRESS_ERROR_TOAST, as NotificationsService already does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0127ERDpxwL2nzDEWZU4qPss --- .../announcements.service.spec.ts | 26 +++++++++++++++++++ .../announcements/announcements.service.ts | 21 ++++++++++++--- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts b/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts index e36d84447..7ac8899e0 100644 --- a/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts +++ b/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts @@ -7,6 +7,7 @@ import { import { provideHttpClient } from '@angular/common/http'; import { signal } from '@angular/core'; import { AnnouncementsService } from './announcements.service'; +import { SUPPRESS_ERROR_TOAST } from '../../auth/error.interceptor'; import { Announcement, AnnouncementFeed } from './announcement.model'; import { ConfigService } from '../config.service'; @@ -124,6 +125,16 @@ describe('AnnouncementsService', () => { expect(service.unreadCount()).toBe(0); expect(service.hasUnread()).toBe(false); }); + + it('opts out of the global error toast', async () => { + service.feedResource.reload(); + service.panelItems(); + await vi.waitFor(() => { + const req = httpMock.expectOne(FEED_URL); + expect(req.request.context.get(SUPPRESS_ERROR_TOAST)).toBe(true); + req.flush(makeFeed()); + }); + }); }); describe('unread count', () => { @@ -229,6 +240,21 @@ describe('AnnouncementsService', () => { await expect(done).resolves.toBe(false); }); + it('opts out of the global error toast', async () => { + // A status-0 network drop on mobile surfaced as "An error occurred" + // even though the failure is swallowed here by design (§D7). + await loadFeed(); + + const done = service.ack('a1', 'acknowledged', 'modal'); + await vi.waitFor(() => { + const req = httpMock.expectOne(`${API}/announcements/a1/ack`); + expect(req.request.context.get(SUPPRESS_ERROR_TOAST)).toBe(true); + req.error(new ProgressEvent('error'), { status: 0, statusText: 'Unknown Error' }); + }); + + expect(await done).toBe(false); + }); + it('a local dismissal hides the modal too', async () => { await loadFeed( makeFeed({ diff --git a/frontend/ai.client/src/app/services/announcements/announcements.service.ts b/frontend/ai.client/src/app/services/announcements/announcements.service.ts index 1499cee85..496f8bac4 100644 --- a/frontend/ai.client/src/app/services/announcements/announcements.service.ts +++ b/frontend/ai.client/src/app/services/announcements/announcements.service.ts @@ -1,6 +1,7 @@ import { Injectable, computed, inject, resource, signal } from '@angular/core'; -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpContext } from '@angular/common/http'; import { firstValueFrom } from 'rxjs'; +import { SUPPRESS_ERROR_TOAST } from '../../auth/error.interceptor'; import { ConfigService } from '../config.service'; import { Announcement, @@ -46,6 +47,16 @@ export class AnnouncementsService { () => `${this.config.appApiUrl()}/announcements`, ); + /** + * Every request here fails open and swallows its own error, so the global + * error toast must not fire either — otherwise a dropped ack (status 0 on a + * flaky mobile connection, a transient 5xx) still puts "An error occurred" + * in front of the user for something they cannot act on. + */ + private readonly options = { + context: new HttpContext().set(SUPPRESS_ERROR_TOAST, true), + }; + /** * Loads on first read. The topnav only renders the user dropdown once the * session bootstrap has resolved, so the loader fires post-auth with the @@ -110,7 +121,11 @@ export class AnnouncementsService { const body: AnnouncementAckRequest = { action, surface }; try { await firstValueFrom( - this.http.post(`${this.baseUrl()}/${announcementId}/ack`, body), + this.http.post( + `${this.baseUrl()}/${announcementId}/ack`, + body, + this.options, + ), ); return true; } catch { @@ -145,7 +160,7 @@ export class AnnouncementsService { private async fetchFeed(): Promise { try { return await firstValueFrom( - this.http.get(`${this.baseUrl()}/`), + this.http.get(`${this.baseUrl()}/`, this.options), ); } catch { // The surface is kill-switched off (404) or the backend is unhappy. From cc3214e4d2554515eb1afbe4d58b334d0e6d56f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 16:41:42 +0000 Subject: [PATCH 2/2] fix(announcements): retry an ack once on a transient failure A dropped ack (status 0 on a flaky mobile connection, or a 502/503/504 from the gateway) was lost silently, so a requires-ack announcement resurfaced on the next load. Retry once after 1s. The endpoint is idempotent and monotonic, so a duplicate that did land is a no-op. 4xx and other errors are not retried. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0127ERDpxwL2nzDEWZU4qPss --- .../announcements.service.spec.ts | 70 ++++++++++++++++++- .../announcements/announcements.service.ts | 33 ++++++++- 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts b/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts index 7ac8899e0..fe506dfe1 100644 --- a/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts +++ b/frontend/ai.client/src/app/services/announcements/announcements.service.spec.ts @@ -6,7 +6,7 @@ import { } from '@angular/common/http/testing'; import { provideHttpClient } from '@angular/common/http'; import { signal } from '@angular/core'; -import { AnnouncementsService } from './announcements.service'; +import { ACK_RETRY_DELAY_MS, AnnouncementsService } from './announcements.service'; import { SUPPRESS_ERROR_TOAST } from '../../auth/error.interceptor'; import { Announcement, AnnouncementFeed } from './announcement.model'; import { ConfigService } from '../config.service'; @@ -249,12 +249,78 @@ describe('AnnouncementsService', () => { await vi.waitFor(() => { const req = httpMock.expectOne(`${API}/announcements/a1/ack`); expect(req.request.context.get(SUPPRESS_ERROR_TOAST)).toBe(true); - req.error(new ProgressEvent('error'), { status: 0, statusText: 'Unknown Error' }); + req.flush('nope', { status: 500, statusText: 'Server Error' }); }); expect(await done).toBe(false); }); + describe('retry', () => { + const ACK_URL = `${API}/announcements/a1/ack`; + const networkDrop = (req: ReturnType) => + req.error(new ProgressEvent('error'), { status: 0, statusText: 'Unknown Error' }); + + afterEach(() => vi.useRealTimers()); + + it('retries once after a network drop and reports success', async () => { + await loadFeed(); + vi.useFakeTimers(); + + const done = service.ack('a1', 'acknowledged', 'modal'); + networkDrop(httpMock.expectOne(ACK_URL)); + + // Nothing is resent until the delay has elapsed. + httpMock.expectNone(ACK_URL); + await vi.advanceTimersByTimeAsync(ACK_RETRY_DELAY_MS); + + const retried = httpMock.expectOne(ACK_URL); + expect(retried.request.body).toEqual({ action: 'acknowledged', surface: 'modal' }); + retried.flush(null); + + expect(await done).toBe(true); + }); + + it('retries a gateway 503', async () => { + await loadFeed(); + vi.useFakeTimers(); + + const done = service.ack('a1', 'acknowledged', 'modal'); + httpMock + .expectOne(ACK_URL) + .flush('nope', { status: 503, statusText: 'Service Unavailable' }); + await vi.advanceTimersByTimeAsync(ACK_RETRY_DELAY_MS); + httpMock.expectOne(ACK_URL).flush(null); + + expect(await done).toBe(true); + }); + + it('gives up after the one retry', async () => { + await loadFeed(); + vi.useFakeTimers(); + + const done = service.ack('a1', 'acknowledged', 'modal'); + networkDrop(httpMock.expectOne(ACK_URL)); + await vi.advanceTimersByTimeAsync(ACK_RETRY_DELAY_MS); + networkDrop(httpMock.expectOne(ACK_URL)); + await vi.advanceTimersByTimeAsync(ACK_RETRY_DELAY_MS * 5); + + httpMock.expectNone(ACK_URL); + expect(await done).toBe(false); + }); + + it('does not retry a 4xx', async () => { + await loadFeed(); + vi.useFakeTimers(); + + const done = service.ack('a1', 'acknowledged', 'modal'); + httpMock.expectOne(ACK_URL).flush('nope', { status: 404, statusText: 'Not Found' }); + await vi.advanceTimersByTimeAsync(ACK_RETRY_DELAY_MS * 5); + + httpMock.expectNone(ACK_URL); + expect(await done).toBe(false); + }); + }); + it('a local dismissal hides the modal too', async () => { await loadFeed( makeFeed({ diff --git a/frontend/ai.client/src/app/services/announcements/announcements.service.ts b/frontend/ai.client/src/app/services/announcements/announcements.service.ts index 496f8bac4..f856cc8f8 100644 --- a/frontend/ai.client/src/app/services/announcements/announcements.service.ts +++ b/frontend/ai.client/src/app/services/announcements/announcements.service.ts @@ -1,6 +1,6 @@ import { Injectable, computed, inject, resource, signal } from '@angular/core'; -import { HttpClient, HttpContext } from '@angular/common/http'; -import { firstValueFrom } from 'rxjs'; +import { HttpClient, HttpContext, HttpErrorResponse } from '@angular/common/http'; +import { firstValueFrom, retry, throwError, timer } from 'rxjs'; import { SUPPRESS_ERROR_TOAST } from '../../auth/error.interceptor'; import { ConfigService } from '../config.service'; import { @@ -11,6 +11,22 @@ import { AnnouncementSurface, } from './announcement.model'; +/** How long to wait before the single ack retry. */ +export const ACK_RETRY_DELAY_MS = 1000; + +/** + * Failures worth one more attempt: the request never landed (status 0 — a + * mobile network blip, a backgrounded tab) or a gateway in front of app-api + * gave up. A 4xx is the server's considered answer and retrying won't change + * it. + */ +function isTransient(error: unknown): boolean { + return ( + error instanceof HttpErrorResponse && + (error.status === 0 || error.status === 502 || error.status === 503 || error.status === 504) + ); +} + const EMPTY_FEED: AnnouncementFeed = { panel: [], banner: null, @@ -106,7 +122,12 @@ export class AnnouncementsService { * * Fails open (§D7): a rejected POST still hides the item locally and * resolves rather than throwing, so no caller has to remember to catch. - * Returns whether the server accepted it, for tests and for a future retry. + * Returns whether the server accepted it. + * + * A transient failure is retried once. The endpoint is idempotent and + * monotonic (§D2), so a duplicate that did land server-side is a no-op — + * and without the retry, a dropped `acknowledged` resurfaces the modal on + * the next load. */ async ack( announcementId: string, @@ -125,6 +146,12 @@ export class AnnouncementsService { `${this.baseUrl()}/${announcementId}/ack`, body, this.options, + ).pipe( + retry({ + count: 1, + delay: error => + isTransient(error) ? timer(ACK_RETRY_DELAY_MS) : throwError(() => error), + }), ), ); return true;