feat(library): stable error codes on the legacy library routes

CLAUDE.md

@@ -544,10 +544,32 @@ public async Handler(req: IRequest, res: IResponse): Promise<IResponse> {
 | 200 | Success |
 | 400 | Bad request / Operation failed |
 | 403 | Forbidden (auth failed) |
+| 404 | `item_not_found` (library routes) |
 | 409 | Conflict (duplicate) |
 | 422 | Validation error |
 | 500 | Server error |
 
+### Error codes (`error`)
+
+A 4xx that can never succeed as sent carries a stable machine-readable code next to the message:
+`{ message, error }` (`src/types/apiError.ts`, and `UploadErrorCode` for `/upload/*`). It's the key the passkey
+routes already used, and iOS decodes it into `networkErrorWithCode`. Both apps retry a failed sync task forever, so
+the code is what lets them stop and show the failure; a 4xx without one keeps being retried. Throw an `ApiError`
+from services and let the controller's `sendLibraryError` answer it.
+
+| error | HTTP | Meaning |
+|---|---|---|
+| `not_subscribed` | 400 | `checkSubscription` failed, confirmed by RevenueCat. If RC couldn't be reached, the same 400 goes out without the code |
+| `tier_required` | 403 | Subscribed, but not on a tier with this feature, confirmed live by RevenueCat; same rule when RC is unreachable |
+| `item_not_found` | 404 | No row, active or deleted, has the uuid (or, without one, the key) the request names. An item the user **deleted** answers success with nothing changed (its intent no longer applies), checked through `LibraryService.confirmDeleted` |
+| `uuid_conflict` | 409 | `PUT /`: the uuid already belongs to an item of a different type |
+| `invalid_request` | 422 | Failed body validation (`validateBody`) |
+
+Removals of something already gone answer success: deleting a bookmark, unlinking an external resource, or
+`folder_in_out` on a folder that isn't there. A failed DB read on these routes is a 500 (`LibraryLookupError`), never
+a "not found" the apps would stop on. The audit log doesn't record `not_subscribed` / `tier_required` rejections: they are
+the same stuck task retrying every 5 seconds, and RevenueCat answers the account's state.
+
 ## Logging
 
 ```typescript

src/__tests__/controllers/LibraryController.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, beforeEach, jest } from '@jest/globals';
 import { LibraryController } from '../../controllers/LibraryController';
-import { LibraryLookupError } from '../../services/LibraryService';
+import { ITEM_DELETED, LibraryLookupError } from '../../services/LibraryService';
+import { ApiError, ApiErrorCode } from '../../types/apiError';
 import { mockLoggerService } from '../setup';
 
 function makeRes() {
@@ -197,3 +198,177 @@ describe('LibraryController.getLastPlayedItem — error mapping mirrors the list
     expect(res.json).toHaveBeenCalledWith({ lastItemPlayed: null });
   });
 });
+
+// A request naming an item with no active row: deleted → success with nothing
+// changed; never existed → 404 `item_not_found`; failed read → 500.
+describe('LibraryController — legacy routes naming a missing item', () => {
+  let libraryService: any;
+  let libraryDB: any;
+  let controller: LibraryController;
+  const uuid = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa';
+  const notFound = new ApiError(ApiErrorCode.ITEM_NOT_FOUND, 404, `Item not found: "${uuid}"`);
+
+  beforeEach(() => {
+    libraryService = {
+      confirmDeleted: jest.fn(async () => true),
+      thumbnailPutRequest: jest.fn(),
+      renameLibraryObject: jest.fn(),
+      deleteFolderMoving: jest.fn(),
+      moveLibraryObjectByUuid: jest.fn(),
+      putExternalResource: jest.fn(),
+      deleteExternalResource: jest.fn(),
+    };
+    libraryDB = {
+      getLibraryByUuid: jest.fn(async () => []),
+      getLibrary: jest.fn(async () => []),
+      upsertBookmark: jest.fn(),
+      deactivateBookmark: jest.fn(),
+    };
+    controller = new LibraryController(libraryService, libraryDB);
+    (controller as any)._logger = mockLoggerService;
+    mockLoggerService.log.mockClear();
+  });
+
+  const request = (body: Record<string, unknown>) =>
+    ({ body, user: { id_user: 1, email: 'user@example.com' } }) as any;
+
+  describe('PUT /bookmark', () => {
+    it('answers success, and writes nothing, for a bookmark on a deleted book', async () => {
+      const res = makeRes();
+      await controller.upsertBookmark(request({ uuid, key: 'Book.m4b', time: 120, active: true }), res);
+
+      expect(libraryService.confirmDeleted).toHaveBeenCalledWith(expect.anything(), { uuid, key: 'Book.m4b' });
+      expect(res.json).toHaveBeenCalledWith({ bookmark: null });
+      expect(libraryDB.upsertBookmark).not.toHaveBeenCalled();
+    });
+
+    it('answers 404 item_not_found for a bookmark on a book that never existed', async () => {
+      libraryService.confirmDeleted.mockRejectedValue(notFound);
+      const res = makeRes();
+      await controller.upsertBookmark(request({ uuid, key: 'Book.m4b', time: 120, active: true }), res);
+
+      expect(res.status).toHaveBeenCalledWith(404);
+      expect(res.json).toHaveBeenCalledWith({ message: notFound.message, error: 'item_not_found' });
+    });
+
+    it('treats deleting a bookmark on a missing book as done, deleted or not', async () => {
+      const res = makeRes();
+      await controller.upsertBookmark(request({ uuid, key: 'Book.m4b', time: 120, active: false }), res);
+
+      expect(libraryService.confirmDeleted).not.toHaveBeenCalled();
+      expect(res.json).toHaveBeenCalledWith({ bookmark: null });
+    });
+
+    it('deletes by deactivating, and answers success for a bookmark the server never had', async () => {
+      libraryDB.getLibraryByUuid.mockResolvedValue([{ id_library_item: 9, title: 'Book', key: 'Book.m4b' }]);
+      libraryDB.deactivateBookmark.mockResolvedValue(undefined);
+      const res = makeRes();
+      await controller.upsertBookmark(request({ uuid, key: 'Book.m4b', time: 120, active: false }), res);
+
+      expect(libraryDB.deactivateBookmark).toHaveBeenCalled();
+      expect(libraryDB.upsertBookmark).not.toHaveBeenCalled();
+      expect(res.json).toHaveBeenCalledWith({ bookmark: null });
+    });
+
+    it('answers 500 when the item lookup fails', async () => {
+      libraryDB.getLibraryByUuid.mockResolvedValue(null);
+      const res = makeRes();
+      await controller.upsertBookmark(request({ uuid, key: 'Book.m4b', time: 120, active: true }), res);
+
+      expect(res.status).toHaveBeenCalledWith(500);
+      expect(res.json).toHaveBeenCalledWith({ message: 'Internal error' });
+    });
+  });
+
+  it('POST /rename answers success for a deleted folder and 404 for one that never existed', async () => {
+    const res = makeRes();
+    await controller.renameLibraryObject(request({ uuid, relativePath: 'Series', newName: 'New' }), res);
+    expect(res.json).toHaveBeenCalledWith({ content: [] });
+    expect(libraryService.renameLibraryObject).not.toHaveBeenCalled();
+
+    libraryService.confirmDeleted.mockRejectedValue(notFound);
+    const res404 = makeRes();
+    await controller.renameLibraryObject(request({ uuid, relativePath: 'Series', newName: 'New' }), res404);
+    expect(res404.status).toHaveBeenCalledWith(404);
+  });
+
+  describe('DELETE /folder_in_out', () => {
+    it('treats a missing folder as already removed, deleted or never there', async () => {
+      const res = makeRes();
+      await controller.deleteFolderMoving(request({ uuid, relativePath: 'Series' }), res);
+
+      expect(res.json).toHaveBeenCalledWith({ success: true });
+      expect(libraryService.confirmDeleted).not.toHaveBeenCalled();
+      expect(libraryService.deleteFolderMoving).not.toHaveBeenCalled();
+    });
+
+    it('answers 500 when the folder lookup fails', async () => {
+      libraryDB.getLibraryByUuid.mockResolvedValue(null);
+      const res = makeRes();
+      await controller.deleteFolderMoving(request({ uuid, relativePath: 'Series' }), res);
+
+      expect(res.status).toHaveBeenCalledWith(500);
+    });
+  });
+
+  describe('POST /thumbnail_set', () => {
+    it('answers a null URL for a deleted book', async () => {
+      libraryService.thumbnailPutRequest.mockResolvedValue(ITEM_DELETED);
+      const res = makeRes();
+      await controller.itemThumbnailPutRequest(request({ uuid, relativePath: 'Book.m4b', thumbnail_name: 't.jpg' }), res);
+
+      expect(res.json).toHaveBeenCalledWith({ thumbnail_name: 't.jpg', thumbnail_url: null, uploaded: false });
+    });
+
+    it('answers 404 item_not_found for a book that never existed', async () => {
+      libraryService.thumbnailPutRequest.mockRejectedValue(notFound);
+      const res = makeRes();
+      await controller.itemThumbnailPutRequest(request({ uuid, relativePath: 'Book.m4b', thumbnail_name: 't.jpg' }), res);
+
+      expect(res.status).toHaveBeenCalledWith(404);
+      expect(res.json).toHaveBeenCalledWith({ message: notFound.message, error: 'item_not_found' });
+    });
+  });
+
+  describe('external resources', () => {
+    it('PUT /external answers an empty content for a deleted book, and 404 for one that never existed', async () => {
+      libraryService.putExternalResource.mockResolvedValueOnce(null);
+      const res = makeRes();
+      await controller.putExternalResource(request({ uuid, providerName: 'jellyfin', providerId: 'p1' }), res);
+      expect(res.json).toHaveBeenCalledWith({ content: {} });
+
+      libraryService.putExternalResource.mockRejectedValueOnce(notFound);
+      const res404 = makeRes();
+      await controller.putExternalResource(request({ uuid, providerName: 'jellyfin', providerId: 'p1' }), res404);
+      expect(res404.status).toHaveBeenCalledWith(404);
+    });
+
+    it('DELETE /external answers 500 when the lookup fails', async () => {
+      libraryService.deleteExternalResource.mockRejectedValue(new LibraryLookupError());
+      const res = makeRes();
+      await controller.deleteExternalResource(request({ uuid, providerName: 'jellyfin', providerId: 'p1' }), res);
+
+      expect(res.status).toHaveBeenCalledWith(500);
+      expect(res.json).toHaveBeenCalledWith({ message: 'Internal error' });
+    });
+  });
+
+  it('POST /move maps a coded error to its status and a failed read to 500', async () => {
+    libraryService.moveLibraryObjectByUuid.mockRejectedValueOnce(notFound);
+    const res = makeRes();
+    await controller.moveLibraryObject(request({ origin: uuid, destination: '' }), res);
+    expect(res.status).toHaveBeenCalledWith(404);
+    expect(res.json).toHaveBeenCalledWith({ message: notFound.message, error: 'item_not_found' });
+
+    libraryService.moveLibraryObjectByUuid.mockRejectedValueOnce(new LibraryLookupError());
+    const res500 = makeRes();
+    await controller.moveLibraryObject(request({ origin: uuid, destination: '' }), res500);
+    expect(res500.status).toHaveBeenCalledWith(500);
+
+    libraryService.moveLibraryObjectByUuid.mockRejectedValueOnce(new Error('The destination is invalid'));
+    const res400 = makeRes();
+    await controller.moveLibraryObject(request({ origin: uuid, destination: '' }), res400);
+    expect(res400.status).toHaveBeenCalledWith(400);
+    expect(res400.json).toHaveBeenCalledWith({ message: 'The destination is invalid' });
+  });
+});

src/__tests__/middlewares/recordSyncOperation.test.ts

@@ -17,6 +17,7 @@ import {
   jobTypeFor,
   isProgressOnlyUpdate,
   extractMessage,
+  extractErrorCode,
   sanitizeParams,
 } from '../../api/middlewares/recordSyncOperation';
 import {
@@ -131,6 +132,19 @@ describe('recordSyncOperation helpers', () => {
     });
   });
 
+  describe('extractErrorCode', () => {
+    it('pulls .error from a stringified or object body', () => {
+      expect(extractErrorCode('{"message":"You are not subscribed","error":"not_subscribed"}')).toBe('not_subscribed');
+      expect(extractErrorCode({ message: 'x', error: 'item_not_found' })).toBe('item_not_found');
+    });
+
+    it('returns null without a code, or for a body that is not JSON', () => {
+      expect(extractErrorCode({ message: 'Invalid key' })).toBeNull();
+      expect(extractErrorCode('not json')).toBeNull();
+      expect(extractErrorCode(null)).toBeNull();
+    });
+  });
+
   describe('sanitizeParams', () => {
     it('drops note/title for set_bookmark without mutating the original body', () => {
       const body = { relativePath: 'Book', time: 5, note: 'secret', title: 'chapter' };
@@ -277,6 +291,44 @@ describe('recordSyncOperation middleware', () => {
     expect(arg.item_uuid).toBe('336453c8-24e3-4298-9e8c-8b41f70ac4e7');
   });
 
+  describe('account-level rejections', () => {
+    const rejected = (id_user: number, path = '/uuids', method = 'POST') => {
+      const req: any = { method, path, route: { path }, user: { id_user }, body: { items: {} } };
+      const res = makeRes();
+      recordSyncOperation(req, res, jest.fn());
+      res.statusCode = 400;
+      res.json({ message: 'You are not subscribed', error: 'not_subscribed' });
+      res.emitFinish();
+    };
+
+    it('never records not_subscribed or tier_required', () => {
+      rejected(7);
+      rejected(8, '/move');
+
+      const req: any = { method: 'PUT', path: '/', route: { path: '/' }, user: { id_user: 7 }, body: { relativePath: 'Book.m4b' } };
+      const res = makeRes();
+      recordSyncOperation(req, res, jest.fn());
+      res.statusCode = 403;
+      // Express's res.json goes out through res.send as a JSON string.
+      res.send('{"message":"Requires one of: pro","error":"tier_required"}');
+      res.emitFinish();
+
+      expect(recordMock()).not.toHaveBeenCalled();
+    });
+
+    it('still records every other error, coded or not', () => {
+      for (let i = 0; i < 2; i += 1) {
+        const req: any = { method: 'POST', path: '/move', route: { path: '/move' }, user: { id_user: 7 }, body: { origin: 'a', destination: 'b' } };
+        const res = makeRes();
+        recordSyncOperation(req, res, jest.fn());
+        res.statusCode = 404;
+        res.json({ message: 'Item not found: "a"', error: 'item_not_found' });
+        res.emitFinish();
+      }
+      expect(recordMock()).toHaveBeenCalledTimes(2);
+    });
+  });
+
   it('does not record reads', () => {
     const req: any = {
       method: 'GET',

src/__tests__/middlewares/subscription.test.ts

@@ -61,15 +61,22 @@ describe('checkSubscription middleware', () => {
     expect(res.status).not.toHaveBeenCalled();
   });
 
-  it('returns 400 "not subscribed" when isActive returns false', async () => {
-    mockIsActive.mockResolvedValue({ active: false, verified: 'local', subscriptions: [] });
+  it('returns 400 "not subscribed" with the stop code when RC confirmed it', async () => {
+    mockIsActive.mockResolvedValue({ active: false, verified: 'rc', subscriptions: [] });
     await checkSubscription(req, res, next);
     expect(mockIsActive).toHaveBeenCalledWith('ext-1');
     expect(res.status).toHaveBeenCalledWith(400);
-    expect(res.json).toHaveBeenCalledWith({ message: 'You are not subscribed' });
+    expect(res.json).toHaveBeenCalledWith({ message: 'You are not subscribed', error: 'not_subscribed' });
     expect(next).not.toHaveBeenCalled();
   });
 
+  it('leaves out the stop code when RC could not be reached', async () => {
+    mockIsActive.mockResolvedValue({ active: false, verified: 'local', subscriptions: [] });
+    await checkSubscription(req, res, next);
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({ message: 'You are not subscribed' });
+  });
+
   it('forwards thrown errors to next()', async () => {
     const error = new Error('boom');
     mockIsActive.mockRejectedValue(error);
@@ -154,7 +161,7 @@ describe('requireSubscription middleware', () => {
 
     expect(mockFetchLiveEntitlements).toHaveBeenCalledWith('ext-1');
     expect(res.status).toHaveBeenCalledWith(403);
-    expect(res.json).toHaveBeenCalledWith({ message: 'Requires one of: pro' });
+    expect(res.json).toHaveBeenCalledWith({ message: 'Requires one of: pro', error: 'tier_required' });
     expect(next).not.toHaveBeenCalled();
   });
 
@@ -165,6 +172,8 @@ describe('requireSubscription middleware', () => {
     await requireSubscription([SubscriptionTierEnum.PRO])(req, res, next);
 
     expect(res.status).toHaveBeenCalledWith(403);
+    // Not confirmed, so no code telling the apps to stop.
+    expect(res.json).toHaveBeenCalledWith({ message: 'Requires one of: pro' });
     expect(next).not.toHaveBeenCalled();
   });
 

src/__tests__/services/LibraryDBExternalResource.test.ts

@@ -204,7 +204,8 @@ describe('LibraryDB — external_resources', () => {
         trx,
       );
 
-      expect(deleted).toBeNull();
+      // undefined = nothing matched; null is reserved for a failed query.
+      expect(deleted).toBeUndefined();
     });
 
     it('allows re-adding the same resource after a soft delete (partial unique index)', async () => {

src/__tests__/services/LibraryServiceExternalResource.test.ts

@@ -141,7 +141,9 @@ describe('LibraryService — external resource flows', () => {
       expect(row.active).toBe(false);
     });
 
-    it('throws when the library item is not found', async () => {
+    // Unlinking asks for "no link", which already holds: success, not an
+    // error the apps would retry forever.
+    it('treats a missing library item as already unlinked', async () => {
       const trx = getTestTransaction();
       const user = await createTestUser(trx);
 
@@ -152,10 +154,10 @@ describe('LibraryService — external resource flows', () => {
           'id:whatever',
           'dropbox',
         ),
-      ).rejects.toThrow();
+      ).resolves.toBeNull();
     });
 
-    it('throws when the resource does not exist on the item', async () => {
+    it('treats a resource that is not on the item as already unlinked', async () => {
       const trx = getTestTransaction();
       const user = await createTestUser(trx);
       const uuid = '99999999-9999-9999-9999-999999999999';
@@ -167,7 +169,7 @@ describe('LibraryService — external resource flows', () => {
 
       await expect(
         service.deleteExternalResource(user as any, uuid, 'id:absent', 'dropbox'),
-      ).rejects.toThrow();
+      ).resolves.toBeNull();
     });
   });
 });