[ENG-12563] Improve error pages

docs/error-handling.md

@@ -0,0 +1,162 @@
+# Error handling
+
+Two complementary layers handle failures from API-backed work:
+
+| Layer                | Responsibility                                                      | Entry point            |
+| -------------------- | ------------------------------------------------------------------- | ---------------------- |
+| **HTTP interceptor** | Global UX: toast, navigation, auth, maintenance, loader             | `error.interceptor.ts` |
+| **NGXS state**       | Section bookkeeping: clear loading flags, store `error`, Sentry tag | `handleSectionError`   |
+
+Both **rethrow**. For a store action that calls HTTP: interceptor runs first → rethrow → state’s `catchError` → `handleSectionError`.
+
+## Index
+
+- [HTTP interceptor](#http-interceptor)
+  - [Flow](#flow)
+  - [Status handling](#status-handling)
+  - [Bypass](#bypass)
+  - [Silent 403](#silent-403)
+  - [Messages](#messages)
+  - [Caller guidelines](#caller-guidelines)
+- [NGXS state errors](#ngxs-state-errors)
+- [Related files](#related-files)
+
+## HTTP interceptor
+
+Central HTTP UX lives in `src/app/core/interceptors/error.interceptor.ts`. Prefer the interceptor for shared UX; handle locally only when the UI needs a special message or flow.
+
+### Flow
+
+Order before the status `switch` matters:
+
+1. **SSR (`!isBrowser`)** — no toast, navigation, loader, auth, or Sentry; rethrow only.
+2. **Bypass** — Sentry capture + rethrow; skip all UX.
+3. **Maintenance** — `503` with `meta.maintenance_mode: true` → hide loader, activate maintenance mode, rethrow.
+4. **Status `switch`** — `401` / `403` / `404` / `409` / `410` / default.
+
+After the `switch` (except early `409` return), the interceptor hides the loader and rethrows. `409` does **not** hide the loader; the caller owns that.
+
+```mermaid
+flowchart TD
+  err[HttpErrorResponse] --> ssr{Browser?}
+  ssr -->|no| rethrow[Rethrow]
+  ssr -->|yes| bypass{BYPASS?}
+  bypass -->|yes| sentry[Sentry + rethrow]
+  bypass -->|no| maint{Maintenance 503?}
+  maint -->|yes| activate[Hide loader + activate maintenance + rethrow]
+  maint -->|no| status{Status}
+  status -->|409| conflict[Rethrow only]
+  status -->|other| rethrow2[Hide loader + rethrow]
+  rethrow2 --> state[NGXS catchError / handleSectionError]
+  conflict --> state
+  rethrow --> state
+  sentry --> state
+  activate --> state
+```
+
+### Status handling
+
+| Status    | Interceptor behavior                                                                                                                                                       |
+| --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| **401**   | Skip auth redirect on view-only (`?view_only=`). Else logout if authenticated, or navigate to sign-in. Hide loader.                                                        |
+| **403**   | Navigate unless silent / pending-moderation (see below). Node URL → `/request-access/:id` (skipped on draft registry routes). Other → `/forbidden`. Hide loader. No toast. |
+| **404**   | Navigate to `/not-found`. Hide loader. No toast.                                                                                                                           |
+| **409**   | Rethrow only (no toast, no loader hide). Caller handles conflict UX.                                                                                                       |
+| **410**   | Navigate to `/content-deleted`. Hide loader. No toast.                                                                                                                     |
+| **Other** | Toast via `getErrorMessage`, hide loader.                                                                                                                                  |
+
+`/not-found` and `/content-deleted` share `PageNotFoundComponent` with different route data (`PAGE_NOT_FOUND_ROUTE_DATA` / `CONTENT_DELETED_ROUTE_DATA`).
+
+### Bypass
+
+Use when the **caller** owns all error UX (or the failure must not navigate/toast).
+
+```ts
+import { bypassErrorInterceptorContext } from '@core/interceptors/error-interceptor.tokens';
+
+this.http.get(url, { context: bypassErrorInterceptorContext() });
+```
+
+`bypassErrorInterceptorContext()` returns a **new** `HttpContext` each call (do not reuse one mutable instance).
+
+Bypass still reports to Sentry and rethrows. It skips toast, navigation, loader, auth, and maintenance activation.
+
+Current examples: maintenance status polling, parent project fetch, Datacite tracker, addon OAuth token refresh, external identity delete.
+
+### Silent 403
+
+These skip `/forbidden` / request-access navigation (loader still hides; error still rethrows):
+
+| Case                        | How                                                                                                                               |
+| --------------------------- | --------------------------------------------------------------------------------------------------------------------------------- |
+| Metadata record fetch       | `X-No-Auth-Redirect` on `MetadataRecordsService.getMetadataRecord` (optional schema.org; prefer migrating to `HttpContext` later) |
+| Node access-request API     | URL matches `/v2/nodes/:id/requests`                                                                                              |
+| Preprint pending moderation | Detail equals `PREPRINT_PENDING_MODERATION_DETAIL` — component routes to pending-moderation                                       |
+
+### Messages
+
+`getErrorMessage` resolution:
+
+1. Client `ErrorEvent.message`
+2. JSON:API `error.errors[0].detail`
+3. For 5xx: `error.message` or `common.errorMessages.serverError`
+4. Else `ERROR_MESSAGES[status]` or `common.errorMessages.unknown`
+
+`ERROR_MESSAGES` maps statuses to i18n keys under `common.errorMessages.*`. Toasts run through the translate pipe.
+
+### Caller guidelines
+
+- **Default** — do not toast in the subscriber; the interceptor already does for unhandled statuses.
+- **Custom toast / flow** — either `bypassErrorInterceptorContext()` **or** remove the local toast so users do not see two messages.
+- **409** — handle in the caller (interceptor is silent).
+- **Expected 404/410 on secondary requests** — use bypass; otherwise the interceptor navigates away from the current page.
+- **Pending-moderation preprint** — leave interceptor special-case + component navigation; do not send that request with full bypass unless you also handle 404/410/5xx locally.
+- **Loader** — interceptor hides for most statuses; for `409` (and bypass) hide in the caller (`finalize` is a good pattern).
+
+## NGXS state errors
+
+Store actions that hit HTTP should end failed sections with `handleSectionError` from `src/app/shared/helpers/state-error.handler.ts`.
+
+It does **not** replace the interceptor. It:
+
+1. Reports to Sentry with `state.section` / feature tags (a second report if the interceptor already captured on bypass)
+2. Patches the named section: `isLoading: false`, `isSubmitting: false`, `error: error.message`
+3. Rethrows so components / `dispatch` subscribers can still react
+
+```ts
+.catchError((error) => handleSectionError(ctx, 'registry', error))
+```
+
+`section` must be a key on the state model whose value follows `AsyncStateModel` (see [NGXS docs](./ngxs.md#state-models)):
+
+```ts
+export interface AsyncStateModel<T> {
+  data: T;
+  isLoading: boolean;
+  isSubmitting?: boolean;
+  error: string | null;
+}
+```
+
+Guidelines:
+
+- Use `handleSectionError` in action `catchError` for async sections — do not only clear loading in an ad-hoc patch unless the section is not `AsyncStateModel`-shaped.
+- Do not toast inside the state handler; leave shared toasts to the interceptor (or the component when bypassed / `409`).
+- UI that needs an inline error can read `section.error` from the store; many screens rely on interceptor toasts instead.
+- Components that `dispatch(...).subscribe({ error })` still receive the error after both layers.
+
+## Related files
+
+| File                                                    | Role                                       |
+| ------------------------------------------------------- | ------------------------------------------ |
+| `src/app/core/interceptors/error.interceptor.ts`        | HTTP UX handler                            |
+| `src/app/core/interceptors/error-interceptor.tokens.ts` | Bypass token + factory                     |
+| `src/app/shared/helpers/state-error.handler.ts`         | NGXS section error helper                  |
+| `src/app/shared/models/store/async-state.model.ts`      | Async section shape (`error` field)        |
+| `src/app/core/constants/error-messages.ts`              | Status → i18n keys                         |
+| `src/app/core/constants/http-error-details.const.ts`    | Shared API detail strings                  |
+| `src/app/core/constants/page-not-found.constants.ts`    | Route data for 404 / 410 pages             |
+| `src/app/core/services/maintenance-mode.service.ts`     | Maintenance detection + activation         |
+| `src/app/core/components/page-not-found/`               | Shared not-found / content-deleted UI      |
+| `docs/ngxs.md`                                          | Store / `AsyncStateModel` conventions      |
+| `docs/sentry.md`                                        | Which HTTP errors are filtered from Sentry |

docs/ngxs.md

@@ -87,7 +87,7 @@ export interface FilesStateModel {
 1. `data` holds strongly typed domain data (not raw JSON:API payloads when a domain model exists).
 2. `isLoading` indicates a read/fetch is in progress.
 3. `isSubmitting` indicates a write (create/update/delete) is in progress.
-4. `error` stores a failed request message for UI or logging.
+4. `error` stores a failed request message for UI or logging. Set it via `handleSectionError` in action `catchError` — see [Error handling](./error-handling.md#ngxs-state-errors).
 
 Each domain state should be minimal and scoped to its feature.
 
@@ -112,4 +112,5 @@ Each domain state should be minimal and scoped to its feature.
 
 - [Models Conventions](./models.md)
 - [Folder Structure](./arch.md)
+- [Error handling](./error-handling.md) (HTTP interceptor + `handleSectionError`)
 - Official NGXS docs: [https://www.ngxs.io/docs](https://www.ngxs.io/docs)

src/app/app.routes.server.ts

@@ -25,6 +25,10 @@ export const serverRoutes: ServerRoute[] = [
     path: 'not-found',
     renderMode: RenderMode.Prerender,
   },
+  {
+    path: 'content-deleted',
+    renderMode: RenderMode.Prerender,
+  },
   {
     path: 'forgotpassword',
     renderMode: RenderMode.Prerender,

src/app/app.routes.ts

@@ -7,6 +7,7 @@ import { isFileGuard } from '@core/guards/is-file.guard';
 import { isProjectGuard } from '@core/guards/is-project.guard';
 import { isRegistryGuard } from '@core/guards/is-registry.guard';
 import { redirectIfLoggedInGuard } from '@core/guards/redirect-if-logged-in.guard';
+import { CONTENT_DELETED_ROUTE_DATA, PAGE_NOT_FOUND_ROUTE_DATA } from '@osf/core/constants/page-not-found.constants';
 
 import { MyPreprintsState } from './features/preprints/store/my-preprints';
 import { ProfileState } from './features/profile/store';
@@ -195,7 +196,13 @@ export const routes: Routes = [
         path: 'not-found',
         loadComponent: () =>
           import('./core/components/page-not-found/page-not-found.component').then((mod) => mod.PageNotFoundComponent),
-        data: { skipBreadcrumbs: true },
+        data: PAGE_NOT_FOUND_ROUTE_DATA,
+      },
+      {
+        path: 'content-deleted',
+        loadComponent: () =>
+          import('./core/components/page-not-found/page-not-found.component').then((mod) => mod.PageNotFoundComponent),
+        data: CONTENT_DELETED_ROUTE_DATA,
       },
       {
         path: ':id/files/:fileGuid/preview',
@@ -244,7 +251,7 @@ export const routes: Routes = [
         path: '**',
         loadComponent: () =>
           import('./core/components/page-not-found/page-not-found.component').then((mod) => mod.PageNotFoundComponent),
-        data: { skipBreadcrumbs: true },
+        data: PAGE_NOT_FOUND_ROUTE_DATA,
       },
     ],
   },

src/app/core/components/page-not-found/page-not-found.component.html

@@ -1,8 +1,7 @@
 <section class="container text-center flex flex-column flex-1 my-7 mx-3 p-3 md:p-4">
-  <h2>{{ 'pageNotFound.title' | translate }}</h2>
-  <p class="mt-4">
-    <span>{{ 'pageNotFound.message' | translate }}</span>
-    <a class="ml-1 font-bold" [href]="'mailto:' + supportEmail">{{ supportEmail }}</a
-    >.
-  </p>
+  <h2>{{ titleKey | translate }}</h2>
+  <p class="mt-4">{{ messageKey | translate }}</p>
+  <a class="p-button font-bold no-underline mt-4 align-self-center" [href]="'mailto:' + supportEmail">
+    {{ 'pageNotFound.contactSupport' | translate }}
+  </a>
 </section>

src/app/core/components/page-not-found/page-not-found.component.spec.ts

@@ -1,29 +1,61 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 
+import { CONTENT_DELETED_ROUTE_DATA, PAGE_NOT_FOUND_ROUTE_DATA } from '@core/constants/page-not-found.constants';
+
 import { provideOSFCore } from '@testing/osf.testing.provider';
+import { ActivatedRouteMockBuilder, provideActivatedRouteMock } from '@testing/providers/route-provider.mock';
 
 import { PageNotFoundComponent } from './page-not-found.component';
 
 describe('PageNotFoundComponent', () => {
   let component: PageNotFoundComponent;
   let fixture: ComponentFixture<PageNotFoundComponent>;
 
-  beforeEach(() => {
+  function setup(data: Record<string, unknown> = {}) {
     TestBed.configureTestingModule({
       imports: [PageNotFoundComponent],
-      providers: [provideOSFCore()],
+      providers: [
+        provideOSFCore(),
+        provideActivatedRouteMock(ActivatedRouteMockBuilder.create().withData(data).build()),
+      ],
     });
 
     fixture = TestBed.createComponent(PageNotFoundComponent);
     component = fixture.componentInstance;
     fixture.detectChanges();
-  });
+  }
 
   it('should create', () => {
+    setup();
     expect(component).toBeTruthy();
   });
 
   it('should expose support email from environment', () => {
+    setup();
     expect(component.supportEmail).toBe('support@test.com');
   });
+
+  it('should fall back to page not found keys when route data is empty', () => {
+    setup();
+    expect(component.titleKey).toBe(PAGE_NOT_FOUND_ROUTE_DATA.titleKey);
+    expect(component.messageKey).toBe(PAGE_NOT_FOUND_ROUTE_DATA.messageKey);
+  });
+
+  it('should use page not found route data when provided', () => {
+    setup(PAGE_NOT_FOUND_ROUTE_DATA);
+    expect(component.titleKey).toBe(PAGE_NOT_FOUND_ROUTE_DATA.titleKey);
+    expect(component.messageKey).toBe(PAGE_NOT_FOUND_ROUTE_DATA.messageKey);
+  });
+
+  it('should use content deleted route data when provided', () => {
+    setup(CONTENT_DELETED_ROUTE_DATA);
+    expect(component.titleKey).toBe(CONTENT_DELETED_ROUTE_DATA.titleKey);
+    expect(component.messageKey).toBe(CONTENT_DELETED_ROUTE_DATA.messageKey);
+  });
+
+  it('should render a contact support mailto link', () => {
+    setup();
+    const link = fixture.nativeElement.querySelector('a') as HTMLAnchorElement;
+    expect(link.getAttribute('href')).toBe('mailto:support@test.com');
+  });
 });

src/app/core/components/page-not-found/page-not-found.component.ts

@@ -1,9 +1,12 @@
 import { TranslatePipe } from '@ngx-translate/core';
 
 import { ChangeDetectionStrategy, Component, inject } from '@angular/core';
+import { ActivatedRoute } from '@angular/router';
 
 import { ENVIRONMENT } from '@core/provider/environment.provider';
 
+import { PAGE_NOT_FOUND_ROUTE_DATA } from '../../constants/page-not-found.constants';
+
 @Component({
   selector: 'osf-page-not-found',
   imports: [TranslatePipe],
@@ -13,5 +16,9 @@ import { ENVIRONMENT } from '@core/provider/environment.provider';
 })
 export class PageNotFoundComponent {
   private readonly environment = inject(ENVIRONMENT);
+  private readonly route = inject(ActivatedRoute);
+
   readonly supportEmail = this.environment.supportEmail;
+  readonly titleKey = this.route.snapshot.data['titleKey'] ?? PAGE_NOT_FOUND_ROUTE_DATA.titleKey;
+  readonly messageKey = this.route.snapshot.data['messageKey'] ?? PAGE_NOT_FOUND_ROUTE_DATA.messageKey;
 }

src/app/core/components/request-access/request-access.component.spec.ts

@@ -118,8 +118,19 @@ describe('RequestAccessComponent', () => {
     expect(loaderServiceMock.show).toHaveBeenCalled();
     expect(requestAccessServiceMock.requestAccessToProject).toHaveBeenCalledWith('project-123', 'please grant access');
     expect(loaderServiceMock.hide).toHaveBeenCalled();
-    expect(routerMock.navigate).toHaveBeenCalledWith(['/']);
     expect(toastServiceMock.showSuccess).toHaveBeenCalledWith('requestAccess.requestedSuccessMessage');
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/']);
+    expect(toastServiceMock.showSuccess.mock.invocationCallOrder[0]).toBeLessThan(
+      (routerMock.navigate as Mock).mock.invocationCallOrder[0]
+    );
+  });
+
+  it('should not request access when route id is missing', () => {
+    setup({ routeId: '' });
+    component.requestAccess();
+
+    expect(loaderServiceMock.show).not.toHaveBeenCalled();
+    expect(requestAccessServiceMock.requestAccessToProject).not.toHaveBeenCalled();
   });
 
   it('should show already requested error on 409', () => {
@@ -129,6 +140,7 @@ describe('RequestAccessComponent', () => {
     component.requestAccess();
 
     expect(loaderServiceMock.show).toHaveBeenCalled();
+    expect(loaderServiceMock.hide).toHaveBeenCalled();
     expect(toastServiceMock.showError).toHaveBeenCalledWith('requestAccess.alreadyRequestedMessage');
     expect(routerMock.navigate).not.toHaveBeenCalled();
   });
@@ -139,6 +151,7 @@ describe('RequestAccessComponent', () => {
     });
     component.requestAccess();
 
+    expect(loaderServiceMock.hide).toHaveBeenCalled();
     expect(toastServiceMock.showError).not.toHaveBeenCalled();
     expect(routerMock.navigate).not.toHaveBeenCalled();
   });