mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix: resolve file library Open File bug, upload reliability, and SSE proxy timeouts (#203)
The Open File button in the Files section did nothing due to a race condition where the home page reset the file store on mount before files from handleOpenFile could render. Upload on the files page used fetch with no timeout, progress, or retry, causing silent failures on mobile and slow connections. SSE connections for job progress had no keepalive pings, allowing reverse proxies to kill idle streams.
This commit is contained in:
@@ -237,11 +237,22 @@ export async function registerProgressRoutes(app: FastifyInstance): Promise<void
|
||||
reply.raw.write(`data: ${JSON.stringify(data)}\n\n`);
|
||||
};
|
||||
|
||||
// Send keepalive comments every 20s to prevent reverse proxies
|
||||
// (Caddy, Nginx, ALBs) from killing idle SSE connections.
|
||||
const keepaliveInterval = setInterval(() => {
|
||||
try {
|
||||
reply.raw.write(": keepalive\n\n");
|
||||
} catch {
|
||||
clearInterval(keepaliveInterval);
|
||||
}
|
||||
}, 20_000);
|
||||
|
||||
// If the job already has progress, send it immediately
|
||||
const existing = jobProgressStore.get(jobId);
|
||||
if (existing) {
|
||||
sendEvent({ ...existing, type: "batch" });
|
||||
if (existing.status === "completed" || existing.status === "failed") {
|
||||
clearInterval(keepaliveInterval);
|
||||
reply.raw.end();
|
||||
return;
|
||||
}
|
||||
@@ -250,6 +261,7 @@ export async function registerProgressRoutes(app: FastifyInstance): Promise<void
|
||||
const existingSingle = singleFileCompletions.get(jobId);
|
||||
if (existingSingle) {
|
||||
sendEvent(existingSingle);
|
||||
clearInterval(keepaliveInterval);
|
||||
reply.raw.end();
|
||||
return;
|
||||
}
|
||||
@@ -268,6 +280,7 @@ export async function registerProgressRoutes(app: FastifyInstance): Promise<void
|
||||
("phase" in data && (data.phase === "complete" || data.phase === "failed"))
|
||||
) {
|
||||
ended = true;
|
||||
clearInterval(keepaliveInterval);
|
||||
const subs = listeners.get(jobId);
|
||||
if (subs) {
|
||||
subs.delete(callback);
|
||||
@@ -281,6 +294,7 @@ export async function registerProgressRoutes(app: FastifyInstance): Promise<void
|
||||
|
||||
// Clean up on client disconnect
|
||||
request.raw.on("close", () => {
|
||||
clearInterval(keepaliveInterval);
|
||||
const subs = listeners.get(jobId);
|
||||
if (subs) {
|
||||
subs.delete(callback);
|
||||
|
||||
@@ -367,6 +367,22 @@ labels:
|
||||
- "traefik.http.routers.snapotter.middlewares=snapotter-body"
|
||||
```
|
||||
|
||||
### Caddy
|
||||
|
||||
```caddyfile
|
||||
images.example.com {
|
||||
reverse_proxy localhost:1349 {
|
||||
flush_interval -1
|
||||
transport http {
|
||||
read_timeout 300s
|
||||
write_timeout 300s
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`flush_interval -1` disables response buffering, which is required for SSE progress events (batch processing, AI tools, feature installs). The extended timeouts allow large file uploads to complete without Caddy closing the connection early.
|
||||
|
||||
### Cloudflare Tunnels
|
||||
|
||||
```bash
|
||||
|
||||
@@ -119,7 +119,7 @@ export function FileDetails({ mobile = false }: FileDetailsProps) {
|
||||
if (valid.length === 0) return;
|
||||
|
||||
setFiles(valid.map((d) => d.file));
|
||||
navigate("/");
|
||||
navigate("/", { state: { fromLibrary: true } });
|
||||
|
||||
// Set serverFileId on each entry so tool processing creates new versions
|
||||
setTimeout(() => {
|
||||
|
||||
@@ -5,7 +5,7 @@ import { cn } from "@/lib/utils";
|
||||
import { useFilesPageStore } from "@/stores/files-page-store";
|
||||
|
||||
export function FileUploadArea() {
|
||||
const { uploadFiles, loading } = useFilesPageStore();
|
||||
const { uploadFiles, loading, uploadProgress } = useFilesPageStore();
|
||||
const [dragging, setDragging] = useState(false);
|
||||
|
||||
function handleDragOver(e: React.DragEvent) {
|
||||
@@ -46,7 +46,12 @@ export function FileUploadArea() {
|
||||
)}
|
||||
>
|
||||
{loading ? (
|
||||
<div className="h-10 w-10 border-2 border-primary border-t-transparent rounded-full animate-spin" />
|
||||
<div className="flex flex-col items-center gap-2">
|
||||
<div className="h-10 w-10 border-2 border-primary border-t-transparent rounded-full animate-spin" />
|
||||
{uploadProgress !== null && uploadProgress > 0 && (
|
||||
<span className="text-xs font-medium text-primary">{uploadProgress}%</span>
|
||||
)}
|
||||
</div>
|
||||
) : (
|
||||
<Upload className="h-10 w-10 text-muted-foreground" />
|
||||
)}
|
||||
@@ -54,7 +59,9 @@ export function FileUploadArea() {
|
||||
<p className="text-sm font-medium text-foreground">
|
||||
{loading ? "Uploading..." : "Drop images here"}
|
||||
</p>
|
||||
<p className="text-xs text-muted-foreground mt-1">or click to select files</p>
|
||||
<p className="text-xs text-muted-foreground mt-1">
|
||||
{loading ? "" : "or click to select files"}
|
||||
</p>
|
||||
</div>
|
||||
<input
|
||||
type="file"
|
||||
|
||||
+46
-16
@@ -237,26 +237,56 @@ export async function apiGetFileDetails(id: string): Promise<UserFileDetail> {
|
||||
return { ...res.file, versions: res.versions };
|
||||
}
|
||||
|
||||
export async function apiUploadUserFiles(
|
||||
export function apiUploadUserFiles(
|
||||
files: File[],
|
||||
onProgress?: (percent: number) => void,
|
||||
): Promise<{ files: Array<{ id: string; originalName: string; size: number; version: number }> }> {
|
||||
const formData = new FormData();
|
||||
for (const f of files) formData.append("files", f);
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetch("/api/v1/files/upload", {
|
||||
method: "POST",
|
||||
headers: formatHeaders(),
|
||||
body: formData,
|
||||
return new Promise((resolve, reject) => {
|
||||
const formData = new FormData();
|
||||
for (const f of files) formData.append("files", f);
|
||||
|
||||
const xhr = new XMLHttpRequest();
|
||||
xhr.open("POST", "/api/v1/files/upload");
|
||||
xhr.timeout = 120_000;
|
||||
|
||||
const headers = formatHeaders();
|
||||
headers.forEach((value, key) => {
|
||||
if (key.toLowerCase() !== "content-type") {
|
||||
xhr.setRequestHeader(key, value);
|
||||
}
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof TypeError) {
|
||||
useConnectionStore.getState().setDisconnected();
|
||||
|
||||
if (onProgress) {
|
||||
xhr.upload.onprogress = (e) => {
|
||||
if (e.lengthComputable) {
|
||||
onProgress(Math.round((e.loaded / e.total) * 100));
|
||||
}
|
||||
};
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (!res.ok) throw new Error(`Upload failed: ${res.status}`);
|
||||
return res.json();
|
||||
|
||||
xhr.onload = () => {
|
||||
if (xhr.status >= 200 && xhr.status < 300) {
|
||||
try {
|
||||
resolve(JSON.parse(xhr.responseText));
|
||||
} catch {
|
||||
reject(new Error("Invalid server response"));
|
||||
}
|
||||
} else {
|
||||
reject(new Error(`Upload failed: ${xhr.status}`));
|
||||
}
|
||||
};
|
||||
|
||||
xhr.onerror = () => {
|
||||
useConnectionStore.getState().setDisconnected();
|
||||
reject(new TypeError("Network error"));
|
||||
};
|
||||
|
||||
xhr.ontimeout = () => {
|
||||
reject(new Error("Upload timed out"));
|
||||
};
|
||||
|
||||
xhr.send(formData);
|
||||
});
|
||||
}
|
||||
|
||||
export async function apiDeleteUserFiles(ids: string[]): Promise<{ deleted: number }> {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { CATEGORIES, PYTHON_SIDECAR_TOOLS, TOOL_BUNDLE_MAP, TOOLS } from "@snapotter/shared";
|
||||
import { Clock, Download, Loader2 } from "lucide-react";
|
||||
import { useCallback, useEffect, useMemo } from "react";
|
||||
import { useNavigate } from "react-router-dom";
|
||||
import { useLocation, useNavigate } from "react-router-dom";
|
||||
import { ImageViewer } from "@/components/common/image-viewer";
|
||||
import { MultiImageViewer } from "@/components/common/multi-image-viewer";
|
||||
import { AppLayout } from "@/components/layout/app-layout";
|
||||
@@ -29,12 +29,17 @@ export function HomePage() {
|
||||
currentEntry,
|
||||
} = useFileStore();
|
||||
const navigate = useNavigate();
|
||||
const location = useLocation();
|
||||
const { fetch: fetchSettings, defaultToolView, loaded: settingsLoaded } = useSettingsStore();
|
||||
const { fetch: fetchFeatures, bundles, installing, queued } = useFeaturesStore();
|
||||
|
||||
useEffect(() => {
|
||||
reset();
|
||||
}, [reset]);
|
||||
if (location.state?.fromLibrary) {
|
||||
navigate(".", { replace: true, state: {} });
|
||||
} else {
|
||||
reset();
|
||||
}
|
||||
}, [reset, location.state, navigate]);
|
||||
|
||||
useEffect(() => {
|
||||
fetchSettings();
|
||||
|
||||
@@ -9,6 +9,7 @@ interface FilesPageState {
|
||||
activeTab: "recent" | "upload";
|
||||
searchQuery: string;
|
||||
loading: boolean;
|
||||
uploadProgress: number | null;
|
||||
error: string | null;
|
||||
|
||||
fetchFiles: () => Promise<void>;
|
||||
@@ -29,6 +30,7 @@ export const useFilesPageStore = create<FilesPageState>((set, get) => ({
|
||||
activeTab: "recent",
|
||||
searchQuery: "",
|
||||
loading: false,
|
||||
uploadProgress: null,
|
||||
error: null,
|
||||
|
||||
fetchFiles: async () => {
|
||||
@@ -43,14 +45,32 @@ export const useFilesPageStore = create<FilesPageState>((set, get) => ({
|
||||
},
|
||||
|
||||
uploadFiles: async (files) => {
|
||||
set({ loading: true, error: null });
|
||||
try {
|
||||
await apiUploadUserFiles(files);
|
||||
await get().fetchFiles();
|
||||
set({ activeTab: "recent" });
|
||||
} catch (err) {
|
||||
set({ error: err instanceof Error ? err.message : "Upload failed", loading: false });
|
||||
set({ loading: true, error: null, uploadProgress: 0 });
|
||||
let lastError: Error | null = null;
|
||||
for (let attempt = 0; attempt < 2; attempt++) {
|
||||
try {
|
||||
await apiUploadUserFiles(files, (percent) => {
|
||||
set({ uploadProgress: percent });
|
||||
});
|
||||
set({ uploadProgress: null });
|
||||
await get().fetchFiles();
|
||||
set({ activeTab: "recent" });
|
||||
return;
|
||||
} catch (err) {
|
||||
lastError = err instanceof Error ? err : new Error("Upload failed");
|
||||
if (attempt === 0 && lastError instanceof TypeError) {
|
||||
set({ uploadProgress: 0 });
|
||||
await new Promise((r) => setTimeout(r, 1500));
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
set({
|
||||
error: lastError?.message ?? "Upload failed",
|
||||
loading: false,
|
||||
uploadProgress: null,
|
||||
});
|
||||
},
|
||||
|
||||
deleteChecked: async () => {
|
||||
|
||||
@@ -216,35 +216,75 @@ describe("apiListFiles", () => {
|
||||
// apiUploadUserFiles
|
||||
// ==========================================================================
|
||||
describe("apiUploadUserFiles", () => {
|
||||
let xhrInstances: Array<Record<string, unknown>>;
|
||||
let OriginalXHR: typeof XMLHttpRequest;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock.mockReset();
|
||||
storageMap.clear();
|
||||
xhrInstances = [];
|
||||
OriginalXHR = globalThis.XMLHttpRequest;
|
||||
|
||||
const MockXHR = vi.fn().mockImplementation(() => {
|
||||
const instance: Record<string, unknown> = {
|
||||
open: vi.fn(),
|
||||
send: vi.fn(),
|
||||
setRequestHeader: vi.fn(),
|
||||
upload: {},
|
||||
readyState: 4,
|
||||
status: 200,
|
||||
responseText: "",
|
||||
timeout: 0,
|
||||
onload: null,
|
||||
onerror: null,
|
||||
ontimeout: null,
|
||||
};
|
||||
xhrInstances.push(instance);
|
||||
return instance;
|
||||
});
|
||||
vi.stubGlobal("XMLHttpRequest", MockXHR);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.stubGlobal("XMLHttpRequest", OriginalXHR);
|
||||
});
|
||||
|
||||
it("sends files as FormData to upload endpoint", async () => {
|
||||
const file = new File(["content"], "img.png", { type: "image/png" });
|
||||
fetchMock.mockReturnValueOnce(
|
||||
okJson({ files: [{ id: "1", originalName: "img.png", size: 7, version: 1 }] }),
|
||||
);
|
||||
const responseData = { files: [{ id: "1", originalName: "img.png", size: 7, version: 1 }] };
|
||||
|
||||
const result = await apiUploadUserFiles([file]);
|
||||
expect(fetchMock.mock.calls[0][0]).toBe("/api/v1/files/upload");
|
||||
expect(fetchMock.mock.calls[0][1].method).toBe("POST");
|
||||
const promise = apiUploadUserFiles([file]);
|
||||
const xhr = xhrInstances[0];
|
||||
expect(xhr.open).toHaveBeenCalledWith("POST", "/api/v1/files/upload");
|
||||
|
||||
xhr.status = 200;
|
||||
xhr.responseText = JSON.stringify(responseData);
|
||||
(xhr.onload as () => void)();
|
||||
|
||||
const result = await promise;
|
||||
expect(result.files).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("throws on non-ok response", async () => {
|
||||
const file = new File(["content"], "img.png", { type: "image/png" });
|
||||
fetchMock.mockReturnValueOnce(
|
||||
Promise.resolve({ ok: false, status: 413, json: () => Promise.reject(new Error("no")) }),
|
||||
);
|
||||
await expect(apiUploadUserFiles([file])).rejects.toThrow("Upload failed: 413");
|
||||
|
||||
const promise = apiUploadUserFiles([file]);
|
||||
const xhr = xhrInstances[0];
|
||||
xhr.status = 413;
|
||||
xhr.responseText = "";
|
||||
(xhr.onload as () => void)();
|
||||
|
||||
await expect(promise).rejects.toThrow("Upload failed: 413");
|
||||
});
|
||||
|
||||
it("triggers disconnected on TypeError", async () => {
|
||||
it("triggers disconnected on network error", async () => {
|
||||
const file = new File(["content"], "img.png", { type: "image/png" });
|
||||
fetchMock.mockRejectedValueOnce(new TypeError("Failed to fetch"));
|
||||
await expect(apiUploadUserFiles([file])).rejects.toThrow("Failed to fetch");
|
||||
|
||||
const promise = apiUploadUserFiles([file]);
|
||||
const xhr = xhrInstances[0];
|
||||
(xhr.onerror as () => void)();
|
||||
|
||||
await expect(promise).rejects.toThrow("Network error");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -330,7 +330,7 @@ describe("useFilesPageStore", () => {
|
||||
|
||||
await useFilesPageStore.getState().uploadFiles([file]);
|
||||
|
||||
expect(mockApiUploadUserFiles).toHaveBeenCalledWith([file]);
|
||||
expect(mockApiUploadUserFiles).toHaveBeenCalledWith([file], expect.any(Function));
|
||||
expect(mockApiListFiles).toHaveBeenCalled();
|
||||
expect(useFilesPageStore.getState().activeTab).toBe("recent");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user