VibecoderMcSwaggins commited on
Commit
c3e7865
·
1 Parent(s): 07db7cc

fix: comprehensive frontend-backend integration fixes

Browse files

Validates and fixes issues from senior code review (Gemini/CodeRabbit):

CRITICAL FIXES:
1. CORS headers for NiiVue binary fetching (TRUE - FIXED)
- Added Range to allow_headers (partial content requests)
- Added HEAD to allow_methods (preflight checks)
- Added expose_headers for Content-Range, Content-Length, Accept-Ranges
- This was blocking NiiVue's volume downloads

2. StaticFiles bypasses CORS middleware (BUG-004 - FIXED)
- Replaced StaticFiles mount with explicit files_router
- Routes now go through main app's middleware stack
- CORS/CORP headers now correctly applied to NIfTI files

HIGH PRIORITY FIXES:
3. CaseSelector cold-start retry (TRUE - FIXED)
- Added exponential backoff retry loop (matches useSegmentation.ts)
- Shows "Backend waking up..." UI during retries
- User no longer sees permanent error if backend is cold

4. Polling 404 = infinite retry (TRUE - FIXED)
- 404 now treated as TERMINAL state (job expired/lost)
- Shows actionable error: "Job expired or lost. Please try again."
- Stops infinite retry loop on HF Space restarts

MAINTENANCE FIXES:
5. Multiple RESULTS_DIR constants (TRUE - FIXED)
- Created api/config.py as single source of truth
- All modules now import RESULTS_DIR from config
- Prevents configuration drift

6. Frontend README outdated (TRUE - FIXED)
- Changed allow_origin_regex -> allow_origins + FRONTEND_ORIGIN

VALIDATED AS FALSE/NOT BUGS:
- CORS origin .static.hf.space: FALSE (HF Static uses .hf.space)
- Missing SharedArrayBuffer headers: FALSE (README has custom_headers)
- Aggressive WebGL context handling: NOT A BUG (defensive programming)
- Redundant path traversal check: NOT A BUG (defense in depth)

Tests: 69/69 frontend tests pass

Dockerfile CHANGED
@@ -75,9 +75,13 @@ EXPOSE 7860
75
  # Reset ENTRYPOINT from base image
76
  ENTRYPOINT []
77
 
78
- # Explicit frontend origin for CORS (backup to regex)
79
  ENV FRONTEND_ORIGIN=https://vibecodermcswaggins-stroke-viewer-frontend.hf.space
80
 
 
 
 
 
81
  # Run FastAPI with uvicorn (module path: stroke_deepisles_demo.api.main:app)
82
  # --proxy-headers: Trust X-Forwarded-Proto from HF Spaces proxy (ensures https:// in request.base_url)
83
  CMD ["uvicorn", "stroke_deepisles_demo.api.main:app", "--host", "0.0.0.0", "--port", "7860", "--proxy-headers"]
 
75
  # Reset ENTRYPOINT from base image
76
  ENTRYPOINT []
77
 
78
+ # Explicit frontend origin for CORS
79
  ENV FRONTEND_ORIGIN=https://vibecodermcswaggins-stroke-viewer-frontend.hf.space
80
 
81
+ # Explicit backend public URL for constructing file URLs
82
+ # This ensures correct https:// URLs even if proxy headers aren't forwarded correctly
83
+ ENV BACKEND_PUBLIC_URL=https://vibecodermcswaggins-stroke-deepisles-demo.hf.space
84
+
85
  # Run FastAPI with uvicorn (module path: stroke_deepisles_demo.api.main:app)
86
  # --proxy-headers: Trust X-Forwarded-Proto from HF Spaces proxy (ensures https:// in request.base_url)
87
  CMD ["uvicorn", "stroke_deepisles_demo.api.main:app", "--host", "0.0.0.0", "--port", "7860", "--proxy-headers"]
frontend/README.md CHANGED
@@ -99,7 +99,7 @@ If you fork this repository, update these files before deploying:
99
  ```
100
 
101
  2. **Backend CORS** (`src/stroke_deepisles_demo/api/main.py`):
102
- Update the `allow_origin_regex` to match your frontend Space URL.
103
 
104
  3. **Rebuild frontend**:
105
  ```bash
 
99
  ```
100
 
101
  2. **Backend CORS** (`src/stroke_deepisles_demo/api/main.py`):
102
+ Add your frontend URL to the `CORS_ORIGINS` list, or set `FRONTEND_ORIGIN` env var.
103
 
104
  3. **Rebuild frontend**:
105
  ```bash
frontend/src/components/CaseSelector.tsx CHANGED
@@ -1,5 +1,10 @@
1
- import { useEffect, useState } from "react";
2
- import { apiClient } from "../api/client";
 
 
 
 
 
3
 
4
  interface CaseSelectorProps {
5
  selectedCase: string | null;
@@ -13,36 +18,78 @@ export function CaseSelector({
13
  const [cases, setCases] = useState<string[]>([]);
14
  const [isLoading, setIsLoading] = useState(true);
15
  const [error, setError] = useState<string | null>(null);
 
 
16
 
17
- useEffect(() => {
18
- const abortController = new AbortController();
19
 
20
- const fetchCases = async () => {
21
  try {
22
- const data = await apiClient.getCases(abortController.signal);
23
  setCases(data.cases);
 
 
 
24
  } catch (err) {
25
- // Ignore abort errors - component unmounted
26
  if (err instanceof Error && err.name === "AbortError") return;
27
 
28
- const message = err instanceof Error ? err.message : "Unknown error";
29
- setError(`Failed to load cases: ${message}`);
30
- } finally {
31
- if (!abortController.signal.aborted) {
32
- setIsLoading(false);
 
 
 
 
 
 
 
 
 
 
 
 
 
33
  }
 
 
 
 
 
 
 
 
 
 
 
34
  }
35
- };
 
36
 
37
- fetchCases();
 
 
 
 
 
 
 
38
 
39
  return () => abortController.abort();
40
- }, []);
41
 
42
  if (isLoading) {
43
  return (
44
  <div className="bg-gray-800 rounded-lg p-4">
45
- <p className="text-gray-400">Loading cases...</p>
 
 
 
 
 
 
46
  </div>
47
  );
48
  }
 
1
+ import { useEffect, useState, useCallback } from "react";
2
+ import { apiClient, ApiError } from "../api/client";
3
+
4
+ // Cold start retry configuration (matches useSegmentation.ts)
5
+ const MAX_COLD_START_RETRIES = 5;
6
+ const INITIAL_RETRY_DELAY = 2000;
7
+ const MAX_RETRY_DELAY = 30000;
8
 
9
  interface CaseSelectorProps {
10
  selectedCase: string | null;
 
18
  const [cases, setCases] = useState<string[]>([]);
19
  const [isLoading, setIsLoading] = useState(true);
20
  const [error, setError] = useState<string | null>(null);
21
+ const [retryCount, setRetryCount] = useState(0);
22
+ const [isWakingUp, setIsWakingUp] = useState(false);
23
 
24
+ const fetchCases = useCallback(async (signal: AbortSignal) => {
25
+ let attempts = 0;
26
 
27
+ while (attempts <= MAX_COLD_START_RETRIES) {
28
  try {
29
+ const data = await apiClient.getCases(signal);
30
  setCases(data.cases);
31
+ setIsWakingUp(false);
32
+ setRetryCount(0);
33
+ return; // Success
34
  } catch (err) {
 
35
  if (err instanceof Error && err.name === "AbortError") return;
36
 
37
+ const is503 = err instanceof ApiError && err.status === 503;
38
+ const isNetworkError =
39
+ err instanceof TypeError &&
40
+ err.message.toLowerCase().includes("fetch");
41
+
42
+ // Retry on cold start (503) or network errors
43
+ if ((is503 || isNetworkError) && attempts < MAX_COLD_START_RETRIES) {
44
+ attempts++;
45
+ setRetryCount(attempts);
46
+ setIsWakingUp(true);
47
+
48
+ // Exponential backoff
49
+ const delay = Math.min(
50
+ INITIAL_RETRY_DELAY * Math.pow(2, attempts - 1),
51
+ MAX_RETRY_DELAY,
52
+ );
53
+ await new Promise((resolve) => setTimeout(resolve, delay));
54
+ continue;
55
  }
56
+
57
+ // Max retries exceeded or non-retryable error
58
+ const message =
59
+ is503 || isNetworkError
60
+ ? "Backend failed to wake up. Please refresh the page."
61
+ : err instanceof Error
62
+ ? err.message
63
+ : "Unknown error";
64
+ setError(`Failed to load cases: ${message}`);
65
+ setIsWakingUp(false);
66
+ return;
67
  }
68
+ }
69
+ }, []);
70
 
71
+ useEffect(() => {
72
+ const abortController = new AbortController();
73
+
74
+ fetchCases(abortController.signal).finally(() => {
75
+ if (!abortController.signal.aborted) {
76
+ setIsLoading(false);
77
+ }
78
+ });
79
 
80
  return () => abortController.abort();
81
+ }, [fetchCases]);
82
 
83
  if (isLoading) {
84
  return (
85
  <div className="bg-gray-800 rounded-lg p-4">
86
+ {isWakingUp ? (
87
+ <p className="text-yellow-400">
88
+ Backend waking up... Retry {retryCount}/{MAX_COLD_START_RETRIES}
89
+ </p>
90
+ ) : (
91
+ <p className="text-gray-400">Loading cases...</p>
92
+ )}
93
  </div>
94
  );
95
  }
frontend/src/hooks/useSegmentation.ts CHANGED
@@ -111,7 +111,19 @@ export function useSegmentation() {
111
  // Ignore abort errors
112
  if (err instanceof Error && err.name === "AbortError") return;
113
 
114
- // Don't stop polling on transient network errors - retry next interval
 
 
 
 
 
 
 
 
 
 
 
 
115
  console.warn("Polling error (will retry):", err);
116
  }
117
  },
 
111
  // Ignore abort errors
112
  if (err instanceof Error && err.name === "AbortError") return;
113
 
114
+ // 404 = job expired or lost (HF restart) - this is TERMINAL, not transient
115
+ if (err instanceof ApiError && err.status === 404) {
116
+ stopPolling();
117
+ setIsLoading(false);
118
+ setJobStatus("failed");
119
+ setError(
120
+ "Job expired or lost. This can happen if the backend restarted. Please try again.",
121
+ );
122
+ setResult(null);
123
+ return;
124
+ }
125
+
126
+ // Other errors (network, 5xx) - retry next interval
127
  console.warn("Polling error (will retry):", err);
128
  }
129
  },
src/stroke_deepisles_demo/api/config.py ADDED
@@ -0,0 +1,10 @@
 
 
 
 
 
 
 
 
 
 
 
1
+ """API configuration constants.
2
+
3
+ Single source of truth for API configuration values.
4
+ """
5
+
6
+ from pathlib import Path
7
+
8
+ # Results directory for job outputs (must be in /tmp for HF Spaces)
9
+ # CRITICAL: This is the single source of truth. Import this instead of hardcoding.
10
+ RESULTS_DIR = Path("/tmp/stroke-results")
src/stroke_deepisles_demo/api/files.py ADDED
@@ -0,0 +1,82 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """File serving routes for NIfTI result files.
2
+
3
+ BUG-004 FIX: This module replaces the StaticFiles mount approach.
4
+
5
+ Previously, files were served via:
6
+ app.mount("/files", StaticFiles(directory=RESULTS_DIR))
7
+
8
+ The problem: StaticFiles is a mounted sub-application, and FastAPI/Starlette
9
+ middleware (including CORSMiddleware) does NOT propagate to mounted apps.
10
+ This caused NiiVue's cross-origin fetch to fail with "Failed to fetch".
11
+
12
+ Solution: Use explicit route handlers that go through the main app's middleware.
13
+ Now CORS headers are correctly applied to file responses.
14
+
15
+ Reference: https://github.com/fastapi/fastapi/discussions/7319
16
+ """
17
+
18
+ from fastapi import APIRouter, HTTPException
19
+ from fastapi.responses import FileResponse
20
+
21
+ from stroke_deepisles_demo.api.config import RESULTS_DIR
22
+ from stroke_deepisles_demo.core.logging import get_logger
23
+
24
+ logger = get_logger(__name__)
25
+
26
+ files_router = APIRouter(prefix="/files", tags=["files"])
27
+
28
+
29
+ @files_router.get("/{job_id}/{case_id}/{filename}")
30
+ async def get_result_file(job_id: str, case_id: str, filename: str) -> FileResponse:
31
+ """Serve NIfTI result files with proper CORS headers.
32
+
33
+ This route goes through the main FastAPI app's middleware stack,
34
+ ensuring CORS and CORP headers are applied to the response.
35
+
36
+ Args:
37
+ job_id: The job UUID from segmentation
38
+ case_id: The case identifier (e.g., sub-stroke0001)
39
+ filename: The NIfTI filename (e.g., dwi.nii.gz, prediction_fused.nii.gz)
40
+
41
+ Returns:
42
+ FileResponse with the NIfTI file
43
+
44
+ Raises:
45
+ 404: File not found (job expired, invalid path, or doesn't exist)
46
+ """
47
+ # Construct file path
48
+ file_path = RESULTS_DIR / job_id / case_id / filename
49
+
50
+ # Security: Ensure path doesn't escape RESULTS_DIR (path traversal protection)
51
+ try:
52
+ resolved = file_path.resolve()
53
+ if not str(resolved).startswith(str(RESULTS_DIR.resolve())):
54
+ logger.warning("Path traversal attempt blocked: %s", filename)
55
+ raise HTTPException(status_code=404, detail="File not found")
56
+ except (OSError, ValueError):
57
+ raise HTTPException(status_code=404, detail="Invalid file path") from None
58
+
59
+ # Check file exists
60
+ if not resolved.exists() or not resolved.is_file():
61
+ logger.debug("File not found: %s", resolved)
62
+ raise HTTPException(
63
+ status_code=404,
64
+ detail=f"File not found: {filename}. Job may have expired (1 hour TTL).",
65
+ )
66
+
67
+ # Determine media type based on extension
68
+ # NIfTI files are typically gzip-compressed
69
+ if filename.endswith(".nii.gz"):
70
+ media_type = "application/gzip"
71
+ elif filename.endswith(".nii"):
72
+ media_type = "application/octet-stream"
73
+ else:
74
+ media_type = "application/octet-stream"
75
+
76
+ logger.debug("Serving file: %s (type: %s)", resolved, media_type)
77
+
78
+ return FileResponse(
79
+ path=resolved,
80
+ media_type=media_type,
81
+ filename=filename,
82
+ )
src/stroke_deepisles_demo/api/job_store.py CHANGED
@@ -23,11 +23,14 @@ import threading
23
  from dataclasses import dataclass
24
  from datetime import datetime, timedelta
25
  from enum import Enum
26
- from pathlib import Path
27
- from typing import Any
28
 
 
29
  from stroke_deepisles_demo.core.logging import get_logger
30
 
 
 
 
31
  logger = get_logger(__name__)
32
 
33
  # Regex for safe job IDs (alphanumeric, hyphens, underscores only)
@@ -135,7 +138,7 @@ class JobStore:
135
  self._jobs: dict[str, Job] = {}
136
  self._lock = threading.RLock()
137
  self._ttl = ttl
138
- self._results_dir = results_dir or Path("/tmp/stroke-results")
139
  self._cleanup_thread: threading.Thread | None = None
140
  self._shutdown = threading.Event()
141
 
 
23
  from dataclasses import dataclass
24
  from datetime import datetime, timedelta
25
  from enum import Enum
26
+ from typing import TYPE_CHECKING, Any
 
27
 
28
+ from stroke_deepisles_demo.api.config import RESULTS_DIR
29
  from stroke_deepisles_demo.core.logging import get_logger
30
 
31
+ if TYPE_CHECKING:
32
+ from pathlib import Path
33
+
34
  logger = get_logger(__name__)
35
 
36
  # Regex for safe job IDs (alphanumeric, hyphens, underscores only)
 
138
  self._jobs: dict[str, Job] = {}
139
  self._lock = threading.RLock()
140
  self._ttl = ttl
141
+ self._results_dir = results_dir or RESULTS_DIR
142
  self._cleanup_thread: threading.Thread | None = None
143
  self._shutdown = threading.Event()
144
 
src/stroke_deepisles_demo/api/main.py CHANGED
@@ -16,23 +16,20 @@ Architecture designed to work within HuggingFace Spaces constraints:
16
  import os
17
  from collections.abc import AsyncIterator
18
  from contextlib import asynccontextmanager
19
- from pathlib import Path
20
  from typing import Any
21
 
22
  from fastapi import FastAPI, Request, Response
23
  from fastapi.middleware.cors import CORSMiddleware
24
- from fastapi.staticfiles import StaticFiles
25
  from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
26
 
 
 
27
  from stroke_deepisles_demo.api.job_store import init_job_store
28
  from stroke_deepisles_demo.api.routes import router
29
  from stroke_deepisles_demo.core.logging import get_logger
30
 
31
  logger = get_logger(__name__)
32
 
33
- # Results directory (must be in /tmp for HF Spaces)
34
- RESULTS_DIR = Path("/tmp/stroke-results")
35
-
36
 
37
  @asynccontextmanager
38
  async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
@@ -115,24 +112,25 @@ if FRONTEND_ORIGIN and FRONTEND_ORIGIN not in CORS_ORIGINS:
115
  # Add CORP middleware first (for COEP compatibility)
116
  app.add_middleware(CORPMiddleware)
117
 
118
- # Add CORS middleware with strict security settings
119
  # Note: Using allow_origins list for exact matching (no regex needed)
120
  # This eliminates regex security concerns while maintaining single source of truth
121
  app.add_middleware(
122
  CORSMiddleware,
123
  allow_origins=CORS_ORIGINS,
124
  allow_credentials=False, # Not needed - no cookies/auth
125
- allow_methods=["GET", "POST"], # Only methods we use
126
- allow_headers=["Content-Type"], # Only headers we need
 
127
  )
128
 
129
- # API routes
130
  app.include_router(router, prefix="/api")
131
 
132
- # Static files for NIfTI results
133
- # Note: Mount happens at import time; ensure directory exists here as well.
134
- RESULTS_DIR.mkdir(parents=True, exist_ok=True)
135
- app.mount("/files", StaticFiles(directory=str(RESULTS_DIR)), name="files")
136
 
137
 
138
  @app.get("/")
 
16
  import os
17
  from collections.abc import AsyncIterator
18
  from contextlib import asynccontextmanager
 
19
  from typing import Any
20
 
21
  from fastapi import FastAPI, Request, Response
22
  from fastapi.middleware.cors import CORSMiddleware
 
23
  from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
24
 
25
+ from stroke_deepisles_demo.api.config import RESULTS_DIR
26
+ from stroke_deepisles_demo.api.files import files_router
27
  from stroke_deepisles_demo.api.job_store import init_job_store
28
  from stroke_deepisles_demo.api.routes import router
29
  from stroke_deepisles_demo.core.logging import get_logger
30
 
31
  logger = get_logger(__name__)
32
 
 
 
 
33
 
34
  @asynccontextmanager
35
  async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
 
112
  # Add CORP middleware first (for COEP compatibility)
113
  app.add_middleware(CORPMiddleware)
114
 
115
+ # Add CORS middleware with settings for NiiVue binary file fetching
116
  # Note: Using allow_origins list for exact matching (no regex needed)
117
  # This eliminates regex security concerns while maintaining single source of truth
118
  app.add_middleware(
119
  CORSMiddleware,
120
  allow_origins=CORS_ORIGINS,
121
  allow_credentials=False, # Not needed - no cookies/auth
122
+ allow_methods=["GET", "POST", "HEAD"], # HEAD for preflight checks
123
+ allow_headers=["Content-Type", "Range"], # Range needed for partial content requests
124
+ expose_headers=["Content-Range", "Content-Length", "Accept-Ranges"], # NiiVue needs these
125
  )
126
 
127
+ # API routes (includes /api/* endpoints)
128
  app.include_router(router, prefix="/api")
129
 
130
+ # File routes (serves NIfTI results through main app's middleware for CORS)
131
+ # BUG-004 FIX: Previously used StaticFiles mount which bypassed CORS middleware.
132
+ # Now using explicit routes so CORS headers are applied to file responses.
133
+ app.include_router(files_router)
134
 
135
 
136
  @app.get("/")
src/stroke_deepisles_demo/api/routes.py CHANGED
@@ -12,10 +12,10 @@ from __future__ import annotations
12
 
13
  import os
14
  import uuid
15
- from pathlib import Path
16
 
17
  from fastapi import APIRouter, BackgroundTasks, HTTPException, Request
18
 
 
19
  from stroke_deepisles_demo.api.job_store import JobStatus, get_job_store
20
  from stroke_deepisles_demo.api.schemas import (
21
  CasesResponse,
@@ -33,9 +33,6 @@ logger = get_logger(__name__)
33
 
34
  router = APIRouter()
35
 
36
- # Base directory for results
37
- RESULTS_BASE = Path("/tmp/stroke-results")
38
-
39
 
40
  def get_backend_base_url(request: Request) -> str:
41
  """Get the backend's public URL for building absolute file URLs.
@@ -215,7 +212,7 @@ def run_segmentation_job(
215
  store.update_progress(job_id, 10, "Loading case data...")
216
 
217
  # Set up output directory
218
- output_dir = RESULTS_BASE / job_id
219
 
220
  store.update_progress(job_id, 20, "Staging files for DeepISLES...")
221
 
 
12
 
13
  import os
14
  import uuid
 
15
 
16
  from fastapi import APIRouter, BackgroundTasks, HTTPException, Request
17
 
18
+ from stroke_deepisles_demo.api.config import RESULTS_DIR
19
  from stroke_deepisles_demo.api.job_store import JobStatus, get_job_store
20
  from stroke_deepisles_demo.api.schemas import (
21
  CasesResponse,
 
33
 
34
  router = APIRouter()
35
 
 
 
 
36
 
37
  def get_backend_base_url(request: Request) -> str:
38
  """Get the backend's public URL for building absolute file URLs.
 
212
  store.update_progress(job_id, 10, "Loading case data...")
213
 
214
  # Set up output directory
215
+ output_dir = RESULTS_DIR / job_id
216
 
217
  store.update_progress(job_id, 20, "Staging files for DeepISLES...")
218