Write the 404 Immich streaming only returned

streamImmichAsset answered { error, status } when the effective owner had no
Immich connection stored, and left the response untouched. All four callers
ignored that return, so the client got no status line and waited out its own
timeout. Its Synology counterpart has always written through
handleServiceResult; this is the same shape.

Every other exit already went through pipeAsset, which writes on all paths
including its catch, so the method is void all the way down now — and the
PhotoProvider interface drops the error return it was carrying purely to
describe this defect.

Three cases pin it, including the one that matters for shared albums: the
connection that gets read is the OWNER's, so a viewer who happens to have
Immich configured must not paper over an owner who does not.

server/src/nest/journey/journey.service.ts

@@ -112,7 +112,7 @@ export class JourneyService {
     provider: string,
     ref: { userId: number; ownerId: number; assetId: string },
     kind: 'thumbnail' | 'original',
-  ): Promise<unknown> | null {
+  ): Promise<void> | null {
     const backend = this.providers.get(provider);
     if (!backend) return null;
     return backend.streamAsset(res, ref, kind);

server/src/nest/memories/immich.service.ts

@@ -6,7 +6,7 @@ import { checkSsrf, safeFetch } from '../../utils/ssrfGuard';
 import { AuditService } from '../audit/audit.service';
 import { DatabaseService } from '../database/database.service';
 import { MemoriesAccessService } from './memories-access.service';
-import { pipeAsset, type Selection } from './memories.helpers';
+import { fail, handleServiceResult, pipeAsset, type Selection } from './memories.helpers';
 import { UPLOADS_ROOT } from './uploads-root';
 
 const ALBUM_PAGE_SIZE = 1000;
@@ -317,6 +317,19 @@ export class ImmichService {
     }
   }
 
+  /**
+   * Proxy one asset to the response.
+   *
+   * The credential guard WRITES the 404 rather than returning it. It used to
+   * answer `{ error, status }` and leave the response untouched, and all four
+   * callers ignored that return — so a user whose Immich connection had been
+   * removed got no status line at all and the request hung until the client
+   * gave up. Its Synology counterpart has always written through
+   * `handleServiceResult`; this is the same shape.
+   *
+   * Every other exit goes through `pipeAsset`, which writes on all paths
+   * including its catch, so the method is `void` all the way down now.
+   */
   async streamImmichAsset(
     response: Response,
     userId: number,
@@ -324,10 +337,13 @@ export class ImmichService {
     kind: 'thumbnail' | 'original',
     ownerUserId?: number,
     opts?: { mediaType?: string | null; range?: string },
-  ): Promise<{ error?: string; status?: number } | void> {
+  ): Promise<void> {
     const effectiveUserId = ownerUserId ?? userId;
     const creds = this.getImmichCredentials(effectiveUserId);
-    if (!creds) return { error: 'Not found', status: 404 };
+    if (!creds) {
+      handleServiceResult(response, fail('Not found', 404));
+      return;
+    }
 
     const isVideo = opts?.mediaType === 'video';
     const headers: Record<string, string> = { 'x-api-key': creds.immich_api_key };

server/src/nest/memories/photo-provider.ts

@@ -52,13 +52,12 @@ export interface PhotoProvider {
   /**
    * Write the asset to the response, headers and status included.
    *
-   * The error return is Immich's: it answers `{ error, status }` without
-   * touching the response when the owner has no credentials stored. Every
-   * caller has always ignored that — which leaves the request open until the
-   * client times out — so the shape is carried here rather than quietly
-   * dropped, and the pre-existing handling is left alone.
+   * Void, and that is a contract rather than an oversight: a provider that
+   * cannot serve the asset writes its own error status. Immich used to return
+   * `{ error, status }` on a missing connection and leave the response
+   * untouched, which hung the request in every one of its four callers.
    */
-  streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<PhotoFetchError | void>;
+  streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<void>;
 
   /** Thumbnail bytes for the cache, rather than straight to a response. */
   fetchThumbnailBytes(ref: PhotoAssetRef): Promise<PhotoBytes | PhotoFetchError>;

server/src/nest/memories/providers/immich.provider.ts

@@ -20,14 +20,11 @@ export class ImmichPhotoProvider implements PhotoProvider {
 
   constructor(private readonly immich: ImmichService) {}
 
-  async streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<PhotoFetchError | void> {
-    const result = await this.immich.streamImmichAsset(res, ref.userId, ref.assetId, kind, ref.ownerId, {
+  streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<void> {
+    return this.immich.streamImmichAsset(res, ref.userId, ref.assetId, kind, ref.ownerId, {
       mediaType: ref.mediaType,
       range: ref.range,
     });
-    // The service types both fields optional; the one path that returns at all
-    // always fills them. Normalised here so the interface can require them.
-    if (result && result.error) return { error: result.error, status: result.status ?? 500 };
   }
 
   fetchThumbnailBytes(ref: PhotoAssetRef): Promise<PhotoBytes | PhotoFetchError> {

server/tests/unit/nest/immich.service.test.ts

@@ -397,6 +397,53 @@ describe('fetchImmichThumbnailBytes', () => {
   });
 });
 
+describe('streamImmichAsset', () => {
+  function makeRes() {
+    return { status: vi.fn().mockReturnThis(), json: vi.fn(), set: vi.fn(), end: vi.fn() };
+  }
+
+  it('IMMICH-041b: answers 404 when the owner has no connection, instead of leaving the request open', async () => {
+    // It used to return { error, status } and touch nothing. All four callers
+    // ignored that return, so the client got no status line at all and waited
+    // out its own timeout. Synology's counterpart always wrote; this matches.
+    seedUser(11, null, null);
+    const res = makeRes();
+
+    await svc.streamImmichAsset(res as never, USER, 'a1', 'thumbnail', 11);
+
+    expect(res.status).toHaveBeenCalledWith(404);
+    expect(res.json).toHaveBeenCalledWith({ error: 'Not found' });
+    expect(safeFetch).not.toHaveBeenCalled();
+  });
+
+  it('IMMICH-041c: reads the connection of the OWNER, not of the viewer', async () => {
+    // A shared album is proxied with the owner's key on the viewer's behalf, so
+    // a viewer who happens to have Immich configured must not paper over an
+    // owner who does not.
+    seedUser(USER, 'https://immich.example', 'viewer-key');
+    seedUser(12, null, null);
+    const res = makeRes();
+
+    await svc.streamImmichAsset(res as never, USER, 'a1', 'original', 12);
+
+    expect(res.status).toHaveBeenCalledWith(404);
+    expect(safeFetch).not.toHaveBeenCalled();
+  });
+
+  it('IMMICH-041d: proxies with the owner key once a connection exists', async () => {
+    seedUser(13, 'https://immich.example', 'owner-key');
+    safeFetch.mockResolvedValue(upstream({ contentType: 'image/jpeg', body: 'bytes' }));
+    const res = makeRes();
+
+    await svc.streamImmichAsset(res as never, USER, 'a1', 'thumbnail', 13);
+
+    expect(safeFetch).toHaveBeenCalledTimes(1);
+    const [url, init] = safeFetch.mock.calls[0];
+    expect(url).toContain('/api/assets/a1/thumbnail');
+    expect((init.headers as Record<string, string>)['x-api-key']).toBe('owner-key');
+  });
+});
+
 describe('collectAlbumSelection', () => {
   it('IMMICH-042: 404s when the album link does not resolve', async () => {
     access.getAlbumIdFromLink.mockReturnValue({ success: false });

server/tests/unit/nest/photo-provider.test.ts

@@ -93,19 +93,6 @@ describe('ImmichPhotoProvider', () => {
     expect(immich.fetchImmichThumbnailBytes).toHaveBeenCalledWith(7, 'asset-1', 42);
   });
 
-  it('PROV-005: normalises the credential-less answer, which the service types as optional', async () => {
-    immich.streamImmichAsset.mockResolvedValue({ error: 'Not found', status: 404 });
-    expect(await immichProvider.streamAsset(res, ref, 'original')).toEqual({ error: 'Not found', status: 404 });
-
-    immich.streamImmichAsset.mockResolvedValue({ error: 'Broken' });
-    expect(await immichProvider.streamAsset(res, ref, 'original')).toEqual({ error: 'Broken', status: 500 });
-  });
-
-  it('PROV-006: a written response reports nothing', async () => {
-    immich.streamImmichAsset.mockResolvedValue(undefined);
-    expect(await immichProvider.streamAsset(res, ref, 'thumbnail')).toBeUndefined();
-  });
-
   it('PROV-007: turns the service\'s {data,error} into a ServiceResult', async () => {
     immich.getAssetInfo.mockResolvedValue({ data: { id: 'a' } });
     expect(await immichProvider.getAssetInfo(ref)).toEqual({ success: true, data: { id: 'a' } });