Handle camera permission errors gracefully#96
Conversation
|
@krishnendu07-code is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for your Pull Request! We're thrilled to have your contribution to FreshScan AI. Before we review, please make sure you have:
A maintainer will review your code as soon as possible! |
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Scan Foundation: Engine, Media Utilities, and Component State src/pages/ScannerPage.tsx |
Defines DisplayResult, deriveGrade, and labelColor; ONNX getEngine() now polls engineReady for concurrent callers; captureVideoBlob renders video to canvas and produces JPEG via toBlob; blobToImageElement waits for image load; adds typed component state and pre-warms engine; camera stream lifecycle supports cancellation, stops tracks, clears video.srcObject, and caps progress animation. |
Hybrid Inference Execution: Online and Offline Paths src/pages/ScannerPage.tsx |
runScan always creates a preview object URL, stops camera, shows progress, attempts api.scanOnline(blob) and maps cloud fields into DisplayResult (stores lastScanId), falls back to ONNX engine.predictSingle if no online result, derives freshness/grade from fusion scores, displays local result, and best-effort submits a thumbnail via api.submitScan. Specialized parsing for NOT_A_FISH errors implemented. |
UI Rendering, Freshness Visualization, and Grade Sharing src/pages/ScannerPage.tsx |
Idle hint hidden when cameraError present; scan-complete overlay colors label via labelColor(result.label) and shows confidence/grade; inference-mode badge classes updated and flash toggle gated to idle; processing caption depends on edge vs cloud mode; freshness numeric uses labelColor with threshold bar coloring; “GRADE-A REPORT” copies /report/{scanId} from sessionStorage.lastScanId only when freshness ≥ 85 and updates copied state. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
🐰 In frames of canvas, a fish is found,
Cloud and ONNX both gather round,
Engines pre-warmed, errors shown with care,
A grade to share when freshness is fair. 🎏✨
🚥 Pre-merge checks | ✅ 4 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | The PR includes substantial refactoring beyond the linked issue scope, including ONNX engine polling, media helpers, hybrid inference flow, and UI state standardization unrelated to camera permission handling. | Focus the PR on camera permission error handling from issue #90. Defer unrelated refactoring changes to separate PRs to maintain clear scope and reviewability. |
|
| Docstring Coverage | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'Handle camera permission errors gracefully' directly matches the main objective from the linked issue #90, which focuses on handling camera permission denial errors. |
| Linked Issues check | ✅ Passed | The PR implements both objectives from issue #90: catches NotAllowedError and NotFoundError from getUserMedia, and displays a friendly UI with camera access guidance inside the camera viewport. |
| Description check | ✅ Passed | The PR provides comprehensive documentation of changes, before/after screenshots, and clear mapping to linked objectives for camera permission error handling. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/ScannerPage.tsx (1)
58-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInfinite poll if initial engine load fails.
If
loadModels()throws,engineLoadingremainstrueandengineReadystaysfalse. SubsequentgetEngine()calls enter the polling branch (lines 60-70) and never resolve becauseengineReadynever becomestrue.🔧 Proposed fix: reset flags on error and propagate failure
async function getEngine(): Promise<FishFreshnessInference> { if (engineReady && engineInstance) return engineInstance; if (engineLoading) { - await new Promise<void>((resolve) => { + await new Promise<void>((resolve, reject) => { + let attempts = 0; const poll = setInterval(() => { if (engineReady) { clearInterval(poll); resolve(); + } else if (!engineLoading) { + // Loading finished but not ready → previous load failed + clearInterval(poll); + reject(new Error("ONNX engine failed to load")); + } else if (++attempts > 100) { + // Timeout after ~10s + clearInterval(poll); + reject(new Error("ONNX engine load timeout")); } }, 100); }); return engineInstance!; } engineLoading = true; - engineInstance = new FishFreshnessInference(); - await engineInstance.loadModels(); - engineReady = true; - engineLoading = false; + try { + engineInstance = new FishFreshnessInference(); + await engineInstance.loadModels(); + engineReady = true; + } finally { + engineLoading = false; + } return engineInstance; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ScannerPage.tsx` around lines 58 - 77, getEngine currently leaves engineLoading true if FishFreshnessInference.loadModels() throws, causing callers to hang in the polling branch; wrap the model load in a try/catch around the engineInstance.loadModels() call so that on error you reset engineLoading = false, engineReady = false, clear engineInstance (set to undefined/null) and rethrow the error to propagate failure; reference getEngine, engineLoading, engineReady, engineInstance, FishFreshnessInference and loadModels when making the change.
🧹 Nitpick comments (1)
src/pages/ScannerPage.tsx (1)
277-282: ⚡ Quick winUnnecessary type assertion obscures the actual type.
api.submitScanreturnsPromise<ScanResponse>, whereScanResponse.scanisScanResultwhich already hasscan_id: stringpersrc/lib/types.ts. The double cast throughunknownis unnecessary and could mask future type mismatches.♻️ Proposed simplification
- if ((saved?.scan as unknown as { scan_id?: string })?.scan_id) { - sessionStorage.setItem( - "lastScanId", - (saved.scan as unknown as { scan_id: string }).scan_id, - ); - } + if (saved?.scan?.scan_id) { + sessionStorage.setItem("lastScanId", saved.scan.scan_id); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ScannerPage.tsx` around lines 277 - 282, The code uses an unnecessary double type assertion on saved.scan which obscures types; replace the (saved?.scan as unknown as { scan_id?: string }) and (saved.scan as unknown as { scan_id: string }) casts with the proper ScanResponse/ScanResult types (or let TypeScript infer them) and access saved.scan.scan_id directly (e.g., ensure saved is typed as ScanResponse and use saved.scan.scan_id) so the sessionStorage.setItem call uses the real scan_id without the unknown cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pages/ScannerPage.tsx`:
- Around line 58-77: getEngine currently leaves engineLoading true if
FishFreshnessInference.loadModels() throws, causing callers to hang in the
polling branch; wrap the model load in a try/catch around the
engineInstance.loadModels() call so that on error you reset engineLoading =
false, engineReady = false, clear engineInstance (set to undefined/null) and
rethrow the error to propagate failure; reference getEngine, engineLoading,
engineReady, engineInstance, FishFreshnessInference and loadModels when making
the change.
---
Nitpick comments:
In `@src/pages/ScannerPage.tsx`:
- Around line 277-282: The code uses an unnecessary double type assertion on
saved.scan which obscures types; replace the (saved?.scan as unknown as {
scan_id?: string }) and (saved.scan as unknown as { scan_id: string }) casts
with the proper ScanResponse/ScanResult types (or let TypeScript infer them) and
access saved.scan.scan_id directly (e.g., ensure saved is typed as ScanResponse
and use saved.scan.scan_id) so the sessionStorage.setItem call uses the real
scan_id without the unknown cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 39e96ebf-9f37-43d7-8899-30842c8906fb
📒 Files selected for processing (1)
src/pages/ScannerPage.tsx
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
Fixes #90
Description
This PR improves the camera access experience in the scanner page by handling camera permission and device availability errors gracefully.
Changes Made
Result
Instead of seeing a blank camera screen, users now receive clear instructions explaining the issue and how to resolve it.
Before
After
Summary by CodeRabbit
New Features
Bug Fixes