fix: surface actionable delete-failure messages instead of a bare HTTP 500

photomap/backend/routers/index.py

@@ -13,6 +13,7 @@
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel
 from send2trash import send2trash
+from send2trash.exceptions import TrashPermissionError
 
 from .. import invokeai_client
 from ..config import get_config_manager
@@ -283,6 +284,52 @@ async def index_metadata(album_config: AlbumDep) -> EmbeddingsIndexMetadata:
     )
 
 
+def _remove_image_file(image_path: Path, move_to_trash: bool) -> None:
+    """Trash or unlink ``image_path``, translating OS failures into HTTP
+    errors whose ``detail`` tells the user what to fix.
+
+    Without this, a read-only folder surfaced as a bare
+    "HTTP 500 Internal Server Error" dialog with no hint that it was a
+    permissions problem.
+    """
+    try:
+        if move_to_trash:
+            send2trash(str(image_path))
+        else:
+            image_path.unlink()
+    except TrashPermissionError as e:
+        raise HTTPException(
+            status_code=403,
+            detail=(
+                f"Cannot move '{image_path.name}' to the trash: permission denied "
+                "creating or writing the trash folder. Fix its permissions, or "
+                'switch "When deleting" to "Just Delete" in Settings to delete '
+                "the file permanently."
+            ),
+        ) from e
+    except PermissionError as e:
+        raise HTTPException(
+            status_code=403,
+            detail=(
+                f"Cannot delete '{image_path.name}': permission denied. "
+                "The PhotoMap server needs write permission on the image file "
+                "and its containing folder."
+            ),
+        ) from e
+    except OSError as e:
+        action = "move to the trash" if move_to_trash else "delete"
+        reason = e.strerror or str(e)
+        hint = (
+            ' Switching "When deleting" to "Just Delete" in Settings may help.'
+            if move_to_trash
+            else ""
+        )
+        raise HTTPException(
+            status_code=500,
+            detail=f"Could not {action} '{image_path.name}': {reason}.{hint}",
+        ) from e
+
+
 @index_router.delete(
     "/delete_image/{album_key}/{index}",
     tags=["Index"],
@@ -326,10 +373,7 @@ async def delete_image(
             raise HTTPException(status_code=404, detail="File not found")
 
         print(f"{'Trashing' if move_to_trash else 'Deleting'} image: {image_path}")
-        if move_to_trash:
-            send2trash(str(image_path))
-        else:
-            image_path.unlink()
+        _remove_image_file(image_path, move_to_trash)
 
         # Remove from embeddings
         embeddings.remove_image_from_embeddings(index)

photomap/frontend/static/javascript/bookmarks.js

@@ -9,7 +9,7 @@ import { showConfirmModal } from "./modal-utils.js";
 import { setSearchResults } from "./search.js";
 import { slideState } from "./slide-state.js";
 import { state } from "./state.js";
-import { fetchJson, hideSpinner, setCheckmarkOnIcon, showSpinner } from "./utils.js";
+import { errorDetail, fetchJson, hideSpinner, setCheckmarkOnIcon, showSpinner } from "./utils.js";
 
 // SVG icons for bookmark actions
 const BOOKMARK_SVG = `<svg width="32" height="32" viewBox="0 0 24 24" fill="none" stroke="#fff" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
@@ -603,7 +603,7 @@ class BookmarkManager {
       this.clearBookmarks();
     } catch (error) {
       console.error("Delete failed:", error);
-      alert(`Delete failed: ${error.message}`);
+      alert(`Delete failed: ${errorDetail(error)}`);
     } finally {
       hideSpinner();
     }

photomap/frontend/static/javascript/control-panel.js

@@ -3,7 +3,7 @@
 import { deleteImage, getIndexMetadata } from "./index.js";
 import { getCurrentFilepath, getCurrentSlideIndex, slideState } from "./slide-state.js";
 import { saveSettingsToLocalStorage, state } from "./state.js";
-import { hideSpinner, showSpinner } from "./utils.js";
+import { errorDetail, hideSpinner, showSpinner } from "./utils.js";
 
 // Cache DOM elements
 let elements = {};
@@ -112,7 +112,7 @@ async function handleDeleteCurrentFile() {
     hideSpinner();
   } catch (error) {
     hideSpinner();
-    alert(`Failed to delete: ${error.message}`);
+    alert(`Failed to delete: ${errorDetail(error)}`);
     console.error("Delete failed:", error);
   }
 }

photomap/frontend/static/javascript/utils.js

@@ -173,6 +173,19 @@ export class HttpError extends Error {
   }
 }
 
+/**
+ * The message to show a user for a failed request: the server's explanation
+ * (FastAPI's ``{"detail": "..."}``) when there is one, else the error's own
+ * message. Alerts that show ``error.message`` directly print the unhelpful
+ * "HTTP 500 Internal Server Error for <url>" and bury the actual reason.
+ */
+export function errorDetail(error) {
+  if (error instanceof HttpError && typeof error.body?.detail === "string") {
+    return error.body.detail;
+  }
+  return error.message;
+}
+
 /**
  * Thin wrapper around ``fetch`` + ``response.json()`` that throws
  * :class:`HttpError` on non-2xx and returns the parsed body on 2xx.

tests/backend/test_index.py

@@ -130,6 +130,92 @@ def test_delete_image(
     ).exists(), "Image file should be deleted"
 
 
+def test_delete_image_permission_error_names_file_and_cause(
+    client: TestClient, new_album: dict, monkeypatch: pytest.MonkeyPatch
+):
+    """A file the server can't remove must surface as a 403 whose detail names
+    the file and says it's a permissions problem — not the generic
+    "HTTP 500 Internal Server Error" the alert dialog used to show."""
+    build_index(client, new_album)
+    album_key = new_album["key"]
+    filename = client.get(f"/retrieve_image/{album_key}/0").json()["filename"]
+
+    def deny_unlink(self, missing_ok=False):
+        raise PermissionError(13, "Permission denied", str(self))
+
+    monkeypatch.setattr(Path, "unlink", deny_unlink)
+    response = client.delete(
+        f"/delete_image/{album_key}/0", params={"move_to_trash": False}
+    )
+    monkeypatch.undo()
+
+    assert response.status_code == 403
+    detail = response.json()["detail"]
+    assert filename in detail
+    assert "permission denied" in detail.lower()
+    assert "write permission" in detail
+
+    # The failed delete must leave both the file and its index row in place.
+    assert (Path(new_album["image_paths"][0]) / filename).exists()
+    metadata = client.get(f"/index_metadata/{album_key}").json()
+    assert metadata["filename_count"] == TEST_IMAGE_COUNT
+
+
+def test_delete_image_trash_permission_error_points_at_settings(
+    client: TestClient, new_album: dict, monkeypatch: pytest.MonkeyPatch
+):
+    """send2trash's TrashPermissionError means the file itself may be
+    deletable but the trash folder isn't usable — the message must offer the
+    "Just Delete" setting as the way out."""
+    from send2trash.exceptions import TrashPermissionError
+
+    from photomap.backend.routers import index as index_router_module
+
+    build_index(client, new_album)
+    album_key = new_album["key"]
+    filename = client.get(f"/retrieve_image/{album_key}/0").json()["filename"]
+
+    def broken_trash(path):
+        raise TrashPermissionError(path)
+
+    monkeypatch.setattr(index_router_module, "send2trash", broken_trash)
+
+    response = client.delete(f"/delete_image/{album_key}/0")  # move_to_trash default
+    assert response.status_code == 403
+    detail = response.json()["detail"]
+    assert filename in detail
+    assert "trash" in detail.lower()
+    assert "Just Delete" in detail
+    assert (Path(new_album["image_paths"][0]) / filename).exists()
+
+
+def test_delete_image_trash_oserror_surfaces_reason(
+    client: TestClient, new_album: dict, monkeypatch: pytest.MonkeyPatch
+):
+    """Non-permission trash failures (e.g. no trash dir on the file's mount)
+    keep the 500 but the detail must carry the OS reason and the Settings
+    hint instead of a bare 'Failed to delete file'."""
+    import errno
+
+    from photomap.backend.routers import index as index_router_module
+
+    build_index(client, new_album)
+    album_key = new_album["key"]
+    filename = client.get(f"/retrieve_image/{album_key}/0").json()["filename"]
+
+    def broken_trash(path):
+        raise OSError(errno.EXDEV, "Invalid cross-device link")
+
+    monkeypatch.setattr(index_router_module, "send2trash", broken_trash)
+
+    response = client.delete(f"/delete_image/{album_key}/0")
+    assert response.status_code == 500
+    detail = response.json()["detail"]
+    assert filename in detail
+    assert "Invalid cross-device link" in detail
+    assert "Just Delete" in detail
+
+
 def test_npz_rewrites_preserve_non_per_image_keys(tmp_path: Path):
     """``remove_image_from_embeddings`` and ``update_image_path`` rewrite the
     whole ``.npz``; they must carry over ``model_id``, ``embedding_dim``, and

tests/frontend/utils.test.js

@@ -9,6 +9,8 @@ import {
   debounce,
   getPercentile,
   setCheckmarkOnIcon,
+  errorDetail,
+  HttpError,
 } from "../../photomap/frontend/static/javascript/utils.js";
 
 describe("utils.js", () => {
@@ -295,4 +297,24 @@ describe("utils.js", () => {
       expect(() => setCheckmarkOnIcon(undefined, false)).not.toThrow();
     });
   });
+
+  describe("errorDetail", () => {
+    it("prefers the server's detail string for an HttpError", () => {
+      const err = new HttpError(403, "Forbidden", "delete_image/family/4425", {
+        detail: "Cannot delete 'img.jpg': permission denied.",
+      });
+      expect(errorDetail(err)).toBe("Cannot delete 'img.jpg': permission denied.");
+    });
+
+    it("falls back to the error message when there is no string detail", () => {
+      // No body at all (e.g. reading the response body failed).
+      expect(errorDetail(new HttpError(500, "Internal Server Error", "u", undefined))).toBe(
+        "HTTP 500 Internal Server Error for u"
+      );
+      // FastAPI validation errors put a list in detail — not showable as-is.
+      expect(errorDetail(new HttpError(422, "", "u", { detail: [{ loc: [] }] }))).toBe("HTTP 422 for u");
+      // Non-HTTP errors pass straight through.
+      expect(errorDetail(new Error("plain failure"))).toBe("plain failure");
+    });
+  });
 });