fix: gracefully handle mixed formats in find-duplicates and fix network errors

The find-duplicates tool failed entirely when any uploaded file couldn't be processed, returning "Duplicate detection failed" or a format-specific error that aborted the whole batch. With mixed-format uploads (77 files), this made the tool unusable. - Skip unprocessable files instead of aborting; return skippedFiles in response - Switch from fetch() to XHR with upload progress tracking (Uploading X%) - Add Vite proxy timeout config (5min) to prevent connection drops on large uploads - Add "Download Grouped" button: ZIP with each duplicate group in its own folder - Add collapsible skipped-files section in the results UI - Add 3 integration tests for skip behavior (43 total)
2026-08-03 07:46:42 +02:00 · 2026-05-13 11:35:33 +08:00
parent 3c3682661e
commit d22bebb8ad
5 changed files with 336 additions and 72 deletions
@@ -153,21 +153,21 @@ export function registerFindDuplicates(app: FastifyInstance) {
    }

    try {
+      const skippedFiles: Array<{ filename: string; reason: string }> = [];
+      const processableFiles: FileData[] = [];
+
      for (const file of files) {
        const validation = await validateImageBuffer(file.buffer, file.filename);
        if (!validation.valid) {
-          return reply
-            .status(400)
-            .send({ error: `Invalid file "${file.filename}": ${validation.reason}` });
+          skippedFiles.push({ filename: file.filename, reason: validation.reason });
+          continue;
        }
        if (validation.format === "heif") {
          try {
            file.buffer = await decodeHeic(file.buffer);
-          } catch (err) {
-            return reply.status(422).send({
-              error: `Failed to decode "${file.filename}" (HEIC). Ensure libheif-examples is installed.`,
-              details: err instanceof Error ? err.message : String(err),
-            });
+          } catch {
+            skippedFiles.push({ filename: file.filename, reason: "Failed to decode HEIC" });
+            continue;
          }
        }
        if (needsCliDecode(validation.format)) {
@@ -177,11 +177,12 @@ export function registerFindDuplicates(app: FastifyInstance) {
          } catch {
            try {
              await sharp(file.buffer).metadata();
-            } catch (err) {
-              return reply.status(422).send({
-                error: `Failed to decode "${file.filename}" (${validation.format.toUpperCase()})`,
-                details: err instanceof Error ? err.message : String(err),
+            } catch {
+              skippedFiles.push({
+                filename: file.filename,
+                reason: `Failed to decode ${validation.format.toUpperCase()}`,
              });
+              continue;
            }
          }
        }
@@ -189,21 +190,53 @@ export function registerFindDuplicates(app: FastifyInstance) {
          try {
            file.buffer = decompressSvgz(file.buffer);
            file.buffer = sanitizeSvg(file.buffer);
-          } catch (err) {
-            return reply.status(400).send({
-              error: `Invalid SVG "${file.filename}": ${err instanceof Error ? err.message : "Unknown error"}`,
-            });
+          } catch {
+            skippedFiles.push({ filename: file.filename, reason: "Invalid SVG" });
+            continue;
          }
        }
-        file.buffer = await autoOrient(file.buffer);
+        try {
+          file.buffer = await autoOrient(file.buffer);
+        } catch {
+          skippedFiles.push({
+            filename: file.filename,
+            reason: "Failed to read image orientation",
+          });
+          continue;
+        }
+        processableFiles.push(file);
+      }
+
+      if (processableFiles.length < 2) {
+        return reply.status(400).send({
+          error:
+            processableFiles.length === 0
+              ? "No supported images found"
+              : "At least 2 processable images are required for duplicate detection",
+          skippedFiles,
+        });
      }

      // Extract metadata, thumbnails, and compute hashes
      const fileInfos: FileInfo[] = [];
-      for (const file of files) {
-        const info = await extractFileInfo(file);
-        info.hash = await computeDHash128(file.buffer);
-        fileInfos.push(info);
+      for (const file of processableFiles) {
+        try {
+          const info = await extractFileInfo(file);
+          info.hash = await computeDHash128(file.buffer);
+          fileInfos.push(info);
+        } catch {
+          skippedFiles.push({ filename: file.filename, reason: "Failed to compute image hash" });
+        }
+      }
+
+      if (fileInfos.length < 2) {
+        return reply.status(400).send({
+          error:
+            fileInfos.length === 0
+              ? "No images could be analyzed"
+              : "At least 2 processable images are required for duplicate detection",
+          skippedFiles,
+        });
      }

      // Group duplicates by hamming distance
@@ -292,10 +325,11 @@ export function registerFindDuplicates(app: FastifyInstance) {
      }

      return reply.send({
-        totalImages: files.length,
+        totalImages: fileInfos.length,
        duplicateGroups: groups,
-        uniqueImages: files.length - assigned.size,
+        uniqueImages: fileInfos.length - assigned.size,
        spaceSaveable,
+        skippedFiles: skippedFiles.length > 0 ? skippedFiles : undefined,
      });
    } catch (err) {
      return reply.status(422).send({
@@ -1,5 +1,5 @@
-import { Download, Loader2 } from "lucide-react";
-import { useCallback, useEffect, useState } from "react";
+import { Download, FolderArchive, Loader2 } from "lucide-react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { formatHeaders } from "@/lib/api";
 import { formatFileSize } from "@/lib/download";
 import type { DuplicateResult } from "@/stores/duplicate-store";
@@ -28,14 +28,22 @@ export function FindDuplicatesSettings() {
  const [preset, setPreset] = useState<Preset | null>("similar");
  const [threshold, setThreshold] = useState(8);
  const [error, setError] = useState<string | null>(null);
+  const [uploadProgress, setUploadProgress] = useState(0);
+  const xhrRef = useRef<XMLHttpRequest | null>(null);

-  // Reset scan results when files change
  // biome-ignore lint/correctness/useExhaustiveDependencies: files is a store value that triggers reset when changed
  useEffect(() => {
    resetDuplicates();
    setError(null);
+    setUploadProgress(0);
  }, [files, resetDuplicates]);

+  useEffect(() => {
+    return () => {
+      xhrRef.current?.abort();
+    };
+  }, []);
+
  const handlePreset = (p: Preset) => {
    setPreset(p);
    setThreshold(PRESET_THRESHOLDS[p]);
@@ -43,45 +51,74 @@ export function FindDuplicatesSettings() {

  const handleSlider = (val: number) => {
    setThreshold(val);
-    // Deselect preset if slider doesn't match any
    const match = (Object.entries(PRESET_THRESHOLDS) as [Preset, number][]).find(
      ([, t]) => t === val,
    );
    setPreset(match ? match[0] : null);
  };

-  const handleScan = async () => {
+  const handleScan = () => {
    if (files.length < 2) return;

    setScanning(true);
    setError(null);
    setResults(null);
+    setUploadProgress(0);

-    try {
-      const formData = new FormData();
-      for (const file of files) {
-        formData.append("file", file);
-      }
-      formData.append("threshold", String(threshold));
-
-      const res = await fetch("/api/v1/tools/find-duplicates", {
-        method: "POST",
-        headers: formatHeaders(),
-        body: formData,
-      });
-
-      if (!res.ok) {
-        const body = await res.json().catch(() => ({}));
-        throw new Error(body.error || `Failed: ${res.status}`);
-      }
-
-      const data: DuplicateResult = await res.json();
-      setResults(data);
-    } catch (err) {
-      setError(err instanceof Error ? err.message : "Detection failed");
-    } finally {
-      setScanning(false);
+    const formData = new FormData();
+    for (const file of files) {
+      formData.append("file", file);
    }
+    formData.append("threshold", String(threshold));
+
+    const xhr = new XMLHttpRequest();
+    xhrRef.current = xhr;
+
+    xhr.upload.onprogress = (e) => {
+      if (e.lengthComputable) {
+        setUploadProgress(Math.round((e.loaded / e.total) * 100));
+      }
+    };
+
+    xhr.onload = () => {
+      xhrRef.current = null;
+      if (xhr.status >= 200 && xhr.status < 300) {
+        try {
+          const data: DuplicateResult = JSON.parse(xhr.responseText);
+          setResults(data);
+        } catch {
+          setError("Failed to parse scan results");
+        }
+      } else {
+        try {
+          const body = JSON.parse(xhr.responseText);
+          setError(body.error || `Failed: ${xhr.status}`);
+        } catch {
+          setError(`Failed: ${xhr.status}`);
+        }
+      }
+      setScanning(false);
+    };
+
+    xhr.onerror = () => {
+      xhrRef.current = null;
+      setError("Network error during upload. Try with fewer files or check connection.");
+      setScanning(false);
+    };
+
+    xhr.ontimeout = () => {
+      xhrRef.current = null;
+      setError("Request timed out. Try with fewer files.");
+      setScanning(false);
+    };
+
+    xhr.open("POST", "/api/v1/tools/find-duplicates");
+    xhr.timeout = 300_000;
+    const headers = formatHeaders();
+    headers.forEach((value, key) => {
+      xhr.setRequestHeader(key, value);
+    });
+    xhr.send(formData);
  };

  const handleDownloadUnique = useCallback(async () => {
@@ -89,7 +126,6 @@ export function FindDuplicatesSettings() {

    const { zipSync } = await import("fflate");

-    // Collect filenames of "best" per group (respecting overrides) + all non-duplicate files
    const duplicateFilenames = new Set<string>();
    const bestFilenames = new Set<string>();
    for (let gi = 0; gi < results.duplicateGroups.length; gi++) {
@@ -122,6 +158,63 @@ export function FindDuplicatesSettings() {
    URL.revokeObjectURL(url);
  }, [files, results, bestOverrides]);

+  const handleDownloadGrouped = useCallback(async () => {
+    if (!results || results.duplicateGroups.length === 0) return;
+
+    const { zipSync } = await import("fflate");
+
+    const duplicateFilenames = new Set<string>();
+    const zipData: Record<string, Uint8Array> = {};
+    const usedPaths = new Set<string>();
+
+    const uniquePath = (dir: string, name: string): string => {
+      let path = `${dir}/${name}`;
+      if (!usedPaths.has(path)) {
+        usedPaths.add(path);
+        return path;
+      }
+      const dot = name.lastIndexOf(".");
+      const base = dot > 0 ? name.slice(0, dot) : name;
+      const ext = dot > 0 ? name.slice(dot) : "";
+      let i = 2;
+      while (usedPaths.has(path)) {
+        path = `${dir}/${base}-${i}${ext}`;
+        i++;
+      }
+      usedPaths.add(path);
+      return path;
+    };
+
+    for (let gi = 0; gi < results.duplicateGroups.length; gi++) {
+      const group = results.duplicateGroups[gi];
+      const similarity = Math.max(...group.files.map((f) => f.similarity));
+      const folderName = `group-${gi + 1}-${similarity}pct`;
+
+      for (const gf of group.files) {
+        duplicateFilenames.add(gf.filename);
+        const file = files.find((f) => f.name === gf.filename);
+        if (!file) continue;
+        const buf = await file.arrayBuffer();
+        zipData[uniquePath(folderName, file.name)] = new Uint8Array(buf);
+      }
+    }
+
+    const uniqueFiles = files.filter((f) => !duplicateFilenames.has(f.name));
+    for (const file of uniqueFiles) {
+      const buf = await file.arrayBuffer();
+      zipData[uniquePath("unique", file.name)] = new Uint8Array(buf);
+    }
+
+    const zipped = zipSync(zipData);
+    const blob = new Blob([zipped as Uint8Array<ArrayBuffer>], { type: "application/zip" });
+    const url = URL.createObjectURL(blob);
+    const a = document.createElement("a");
+    a.href = url;
+    a.download = "duplicates-grouped.zip";
+    a.click();
+    URL.revokeObjectURL(url);
+  }, [files, results]);
+
  const handleDownloadAll = useCallback(async () => {
    const { zipSync } = await import("fflate");

@@ -204,18 +297,32 @@ export function FindDuplicatesSettings() {

      {error && <p className="text-xs text-red-500">{error}</p>}

-      {/* Scan button */}
+      {/* Scan button + progress */}
      {!results && (
-        <button
-          type="button"
-          data-testid="find-duplicates-submit"
-          onClick={handleScan}
-          disabled={!hasFiles || scanning}
-          className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium disabled:opacity-50 disabled:cursor-not-allowed flex items-center justify-center gap-2"
-        >
-          {scanning && <Loader2 className="h-4 w-4 animate-spin" />}
-          {scanning ? "Scanning..." : `Scan ${files.length} Images`}
-        </button>
+        <>
+          <button
+            type="button"
+            data-testid="find-duplicates-submit"
+            onClick={handleScan}
+            disabled={!hasFiles || scanning}
+            className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium disabled:opacity-50 disabled:cursor-not-allowed flex items-center justify-center gap-2"
+          >
+            {scanning && <Loader2 className="h-4 w-4 animate-spin" />}
+            {scanning
+              ? uploadProgress < 100
+                ? `Uploading... ${uploadProgress}%`
+                : "Analyzing..."
+              : `Scan ${files.length} Images`}
+          </button>
+          {scanning && (
+            <div className="w-full h-1.5 rounded-full bg-muted overflow-hidden">
+              <div
+                className="h-full bg-primary rounded-full transition-all duration-300"
+                style={{ width: `${uploadProgress < 100 ? uploadProgress : 100}%` }}
+              />
+            </div>
+          )}
+        </>
      )}

      {/* Results: summary + actions */}
@@ -243,25 +350,48 @@ export function FindDuplicatesSettings() {
                </span>
              </div>
            )}
+            {results.skippedFiles && results.skippedFiles.length > 0 && (
+              <div className="flex justify-between">
+                <span className="text-muted-foreground">Skipped</span>
+                <span className="text-orange-500 font-medium">{results.skippedFiles.length}</span>
+              </div>
+            )}
          </div>

+          {results.skippedFiles && results.skippedFiles.length > 0 && (
+            <details className="text-xs">
+              <summary className="text-orange-500 cursor-pointer">
+                {results.skippedFiles.length} file{results.skippedFiles.length > 1 ? "s" : ""} could
+                not be analyzed
+              </summary>
+              <ul className="mt-1.5 space-y-0.5 text-muted-foreground">
+                {results.skippedFiles.map((sf) => (
+                  <li key={sf.filename} className="truncate" title={`${sf.filename}: ${sf.reason}`}>
+                    {sf.filename}
+                  </li>
+                ))}
+              </ul>
+            </details>
+          )}
+
          {/* Download actions */}
          {results.duplicateGroups.length > 0 && (
            <>
              <button
                type="button"
-                onClick={handleDownloadUnique}
+                onClick={handleDownloadGrouped}
                className="w-full py-2.5 rounded-lg bg-primary text-primary-foreground font-medium flex items-center justify-center gap-2"
              >
-                <Download className="h-4 w-4" />
-                Download Unique Only
+                <FolderArchive className="h-4 w-4" />
+                Download Grouped
              </button>
              <button
                type="button"
-                onClick={handleDownloadAll}
+                onClick={handleDownloadUnique}
                className="w-full py-2.5 rounded-lg border border-primary text-primary font-medium flex items-center justify-center gap-2 hover:bg-primary/5"
              >
-                Download All
+                <Download className="h-4 w-4" />
+                Download Unique Only
              </button>
            </>
          )}
@@ -272,6 +402,7 @@ export function FindDuplicatesSettings() {
            onClick={() => {
              resetDuplicates();
              setError(null);
+              setUploadProgress(0);
            }}
            className="w-full py-2 rounded-lg border border-border text-muted-foreground text-xs hover:text-foreground hover:border-foreground/20"
          >
@@ -16,11 +16,17 @@ export interface DuplicateGroup {
  files: DuplicateFileInfo[];
 }

+export interface SkippedFile {
+  filename: string;
+  reason: string;
+}
+
 export interface DuplicateResult {
  totalImages: number;
  uniqueImages: number;
  spaceSaveable: number;
  duplicateGroups: DuplicateGroup[];
+  skippedFiles?: SkippedFile[];
 }

 interface DuplicateState {
@@ -15,7 +15,11 @@ export default defineConfig({
    host: true,
    port: Number(process.env.PORT) || 1351,
    proxy: {
-      "/api": process.env.VITE_API_URL || "http://localhost:13490",
+      "/api": {
+        target: process.env.VITE_API_URL || "http://localhost:13490",
+        timeout: 300_000,
+        proxyTimeout: 300_000,
+      },
    },
  },
  build: {
@@ -592,9 +592,10 @@ describe("Find Duplicates", () => {
      body,
    });

-    // Should either process (with sharp handling corrupt data gracefully)
+    // Should either process (with sharp handling corrupt data gracefully),
+    // return 400 (corrupt files skipped, not enough processable images),
    // or return 422 for processing failure
-    expect([200, 422]).toContain(res.statusCode);
+    expect([200, 400, 422]).toContain(res.statusCode);
  });

  // ── Branch coverage: thumbnail generation for different formats ──────
@@ -1108,6 +1109,94 @@ describe("Find Duplicates", () => {

  // ── Threshold at 1 ────────────────────────────────────────────────

+  // ── Skip behavior: mixed valid/invalid files ─────────────────────
+
+  it("skips unsupported files and processes valid ones in the same batch", async () => {
+    const corruptBuf = Buffer.from("not an image at all");
+    const { body, contentType } = createMultipartPayload([
+      { name: "file", filename: "good1.png", contentType: "image/png", content: PNG },
+      { name: "file", filename: "good2.png", contentType: "image/png", content: PNG },
+      {
+        name: "file",
+        filename: "bad.xyz",
+        contentType: "application/octet-stream",
+        content: corruptBuf,
+      },
+      { name: "file", filename: "good3.jpg", contentType: "image/jpeg", content: JPG },
+    ]);
+
+    const res = await app.inject({
+      method: "POST",
+      url: "/api/v1/tools/find-duplicates",
+      headers: {
+        authorization: `Bearer ${adminToken}`,
+        "content-type": contentType,
+      },
+      body,
+    });
+
+    expect(res.statusCode).toBe(200);
+    const result = JSON.parse(res.body);
+    expect(result.totalImages).toBe(3);
+    expect(result.skippedFiles).toHaveLength(1);
+    expect(result.skippedFiles[0].filename).toBe("bad.xyz");
+    expect(result.skippedFiles[0].reason).toBeTruthy();
+    expect(result.duplicateGroups).toHaveLength(1);
+  });
+
+  it("returns 400 when all files are unsupported", async () => {
+    const garbage = Buffer.from("not an image");
+    const { body, contentType } = createMultipartPayload([
+      {
+        name: "file",
+        filename: "a.xyz",
+        contentType: "application/octet-stream",
+        content: garbage,
+      },
+      {
+        name: "file",
+        filename: "b.xyz",
+        contentType: "application/octet-stream",
+        content: garbage,
+      },
+    ]);
+
+    const res = await app.inject({
+      method: "POST",
+      url: "/api/v1/tools/find-duplicates",
+      headers: {
+        authorization: `Bearer ${adminToken}`,
+        "content-type": contentType,
+      },
+      body,
+    });
+
+    expect(res.statusCode).toBe(400);
+    const result = JSON.parse(res.body);
+    expect(result.skippedFiles).toHaveLength(2);
+  });
+
+  it("omits skippedFiles from response when all files are valid", async () => {
+    const { body, contentType } = createMultipartPayload([
+      { name: "file", filename: "a.png", contentType: "image/png", content: PNG },
+      { name: "file", filename: "b.png", contentType: "image/png", content: PNG },
+    ]);
+
+    const res = await app.inject({
+      method: "POST",
+      url: "/api/v1/tools/find-duplicates",
+      headers: {
+        authorization: `Bearer ${adminToken}`,
+        "content-type": contentType,
+      },
+      body,
+    });
+
+    expect(res.statusCode).toBe(200);
+    const result = JSON.parse(res.body);
+    expect(result.skippedFiles).toBeUndefined();
+  });
+
  it("uses threshold 1 for very strict duplicate matching", async () => {
    const { body, contentType } = createMultipartPayload([
      { name: "file", filename: "a.png", contentType: "image/png", content: PNG },