fix: Add react ErrorBoundary Component to prevent app crashes from unhandled errors#958
Open
monu808 wants to merge 9 commits intoAOSSIE-Org:mainfrom
Open
fix: Add react ErrorBoundary Component to prevent app crashes from unhandled errors#958monu808 wants to merge 9 commits intoAOSSIE-Org:mainfrom
monu808 wants to merge 9 commits intoAOSSIE-Org:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis change implements an ErrorBoundary component to catch unhandled JavaScript errors and prevent app crashes with a blank screen. The component provides error catching, retry functionality, and a fallback UI. App.tsx is updated to wrap the entire application with this ErrorBoundary. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @frontend/src/components/ErrorBoundary/ErrorBoundary.tsx:
- Around line 34-41: The button labeled "Reload App" currently triggers a soft
reset in handleRetry (clearing hasError / error and isRetrying via setState)
which doesn't perform a full page reload; change handleRetry to perform a real
app reload by calling window.location.reload() (optionally after a short
setTimeout to show the spinner) instead of only resetting state so the button
label matches behavior and persistent child errors are cleared by a full
refresh.
🧹 Nitpick comments (1)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (1)
25-27: Consider removing redundantisRetrying: falsefrom error state.The
isRetrying: falseassignment ingetDerivedStateFromErroris redundant since the state will either be freshly initialized (already false) or transitioning from a retry attempt (where it will be overridden anyway).🔎 Simplified implementation
static getDerivedStateFromError(error: Error): Partial<ErrorBoundaryState> { - return { hasError: true, error, isRetrying: false }; + return { hasError: true, error }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/App.tsxfrontend/src/components/ErrorBoundary/ErrorBoundary.tsxfrontend/src/components/ErrorBoundary/index.ts
🔇 Additional comments (5)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (3)
1-13: LGTM! Clean imports and well-defined interfaces.The component imports are appropriate, and the TypeScript interfaces clearly define the props and state structure for the error boundary.
29-32: LGTM! Proper error logging implementation.The error logging to console is appropriate and aligns with the PR objective to log errors without displaying them in the UI.
43-73: LGTM! Well-structured fallback UI with proper theming.The render method provides a clean, user-friendly fallback UI with appropriate theme integration. The decision to log errors to console rather than displaying them in the UI follows the stated PR objective.
frontend/src/App.tsx (1)
6-6: LGTM! ErrorBoundary is correctly integrated.The ErrorBoundary is properly placed inside
ThemeProviderand wraps all main application content, ensuring comprehensive error protection while maintaining access to the theme context for the fallback UI.Also applies to: 23-37
frontend/src/components/ErrorBoundary/index.ts (1)
1-1: LGTM! Standard barrel export pattern.The barrel export provides a clean public API for importing the ErrorBoundary component.
monu808
force-pushed
the
fix/error-app-crash
branch
from
6bcd0e8 to
b6a57ed
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx`:
- Around line 37-39: In ErrorBoundary.componentDidCatch, in addition to
this.setState({ errorInfo }), log the caught error and errorInfo (or forward
them to the app logger) so errors are recorded; update the componentDidCatch
method in the ErrorBoundary class to call the logger (e.g., logger.error or
console.error) with a clear message and include both the error and errorInfo
objects to preserve stack/context for downstream reporting.
🧹 Nitpick comments (1)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (1)
77-91: Addtype="button"to the details toggle.Prevents unintended form submissions if this UI ever renders within a form context.
🔧 Proposed tweak
<button + type="button" onClick={this.toggleDetails} className="text-muted-foreground hover:text-foreground flex w-full items-center justify-center gap-1 text-sm transition-colors" >
monu808
commented
Jan 28, 2026
Contributor
Author
There was a problem hiding this comment.
I'm using class based component as react only allows class based components for creating error boundary components, its a limitation of react
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/hooks/useToggleFav.ts (1)
21-23:⚠️ Potential issue | 🟡 MinorUse
stringtype instead ofanyfor the public hook callback parameter.The mutation function (line 11) and Redux reducer both expect
stringIDs. Usinganycircumvents type checking and allows invalid types to reach the API and state updates.return { - toggleFavourite: (id: any) => toggleFavouriteMutation.mutate(id), + toggleFavourite: (id: string) => toggleFavouriteMutation.mutate(id), toggleFavouritePending: toggleFavouriteMutation.isPending, };As per coding guidelines: "Avoid 'any', use explicit types."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useToggleFav.ts` around lines 21 - 23, The public hook callback currently accepts id: any; change it to a strongly typed string by updating the useToggleFav hook's exported function signature from toggleFavourite: (id: any) => ... to toggleFavourite: (id: string) => toggleFavouriteMutation.mutate(id), and ensure the underlying toggleFavouriteMutation (and any related mutation function or Redux action payload) is typed to accept a string ID so type checking prevents non-string values from reaching the API or reducer.backend/app/database/images.py (2)
22-35:⚠️ Potential issue | 🔴 Critical
total=FalseonImageRecordmakes all fields optional, breaking type safety.Changing
ImageRecordtototal=Falsemeans all fields become optional, includingid,path,folder_id, andthumbnailPathwhich should be required. This can causeKeyErrorexceptions in downstream code that accesses these fields directly (e.g.,memory_clustering.pylines 351-358, 796-798 andmemories.pylines 430-436).🐛 Proposed fix using Required for core fields
-class ImageRecord(TypedDict, total=False): +from typing import Required # Python 3.11+ + +class ImageRecord(TypedDict, total=False): """Represents the full images table structure""" - id: ImageId - path: ImagePath - folder_id: FolderId - thumbnailPath: str + id: Required[ImageId] + path: Required[ImagePath] + folder_id: Required[FolderId] + thumbnailPath: Required[str] metadata: Union[Mapping[str, Any], str] isTagged: bool isFavourite: bool # New fields for Memories feature latitude: Optional[float] longitude: Optional[float] captured_at: Optional[datetime]For Python < 3.11, consider using two TypedDicts (one required, one optional) and combining them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/database/images.py` around lines 22 - 35, ImageRecord was declared with total=False which makes all fields optional and breaks callers that expect required keys (e.g., id, path, folder_id, thumbnailPath); change the type definition so core fields are required and only the new/optional fields remain optional—either by using typing_extensions.Required (or Required from typing if on 3.11+) for id/path/folder_id/thumbnailPath or by splitting into two TypedDicts (e.g., ImageRecordRequired with required keys and ImageRecordOptional with latitude/longitude/captured_at) and combine them into the final ImageRecord used throughout the codebase (update references to ImageRecord to the new combined type).
58-91:⚠️ Potential issue | 🟠 MajorMissing database migration for existing databases.
CREATE TABLE IF NOT EXISTSwill not add the new columns (latitude,longitude,captured_at) to existing databases. Deployments with pre-existing databases will fail when queries expect these columns. Consider adding anALTER TABLEmigration.🐛 Proposed fix - add column migration
def db_create_images_table() -> None: conn = _connect() cursor = conn.cursor() + # Add new columns if they don't exist (for existing databases) + try: + cursor.execute("ALTER TABLE images ADD COLUMN latitude REAL") + except sqlite3.OperationalError: + pass # Column already exists + try: + cursor.execute("ALTER TABLE images ADD COLUMN longitude REAL") + except sqlite3.OperationalError: + pass + try: + cursor.execute("ALTER TABLE images ADD COLUMN captured_at DATETIME") + except sqlite3.OperationalError: + pass + # Create new images table with merged fields including Memories feature columns cursor.execute( """ CREATE TABLE IF NOT EXISTS images (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/database/images.py` around lines 58 - 91, db_create_images_table currently only uses CREATE TABLE IF NOT EXISTS so existing DBs won't get the new columns; update db_create_images_table to detect missing columns (e.g., via PRAGMA table_info('images') or similar) and run ALTER TABLE statements to add latitude, longitude, and captured_at if they are absent, then ensure the corresponding indexes (ix_images_latitude, ix_images_longitude, ix_images_captured_at, ix_images_favourite_captured_at) are created afterwards; reference the function name db_create_images_table and the column names latitude, longitude, captured_at and index names so the migration logic targets the proper table/schema.
🧹 Nitpick comments (11)
frontend/src/features/memoriesSlice.ts (1)
53-54: Consider usingRootStatefor the selector.The inline type
{ memories: MemoriesState }works but could drift from the actual store structure. UsingRootStatefrom the store ensures consistency.Use RootState for type safety
+import type { RootState } from '@/app/store'; + -export const selectSelectedMemory = (state: { memories: MemoriesState }) => +export const selectSelectedMemory = (state: RootState) => state.memories.selectedMemory;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/memoriesSlice.ts` around lines 53 - 54, The selector selectSelectedMemory currently types its state parameter as { memories: MemoriesState } which can drift from the actual store; import and use the centralized RootState type (from your store module) instead of the inline type and update the selector signature to accept (state: RootState) => state.memories.selectedMemory so the selector stays type-safe and consistent with the application's store shape.frontend/src/components/Memories/FeaturedMemoryCard.tsx (1)
29-31: Unused proptoday.The
todayprop is destructured but never used in the component. Either remove it from the props interface or use it (e.g., display it in the UI).If unused, remove the prop
- ({ images, years, memoryId }) => { + ({ images, years, memoryId }) => {Update the interface:
interface FeaturedMemoryCardProps { images: MemoryImage[]; - today: string; years: number[]; memoryId: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/FeaturedMemoryCard.tsx` around lines 29 - 31, The component FeaturedMemoryCard is destructuring a today prop from FeaturedMemoryCardProps but never uses it; remove the unused today prop from the FeaturedMemoryCardProps type and from the destructuring in the FeaturedMemoryCard component, and update any call sites that pass today to stop supplying it (or, if you prefer to keep it, use the value in the UI). Ensure you modify the FeaturedMemoryCardProps definition and the ({ images, years, memoryId }) parameter list in FeaturedMemoryCard accordingly so the prop is no longer referenced.frontend/src/components/Memories/MemoriesPage.tsx (2)
133-133: Use typeduseAppDispatchhook.The file imports
useDispatchfrom react-redux, but the codebase now has a typeduseAppDispatchhook infrontend/src/store/hooks.ts. Using it ensures type-safe dispatch calls.Use useAppDispatch
-import { useDispatch } from 'react-redux'; +import { useAppDispatch } from '@/store/hooks'; ... - const dispatch = useDispatch(); + const dispatch = useAppDispatch();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoriesPage.tsx` at line 133, Replace the untyped useDispatch import/usage with the project's typed hook: import and use useAppDispatch from frontend/src/store/hooks.ts instead of react-redux's useDispatch, and change the local dispatch declaration (currently const dispatch = useDispatch()) to useAppDispatch() so dispatch calls are type-safe (look for the useDispatch symbol and update it to useAppDispatch).
163-212: Repetitive error handling pattern.Four nearly identical
useEffectblocks for error handling. Consider consolidating into a custom hook or single effect.Consolidate error handling
// Custom hook or helper const useQueryErrorDialog = ( query: { isError: boolean; errorMessage?: string }, title: string, fallbackMessage: string ) => { const dispatch = useAppDispatch(); useEffect(() => { if (query.isError) { dispatch(showInfoDialog({ title, message: query.errorMessage || fallbackMessage, variant: 'error', })); } }, [query.isError, query.errorMessage, dispatch, title, fallbackMessage]); }; // Usage: useQueryErrorDialog(allMemoriesQuery, 'Error Loading Memories', 'Failed to load memories'); useQueryErrorDialog(recentMemoriesQuery, 'Error Loading Recent Memories', 'Failed to load recent memories'); // etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoriesPage.tsx` around lines 163 - 212, Replace the four repetitive useEffect blocks with a small reusable hook: implement useQueryErrorDialog(query, title, fallbackMessage) that uses useAppDispatch and a single useEffect to dispatch showInfoDialog when query.isError, using query.errorMessage || fallbackMessage; then call useQueryErrorDialog(allMemoriesQuery, 'Error Loading Memories', 'Failed to load memories'), useQueryErrorDialog(recentMemoriesQuery, 'Error Loading Recent Memories', 'Failed to load recent memories'), useQueryErrorDialog(yearMemoriesQuery, 'Error Loading Year Memories', 'Failed to load year memories'), and useQueryErrorDialog(onThisDayQuery, 'Error Loading On This Day', 'Failed to load On This Day'); ensure the hook lists query.isError, query.errorMessage, dispatch, title and fallbackMessage in its dependency array.backend/app/routes/memories.py (1)
439-439: Calling private method_reverse_geocodebreaks encapsulation.Accessing
clustering._reverse_geocode()(note the underscore prefix) relies on an internal implementation detail ofMemoryClustering. If the class is refactored, this code will break. Consider making this method public or adding a public wrapper.♻️ Suggested approach
Either:
- Rename
_reverse_geocodetoreverse_geocodeinMemoryClusteringto make it public, or- Add a public method like
get_location_name(lat, lon)toMemoryClustering# In MemoryClustering class: def get_location_name(self, lat: float, lon: float) -> str: """Public method to get location name for coordinates.""" return self._reverse_geocode(lat, lon)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/routes/memories.py` at line 439, The call to the private method clustering._reverse_geocode breaches encapsulation; update MemoryClustering to expose a public API (either rename _reverse_geocode to reverse_geocode or add a public wrapper like get_location_name(self, lat, lon) that internally calls _reverse_geocode) and then replace the call in this file to use the new public method (e.g., clustering.reverse_geocode(center_lat, center_lon) or clustering.get_location_name(center_lat, center_lon)); ensure the MemoryClustering class signature and any imports are adjusted accordingly.frontend/src/components/Memories/MemoryDetail.tsx (3)
102-108: Consider extracting the hardcoded'on-this-day'to a constant.The string
'on-this-day'is used for comparison but isn't defined inroutes.tsalongsideMEMORY_DETAIL. For consistency and maintainability, consider adding a constant likeON_THIS_DAY_MEMORY_IDin the routes or constants file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoryDetail.tsx` around lines 102 - 108, Extract the hardcoded 'on-this-day' into a shared constant (e.g. ON_THIS_DAY_MEMORY_ID) in your routes or constants file and import it into MemoryDetail.tsx; replace all uses of the literal string in the comparison (memoryId === 'on-this-day'), in the constructed tempMemory.memory_id, and any other occurrences in this file with the new constant to keep identifiers consistent and maintainable (refer to variables memoryId, onThisDayImages, onThisDayMeta, and tempMemory in MemoryDetail.tsx to locate all replacements).
172-191: Use explicit type instead ofimg: any.The
img: anyannotation bypasses type checking. Use theMemoryImagetype imported from the API functions.♻️ Proposed fix
- const formattedImages = memory.images.map((img: any) => ({ + const formattedImages = memory.images.map((img: MemoryImage) => ({ id: img.id, path: img.path, thumbnailPath: img.thumbnailPath, folder_id: '', isTagged: false, isFavourite: img.isFavourite || false, tags: [], metadata: { name: img.path.split('/').pop() || '', date_created: img.captured_at, width: 0, height: 0, file_location: img.path, file_size: 0, item_type: 'image' as const, - latitude: img.latitude ?? undefined, - longitude: img.longitude ?? undefined, + latitude: img.latitude !== null ? img.latitude : undefined, + longitude: img.longitude !== null ? img.longitude : undefined, }, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoryDetail.tsx` around lines 172 - 191, The map callback for formattedImages uses img: any which disables type checking; change the parameter type to the imported MemoryImage (e.g., memory.images.map((img: MemoryImage) => ...)) and add the MemoryImage import from the API functions at the top of the file if it’s missing; update any derived properties (thumbnailPath, captured_at, latitude, longitude, isFavourite) to match the MemoryImage fields used in the mapping to ensure correct typing for formattedImages.
52-62: Avoidas anytype assertions; use explicit types.The extensive use of
as anybypasses TypeScript's type safety. Per coding guidelines, avoidanyand use explicit types. Consider defining proper response types for your query hooks or using type guards.♻️ Suggested approach
Define explicit types for the query response data:
interface MemoriesQueryData { data?: { memories?: Memory[]; }; } // Then use typed extraction: const allMemories = (allMemoriesQuery.data as MemoriesQueryData)?.data?.memories || [];Or better yet, type the query hooks properly at the source in
useMemories.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoryDetail.tsx` around lines 52 - 62, Replace the unsafe "as any" assertions by introducing explicit response interfaces for the memory queries and typing the hooks or their results; e.g., create types that include { data?: { memories?: Memory[] } } for allMemoriesQuery/recentMemoriesQuery/yearMemoriesQuery and { data?: { images?: Image[]; today?: string; years?: number[] } } for onThisDayQuery, then use (allMemoriesQuery.data as MemoriesResponse)?.data?.memories || [] (and analogous typed casts or, better, change the return type of the hook in useMemories.ts) and build onThisDayMeta from the strongly typed shape instead of asserting any.frontend/src/components/Memories/index.ts (1)
8-13: Consider adding exports forMemoryDetailandMemoryViewer.The barrel file exports
MemoriesPage,MemoryCard, andFeaturedMemoryCard, butMemoryDetailandMemoryViewer(which exist in this directory) are not included. If these components are intended to be consumed externally, consider adding them here for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/index.ts` around lines 8 - 13, The barrel currently exports MemoriesPage, MemoryCard, and FeaturedMemoryCard but omits MemoryDetail and MemoryViewer; add exports for these components by exporting their defaults from the same directory (e.g., add lines exporting MemoryDetail and MemoryViewer using their default exports) so external consumers can import MemoryDetail and MemoryViewer from the barrel alongside MemoriesPage, MemoryCard, and FeaturedMemoryCard.docs/backend/backend_python/openapi.json (1)
1360-1380: Memory endpoint 200 responses have empty schemas.The memories endpoints return
"schema": {}for successful responses instead of referencing the Pydantic models (GenerateMemoriesResponse,TimelineResponse, etc.). This is because the routes return plain dicts instead of typed response models, which prevents OpenAPI documentation from showing the response structure.To fix this, use the response models in the route decorators:
`@router.post`("/generate", response_model=GenerateMemoriesResponse) def generate_memories(...): ... return GenerateMemoriesResponse( success=True, message=..., memory_count=..., ... )Also applies to: 1434-1454, 1464-1474, 1514-1534
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/backend/backend_python/openapi.json` around lines 1360 - 1380, The OpenAPI 200 responses are empty because the memory routes return plain dicts instead of using typed response models; update the FastAPI route decorators to include response_model parameters (e.g., change `@router.post`("/generate") to `@router.post`("/generate", response_model=GenerateMemoriesResponse) and similarly add response_model=TimelineResponse or the appropriate Pydantic model for the other routes referenced), and change the returned values to return instances of those Pydantic models (e.g., return GenerateMemoriesResponse(...)) instead of raw dicts so the schemas (GenerateMemoriesResponse, TimelineResponse, etc.) are emitted in OpenAPI.frontend/src/components/Memories/MemoryViewer.tsx (1)
73-92: Duplicate image transformation logic withMemoryDetail.tsx.The
MemoryImage→Image[]transformation logic appears in bothMemoryViewer.tsx(lines 73-92 and 338-357) andMemoryDetail.tsx. Consider extracting this to a shared utility function to maintain consistency and reduce duplication.♻️ Suggested utility function
Create a shared utility in
@/utils/memories.ts:import { MemoryImage } from '@/api/api-functions/memories'; import { Image } from '@/types/Media'; export const convertMemoryImagesToImages = (memoryImages: MemoryImage[]): Image[] => { return memoryImages.map((img) => ({ id: img.id, path: img.path, thumbnailPath: img.thumbnailPath, folder_id: '', isTagged: false, isFavourite: img.isFavourite || false, tags: [], metadata: { name: img.path.split('/').pop() || '', date_created: img.captured_at, width: 0, height: 0, file_location: img.path, file_size: 0, item_type: 'image' as const, latitude: img.latitude ?? undefined, longitude: img.longitude ?? undefined, }, })); };Also applies to: 338-357
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Memories/MemoryViewer.tsx` around lines 73 - 92, Extract the duplicated mapping logic into a shared function convertMemoryImagesToImages(memoryImages: MemoryImage[]): Image[] in a new utility module, implement the same mapping (id, path, thumbnailPath, folder_id:'', isTagged:false, isFavourite defaulted, tags:[], and metadata with name from path, date_created from captured_at, width/height/file_size defaults, item_type:'image', and latitude/longitude using nullish coalescing), export it, and then replace the inlined map blocks in MemoryViewer (both occurrences) and MemoryDetail to import and call convertMemoryImagesToImages(memory.images) so both components use the single canonical transformation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/routes/memories.py`:
- Around line 175-183: The route handlers currently return a nested {"data":
{...}, "success": True, ...} shape which doesn't match the flat Pydantic
response models (e.g., GenerateMemoriesResponse, TimelineResponse); update the
handlers that build the response (the memory-related route functions in this
module that currently create the "data" key) to return the fields at the top
level (memory_count, image_count, memories, success, message, etc.) to match the
Pydantic models OR alternatively update the Pydantic models to include a
top-level "data" field and adjust all usages consistently; ensure you modify
every similar return site in this file (both the block returning
memory_count/image_count/memories and the similar block around the other
endpoint) so the returned dict structure matches the declared response model
exactly.
- Around line 430-436: The center calculation can raise KeyError or
ZeroDivisionError if some images lack "latitude"/"longitude" or all are missing;
change the computation in the cluster handling that sets center_lat and
center_lon to only use images that have valid coordinates (e.g., filter
cluster_images by img.get("latitude") and img.get("longitude")), compute sums
from those filtered lists, and guard the division so if the filtered list is
empty you provide a sensible fallback (skip the cluster or set center to
None/default). Update the code that assigns center_lat/center_lon in memories.py
(the block that currently iterates over cluster_images) to use .get() and an
empty-check before dividing.
In `@backend/app/utils/extract_location_metadata.py`:
- Around line 62-78: The code currently treats 0 values as missing by using
falsy checks like "if not lat or not lon" and "lat = lat or gps.get(...)", so
update checks to explicitly test for None and only fall back when a value is
None; e.g., replace occurrences of "if not lat or not lon" with "if lat is None
or lon is None" and use conditional assignments that only overwrite when the
target is None (e.g., "if lat is None: lat = ...", "if lon is None: lon = ...")
when handling direct fields, the exif->gps block, and the alternative field-name
block so that 0.0 is preserved. Reference symbols: lat, lon, metadata, exif,
gps.
In `@backend/app/utils/memory_clustering.py`:
- Around line 383-390: The date-based memory sentinel for location_name is
inconsistent: _create_simple_memory(...) sets location_name = "" for
memory_type="date" while _create_date_based_memory() uses "Date-Based Memory",
causing frontend heuristics to mis-render titles; update _create_simple_memory
to set location_name = "Date-Based Memory" whenever memory_type == "date" so
both creators use the same sentinel and the frontend title helper receives the
expected value.
- Around line 762-774: The clustering loop currently compares each image only to
the immediately previous image, allowing chain-linking past date_tolerance_days;
change the logic in the cluster-building loop (the block using sorted_images,
current_cluster, clusters, and date_tolerance_days) to compare the candidate
image's captured_at to the first image's captured_at in current_cluster (track a
cluster_start_date from current_cluster[0]) and only append to current_cluster
if abs((candidate_date - cluster_start_date).days) <= self.date_tolerance_days;
when starting a new cluster set cluster_start_date to the new cluster's first
image timestamp so subsequent comparisons use the cluster start instead of the
previous image.
In `@frontend/src/api/api-functions/memories.ts`:
- Around line 29-41: The Memory frontend interface is missing the backend `type`
field; update the `Memory` interface in
frontend/src/api/api-functions/memories.ts to include a `type` property matching
the backend literal (e.g., type: "location" | "date" or a string union
equivalent) so the frontend model (Memory) matches the backend `Memory` model
and consumers of Memory objects (including any uses of Memory.images or
thumbnail_image_id) receive the type information.
In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx`:
- Around line 21-124: Add automated component tests for ErrorBoundary covering:
(1) when a child throws, ErrorBoundary (via
getDerivedStateFromError/componentDidCatch) shows the fallback UI; (2) the
toggleDetails button (toggleDetails) reveals the error name/message and
componentStack; and (3) clicking the Reload App button (handleRetry) resets
state so children render again. Implement tests using react-testing-library +
jest: render ErrorBoundary around a test Child that throws during render, assert
fallback text and Show Details button appear, click Show Details and assert
error fields and stack are visible, then click Reload App and assert the
fallback is removed and the Child is mounted/re-rendered. Ensure to use act
where needed and to simulate the thrown error consistently so the recovery path
is exercised.
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 168-173: The "Open Original File" button in MediaInfoPanel is
rendered as an active control but its onClick is a no-op; either restore the
real opener or make it clearly disabled/hidden. Locate the button in
MediaInfoPanel.tsx (the element with the "Open Original File" label) and either:
1) rewire its onClick to call the existing opener function (e.g.,
openOriginalFile or handleOpenOriginal if present) and pass required args so it
actually opens the file, or 2) remove the no-op handler and render it as a
disabled control (add disabled attribute, aria-disabled="true", appropriate
disabled styling and a title/aria-label explaining why) or hide it until the
action is supported. Ensure keyboard and screen-reader users get proper feedback
(no focusable dead CTA).
In `@frontend/src/components/Media/MediaView.tsx`:
- Around line 115-133: Make handleToggleFavourite await the toggle operation and
handle errors: change handleToggleFavourite to perform an awaited call to
onToggleFavorite(currentImage.id) if provided (or await
toggleFavourite(currentImage.id) otherwise), catch and log any rejection to
avoid unhandled promise rejections, and only call handleClose() after the
awaited toggle completes successfully (or still call it after logging on failure
if desired); ensure references to currentImage, onToggleFavorite,
toggleFavourite, handleClose, ROUTES.FAVOURITES and location.pathname are used
to locate and update the logic and update the useCallback dependencies
accordingly.
In `@frontend/src/components/Media/ZoomableImage.tsx`:
- Around line 203-243: The wheel handler is passing imageRect.width/height
(which already include CSS rotation) into getEffectiveDimensions, causing
rotated images to have their width/height swapped twice; change the base
dimensions calculation to use the image's intrinsic (unrotated) dimensions from
imageRef.current.naturalWidth and naturalHeight (divided by currentScale) when
computing baseW and baseH, then pass those baseW/baseH into
getEffectiveDimensions; update the same logic in the other zoom branch that
currently repeats this calculation so both places use intrinsic dims rather than
imageRect-derived values (references: imageRef, imageRect,
getEffectiveDimensions, baseW, baseH, transformState.scale).
- Around line 355-402: The panning clamp currently treats both axes the same,
allowing the non-overflowing axis to drift; in the onPanning handler (use
variables ref, positionX/positionY, viewW/viewH, scaledW/scaledH, PAN_PADDING
and ref.setTransform) detect which axis is overflowing using the earlier
overflow = getOverflowState(...) and pin the non-overflowing axis to the
centered position instead of using the wide min/max limits: compute centeredX =
(viewW - scaledW) / 2 and centeredY = (viewH - scaledH) / 2 and when
overflow.width is false force finalX = centeredX, and when overflow.height is
false force finalY = centeredY before calling ref.setTransform(finalX, finalY,
scale, 0); ensure setIsOverflowing remains unchanged.
In `@frontend/src/components/Memories/MemoriesPage.tsx`:
- Around line 142-152: Replace the unsafe "as any" casts by typing the query
responses and using those types when accessing data: define interfaces (e.g.,
MemoriesResponse with memories: Memory[] and OnThisDayResponse with images:
MemoryImage[], today: string, years: number[]) or update the query hook generics
(allMemoriesQuery, recentMemoriesQuery, yearMemoriesQuery, onThisDayQuery) to
return these types, then access values via optional chaining/nullish coalescing
(e.g., allMemoriesQuery.data?.data?.memories ?? []) and construct onThisDayMeta
from the typed onThisDayQuery.data.data fields instead of casting to any; update
variables allMemories, recentMemories, yearMemories, onThisDayImages, and
onThisDayMeta accordingly.
In `@frontend/src/components/Memories/MemoryViewer.tsx`:
- Around line 89-90: Replace the use of logical OR with nullish coalescing for
GPS coords: wherever img.latitude || undefined and img.longitude || undefined
are used (in MemoryViewer.tsx, e.g., the mapping that builds the image/location
object), change to img.latitude ?? undefined and img.longitude ?? undefined (and
apply the same change at the other occurrence around lines 354-355) so numeric 0
coordinates are preserved instead of being treated as falsy.
In `@frontend/src/hooks/useMemories.tsx`:
- Around line 11-17: The hook useAllMemories currently calls usePictoQuery with
queryFn: () => generateMemories(), but generateMemories performs a POST and
should be invoked via a mutation; change useAllMemories to use usePictoMutation
instead of usePictoQuery, wiring generateMemories as the mutation function and
exposing mutate/mutateAsync (and any loading/error states) rather than query
results so the POST is only run on explicit calls and not on refetch or focus
events; update any callers of useAllMemories to call the returned
mutate/mutateAsync from usePictoMutation.
In `@frontend/src/utils/memories.ts`:
- Around line 156-163: calculateYearsAgo currently only subtracts years and
doesn't validate dates; update the function calculateYearsAgo(isoDate: string)
to (1) parse the date and explicitly check validity using isNaN(date.getTime())
and return 0 for invalid inputs, and (2) compute the year difference then
decrement it by 1 when the current month/day is before the date's month/day so
you don't overcount before the anniversary; keep error handling simple (no
try/catch needed) and reference calculateYearsAgo to locate and fix the logic.
- Around line 101-120: The calculation for daysDiff in
frontend/src/utils/memories.ts uses raw timestamps (now and end) which
misclassifies near-midnight and DST-shifted dates; normalize both dates to local
midnight (e.g., create midnight-only Date objects or call setHours(0,0,0,0) on
copies of now and end) before computing daysDiff, then compute daysDiff =
Math.floor((nowMidnight.getTime() - endMidnight.getTime()) / (1000*60*60*24));
update the references to now and daysDiff in the "Today"/"Yesterday"/"This week"
checks so they use the normalized midnight values.
---
Outside diff comments:
In `@backend/app/database/images.py`:
- Around line 22-35: ImageRecord was declared with total=False which makes all
fields optional and breaks callers that expect required keys (e.g., id, path,
folder_id, thumbnailPath); change the type definition so core fields are
required and only the new/optional fields remain optional—either by using
typing_extensions.Required (or Required from typing if on 3.11+) for
id/path/folder_id/thumbnailPath or by splitting into two TypedDicts (e.g.,
ImageRecordRequired with required keys and ImageRecordOptional with
latitude/longitude/captured_at) and combine them into the final ImageRecord used
throughout the codebase (update references to ImageRecord to the new combined
type).
- Around line 58-91: db_create_images_table currently only uses CREATE TABLE IF
NOT EXISTS so existing DBs won't get the new columns; update
db_create_images_table to detect missing columns (e.g., via PRAGMA
table_info('images') or similar) and run ALTER TABLE statements to add latitude,
longitude, and captured_at if they are absent, then ensure the corresponding
indexes (ix_images_latitude, ix_images_longitude, ix_images_captured_at,
ix_images_favourite_captured_at) are created afterwards; reference the function
name db_create_images_table and the column names latitude, longitude,
captured_at and index names so the migration logic targets the proper
table/schema.
In `@frontend/src/hooks/useToggleFav.ts`:
- Around line 21-23: The public hook callback currently accepts id: any; change
it to a strongly typed string by updating the useToggleFav hook's exported
function signature from toggleFavourite: (id: any) => ... to toggleFavourite:
(id: string) => toggleFavouriteMutation.mutate(id), and ensure the underlying
toggleFavouriteMutation (and any related mutation function or Redux action
payload) is typed to accept a string ID so type checking prevents non-string
values from reaching the API or reducer.
---
Nitpick comments:
In `@backend/app/routes/memories.py`:
- Line 439: The call to the private method clustering._reverse_geocode breaches
encapsulation; update MemoryClustering to expose a public API (either rename
_reverse_geocode to reverse_geocode or add a public wrapper like
get_location_name(self, lat, lon) that internally calls _reverse_geocode) and
then replace the call in this file to use the new public method (e.g.,
clustering.reverse_geocode(center_lat, center_lon) or
clustering.get_location_name(center_lat, center_lon)); ensure the
MemoryClustering class signature and any imports are adjusted accordingly.
In `@docs/backend/backend_python/openapi.json`:
- Around line 1360-1380: The OpenAPI 200 responses are empty because the memory
routes return plain dicts instead of using typed response models; update the
FastAPI route decorators to include response_model parameters (e.g., change
`@router.post`("/generate") to `@router.post`("/generate",
response_model=GenerateMemoriesResponse) and similarly add
response_model=TimelineResponse or the appropriate Pydantic model for the other
routes referenced), and change the returned values to return instances of those
Pydantic models (e.g., return GenerateMemoriesResponse(...)) instead of raw
dicts so the schemas (GenerateMemoriesResponse, TimelineResponse, etc.) are
emitted in OpenAPI.
In `@frontend/src/components/Memories/FeaturedMemoryCard.tsx`:
- Around line 29-31: The component FeaturedMemoryCard is destructuring a today
prop from FeaturedMemoryCardProps but never uses it; remove the unused today
prop from the FeaturedMemoryCardProps type and from the destructuring in the
FeaturedMemoryCard component, and update any call sites that pass today to stop
supplying it (or, if you prefer to keep it, use the value in the UI). Ensure you
modify the FeaturedMemoryCardProps definition and the ({ images, years, memoryId
}) parameter list in FeaturedMemoryCard accordingly so the prop is no longer
referenced.
In `@frontend/src/components/Memories/index.ts`:
- Around line 8-13: The barrel currently exports MemoriesPage, MemoryCard, and
FeaturedMemoryCard but omits MemoryDetail and MemoryViewer; add exports for
these components by exporting their defaults from the same directory (e.g., add
lines exporting MemoryDetail and MemoryViewer using their default exports) so
external consumers can import MemoryDetail and MemoryViewer from the barrel
alongside MemoriesPage, MemoryCard, and FeaturedMemoryCard.
In `@frontend/src/components/Memories/MemoriesPage.tsx`:
- Line 133: Replace the untyped useDispatch import/usage with the project's
typed hook: import and use useAppDispatch from frontend/src/store/hooks.ts
instead of react-redux's useDispatch, and change the local dispatch declaration
(currently const dispatch = useDispatch()) to useAppDispatch() so dispatch calls
are type-safe (look for the useDispatch symbol and update it to useAppDispatch).
- Around line 163-212: Replace the four repetitive useEffect blocks with a small
reusable hook: implement useQueryErrorDialog(query, title, fallbackMessage) that
uses useAppDispatch and a single useEffect to dispatch showInfoDialog when
query.isError, using query.errorMessage || fallbackMessage; then call
useQueryErrorDialog(allMemoriesQuery, 'Error Loading Memories', 'Failed to load
memories'), useQueryErrorDialog(recentMemoriesQuery, 'Error Loading Recent
Memories', 'Failed to load recent memories'),
useQueryErrorDialog(yearMemoriesQuery, 'Error Loading Year Memories', 'Failed to
load year memories'), and useQueryErrorDialog(onThisDayQuery, 'Error Loading On
This Day', 'Failed to load On This Day'); ensure the hook lists query.isError,
query.errorMessage, dispatch, title and fallbackMessage in its dependency array.
In `@frontend/src/components/Memories/MemoryDetail.tsx`:
- Around line 102-108: Extract the hardcoded 'on-this-day' into a shared
constant (e.g. ON_THIS_DAY_MEMORY_ID) in your routes or constants file and
import it into MemoryDetail.tsx; replace all uses of the literal string in the
comparison (memoryId === 'on-this-day'), in the constructed
tempMemory.memory_id, and any other occurrences in this file with the new
constant to keep identifiers consistent and maintainable (refer to variables
memoryId, onThisDayImages, onThisDayMeta, and tempMemory in MemoryDetail.tsx to
locate all replacements).
- Around line 172-191: The map callback for formattedImages uses img: any which
disables type checking; change the parameter type to the imported MemoryImage
(e.g., memory.images.map((img: MemoryImage) => ...)) and add the MemoryImage
import from the API functions at the top of the file if it’s missing; update any
derived properties (thumbnailPath, captured_at, latitude, longitude,
isFavourite) to match the MemoryImage fields used in the mapping to ensure
correct typing for formattedImages.
- Around line 52-62: Replace the unsafe "as any" assertions by introducing
explicit response interfaces for the memory queries and typing the hooks or
their results; e.g., create types that include { data?: { memories?: Memory[] }
} for allMemoriesQuery/recentMemoriesQuery/yearMemoriesQuery and { data?: {
images?: Image[]; today?: string; years?: number[] } } for onThisDayQuery, then
use (allMemoriesQuery.data as MemoriesResponse)?.data?.memories || [] (and
analogous typed casts or, better, change the return type of the hook in
useMemories.ts) and build onThisDayMeta from the strongly typed shape instead of
asserting any.
In `@frontend/src/components/Memories/MemoryViewer.tsx`:
- Around line 73-92: Extract the duplicated mapping logic into a shared function
convertMemoryImagesToImages(memoryImages: MemoryImage[]): Image[] in a new
utility module, implement the same mapping (id, path, thumbnailPath,
folder_id:'', isTagged:false, isFavourite defaulted, tags:[], and metadata with
name from path, date_created from captured_at, width/height/file_size defaults,
item_type:'image', and latitude/longitude using nullish coalescing), export it,
and then replace the inlined map blocks in MemoryViewer (both occurrences) and
MemoryDetail to import and call convertMemoryImagesToImages(memory.images) so
both components use the single canonical transformation.
In `@frontend/src/features/memoriesSlice.ts`:
- Around line 53-54: The selector selectSelectedMemory currently types its state
parameter as { memories: MemoriesState } which can drift from the actual store;
import and use the centralized RootState type (from your store module) instead
of the inline type and update the selector signature to accept (state:
RootState) => state.memories.selectedMemory so the selector stays type-safe and
consistent with the application's store shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97689471-7fe9-47c8-be90-82038f0e75a5
⛔ Files ignored due to path filters (2)
__pycache__/app.cpython-313.pycis excluded by!**/*.pyc,!**/__pycache__/**utils/__pycache__/cache.cpython-313.pycis excluded by!**/*.pyc,!**/__pycache__/**
📒 Files selected for processing (46)
.coderabbit.yaml.env.exampleapp.pybackend/app/database/images.pybackend/app/routes/face_clusters.pybackend/app/routes/memories.pybackend/app/routes/user_preferences.pybackend/app/utils/extract_location_metadata.pybackend/app/utils/images.pybackend/app/utils/memory_clustering.pybackend/app/utils/memory_monitor.pybackend/main.pybackend/requirements.txtbackend/tests/test_face_clusters.pybackend/tests/test_folders.pybackend/tests/test_user_preferences.pydocs/backend/backend_python/openapi.jsondocs/frontend/memories.mddocs/overview/features.mdfrontend/src/api/api-functions/index.tsfrontend/src/api/api-functions/memories.tsfrontend/src/api/apiEndpoints.tsfrontend/src/app/store.tsfrontend/src/components/ErrorBoundary/ErrorBoundary.tsxfrontend/src/components/Media/ImageViewer.tsxfrontend/src/components/Media/MediaInfoPanel.tsxfrontend/src/components/Media/MediaView.tsxfrontend/src/components/Media/ZoomableImage.tsxfrontend/src/components/Memories/FeaturedMemoryCard.tsxfrontend/src/components/Memories/MemoriesPage.tsxfrontend/src/components/Memories/MemoryCard.tsxfrontend/src/components/Memories/MemoryDetail.tsxfrontend/src/components/Memories/MemoryViewer.tsxfrontend/src/components/Memories/index.tsfrontend/src/constants/routes.tsfrontend/src/features/imageSlice.tsfrontend/src/features/memoriesSlice.tsfrontend/src/hooks/useMemories.tsxfrontend/src/hooks/useToggleFav.tsfrontend/src/routes/AppRoutes.tsxfrontend/src/store/hooks.tsfrontend/src/types/Media.tsfrontend/src/utils/memories.tsscripts/linux-dev.shscripts/win-dev.batutils/cache.py
💤 Files with no reviewable changes (10)
- .env.example
- utils/cache.py
- scripts/linux-dev.sh
- scripts/win-dev.bat
- backend/tests/test_user_preferences.py
- backend/app/routes/face_clusters.py
- backend/tests/test_folders.py
- backend/tests/test_face_clusters.py
- backend/app/routes/user_preferences.py
- app.py
✅ Files skipped from review due to trivial changes (2)
- docs/overview/features.md
- backend/requirements.txt
- Added a fallback UI with animated button to reload in case error vanishes. - Instead of loggin error on UI logged errors to console.
…n a crash happens
- Imported ErrorBoundary Component. - Wrapped children inside themeprovider for same theme of fallback UI. - catches rendering error in any child component.
Log error details to the console when an error is caught.
monu808
force-pushed
the
fix/error-app-crash
branch
from
cdca876 to
80dfab7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (1)
21-124:⚠️ Potential issue | 🟠 MajorAdd automated coverage before merging.
This component is now the app's main crash-recovery path, but the PR still has no automated coverage for the flows most likely to regress: rendering the fallback when a child throws, toggling details, and recovering on retry. As per coding guidelines, "Ensure that test code is automated, comprehensive, and follows testing best practices" and "Verify that all critical functionality is covered by tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx` around lines 21 - 124, Add automated tests for ErrorBoundary covering the three critical flows: when a child throws (exercise getDerivedStateFromError/componentDidCatch and verify the fallback UI renders with "Something went wrong"), toggling details (call toggleDetails via clicking the "Show Details" button and assert error name/message and componentStack appear), and recovery on retry (click the "Reload App" button to exercise handleRetry, assert isRetrying state UI like "Reloading..." and spinner, then advance timers to ensure the boundary resets and children are rendered again). Use React Testing Library + Jest, mount ErrorBoundary with a test component that throws during render, spy/mock console.error to silence logs, and use jest.useFakeTimers() to advance the 500ms retry timeout; reference ErrorBoundary, handleRetry, toggleDetails, getDerivedStateFromError, and componentDidCatch to locate hooks to test.
🧹 Nitpick comments (1)
frontend/src/components/ErrorBoundary/ErrorBoundary.tsx (1)
79-106: Expose the details toggle state to assistive tech.The disclosure button changes content visibility, but it doesn't set
aria-expandedoraria-controls, so screen readers can't tell whether the error details are open. Wire those attributes toshowDetailsand give the details panel a stable id.♿ Suggested tweak
<button type="button" onClick={this.toggleDetails} + aria-expanded={this.state.showDetails} + aria-controls="error-boundary-details" className="text-muted-foreground hover:text-foreground flex w-full items-center justify-center gap-1 text-sm transition-colors" > @@ {this.state.showDetails && ( - <div className="bg-muted mt-3 max-h-48 overflow-auto rounded-md p-3 text-left"> + <div + id="error-boundary-details" + className="bg-muted mt-3 max-h-48 overflow-auto rounded-md p-3 text-left" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx` around lines 79 - 106, The disclosure button that calls toggleDetails should expose its state to assistive tech by adding aria-expanded={this.state.showDetails} and aria-controls referencing a stable id for the details panel; give the details <div> a stable id (e.g. this.detailsPanelId or a hardcoded "error-details-panel" if single-instance) and ensure the button's aria-controls matches that id and updates when this.state.showDetails changes, leaving the existing toggleDetails, showDetails state, and error/details rendering logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/App.tsx`:
- Around line 23-37: Wrap the app tree that uses React Query with
QueryErrorResetBoundary in App.tsx (wrap QueryClientProviders / BrowserRouter /
AppRoutes) and pass the reset callback into ErrorBoundary via a new onRetry
prop; update ErrorBoundary.tsx to accept onRetry?: () => void and call
this.props.onRetry?.() inside handleRetry after clearing its own state so React
Query's error state is reset when the ErrorBoundary retry path is invoked.
---
Duplicate comments:
In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx`:
- Around line 21-124: Add automated tests for ErrorBoundary covering the three
critical flows: when a child throws (exercise
getDerivedStateFromError/componentDidCatch and verify the fallback UI renders
with "Something went wrong"), toggling details (call toggleDetails via clicking
the "Show Details" button and assert error name/message and componentStack
appear), and recovery on retry (click the "Reload App" button to exercise
handleRetry, assert isRetrying state UI like "Reloading..." and spinner, then
advance timers to ensure the boundary resets and children are rendered again).
Use React Testing Library + Jest, mount ErrorBoundary with a test component that
throws during render, spy/mock console.error to silence logs, and use
jest.useFakeTimers() to advance the 500ms retry timeout; reference
ErrorBoundary, handleRetry, toggleDetails, getDerivedStateFromError, and
componentDidCatch to locate hooks to test.
---
Nitpick comments:
In `@frontend/src/components/ErrorBoundary/ErrorBoundary.tsx`:
- Around line 79-106: The disclosure button that calls toggleDetails should
expose its state to assistive tech by adding
aria-expanded={this.state.showDetails} and aria-controls referencing a stable id
for the details panel; give the details <div> a stable id (e.g.
this.detailsPanelId or a hardcoded "error-details-panel" if single-instance) and
ensure the button's aria-controls matches that id and updates when
this.state.showDetails changes, leaving the existing toggleDetails, showDetails
state, and error/details rendering logic intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 51a00065-01a0-4d1a-a118-a106d13674bc
📒 Files selected for processing (3)
frontend/src/App.tsxfrontend/src/components/ErrorBoundary/ErrorBoundary.tsxfrontend/src/components/ErrorBoundary/index.ts
…proved error handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi @rahulharpal1603 here is the brief summary and related screenshots and videosfor this pr and issue.
Summary:
Added a React ErrorBoundary component to catch unhandled JavaScript errors during rendering, preventing the app from crashing to a blank white/black screen depends on the theme.
Fixes: #937
Changes Made:
Files Modified/Created:
Screenshots:
Before:
After:
Demo:
Recording.2026-01-06.010312.1.mp4
How to Test:
npm run tauri dev// Add this inside the component, before the return statement
throw new Error('Test ErrorBoundary');
Summary by CodeRabbit