Improvement: Improved 404 page error for /browse #837
Conversation
WalkthroughIntroduces a new FileNotFound component for rendering 404 pages when files are missing in the browse feature. Integrates it into codePreviewPanel to display this component when file source requests return NOT_FOUND status. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
imsarang
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/web/src/app/`[domain]/browse/[...path]/components/404NotFound.tsx:
- Around line 101-107: The "Return to the repository overview" Link in
404NotFound builds its href via getBrowsePath but omits the optional
revisionName, causing the link to drop branch context; update the Link's
getBrowsePath call to pass the component's revisionName value (alongside
repoName/repoInfoResponse.name, path '/', pathType 'tree' and
SINGLE_TENANT_ORG_DOMAIN) so the generated URL preserves the current branch;
modify the Link invocation where getBrowsePath is called (inside the 404NotFound
component) to include revisionName as the appropriate argument.
- Around line 1-22: Remove the client-side rendering directive and the unused
hook: delete the "'use client';" line at the top of 404NotFound.tsx and remove
the unused import "useEffect" so the component becomes a server component; keep
the RepoInfoResponse type (with indexedAt: Date | undefined) as-is since it will
now stay on the server, and ensure no client-only hooks or "use client"
directives are reintroduced in this file or in any components that receive
repoInfoResponse (refer to RepoInfoResponse and PathHeader to locate usages).
| 'use client'; | ||
|
|
||
| import { $Enums } from "@sourcebot/db"; | ||
| import { ServiceError } from "@/lib/serviceError"; | ||
| import { TriangleAlert } from "lucide-react"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import Link from "next/link"; | ||
| import { getBrowsePath } from "@/app/[domain]/browse/hooks/utils"; | ||
| import { SINGLE_TENANT_ORG_DOMAIN } from "@/lib/constants"; | ||
| import { useEffect } from "react"; | ||
| import { FileSourceResponse } from "@/features/git/getFileSourceApi"; | ||
| import { PathHeader } from "@/app/[domain]/components/pathHeader"; | ||
|
|
||
| type RepoInfoResponse = ServiceError | { | ||
| id: number; | ||
| name: string; | ||
| displayName: string | undefined; | ||
| codeHostType: $Enums.CodeHostType; | ||
| externalWebUrl: string | undefined; | ||
| imageUrl: string | undefined; | ||
| indexedAt: Date | undefined; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "404NotFound.tsx" -type f | head -20Repository: sourcebot-dev/sourcebot
Length of output: 142
🌐 Web query:
Next.js client component props serialization Date objects RSC limitations
💡 Result:
In Next.js App Router (React Server Components), anything you pass from a Server Component to a Client Component via props must be serializable by the React Server Components (Flight) protocol. Next.js summarizes this as “props need to be serializable by React.” [1]
What this means for Date
- In many Next.js versions you’ll see warnings like “Date objects are not supported” when a
Datecrosses the Server→Client boundary. [2] - Practically, you should assume you won’t reliably get a
Dateinstance on the client. The common, safe approach is:- pass a primitive (e.g.
date.toISOString()ordate.getTime()), then - reconstruct in the client:
new Date(isoStringOrMs).
- pass a primitive (e.g.
Why the docs feel inconsistent
- React’s underlying RSC serialization supports more than JSON, and the Next.js team has discussed/documented that types like
Date,Map,Set, and evenPromiseare part of React’s serializable set—so Next’s older/stricter messaging has been a source of confusion. [3]
Rule of thumb (works across versions)
- Pass primitives and plain objects across the boundary.
- Convert “richer” values (like
Date) explicitly to a string/number at the boundary, or use a dedicated serializer (e.g. SuperJSON) if you control both sides and want automatic revival. [2]
Sources: Next.js docs on Server→Client prop serialization [1], Next.js discussion showing the Date warning and why it happens [2], Next.js issue noting React Flight’s broader serializable types (incl. Date) [3].
[1] (nextjs.org)
[2] (github.com)
[3] (github.com)
Citations:
- 1: https://nextjs.org/docs/app/getting-started/server-and-client-components?utm_source=openai
- 2: Warning: Only plain objects can be passed to Client Components from Server Components. Date objects are not supported vercel/next.js#46137
- 3: Docs: Serializable values that can be passed from the Server to Client Components vercel/next.js#54291
🏁 Script executed:
# Check if the file exists and read its content
find . -path "*/packages/web/src/app/**/404NotFound.tsx" -type f | head -1 | xargs wc -l 2>/dev/null || echo "File not found with exact path, searching differently..."
fd "404NotFound.tsx" --type fRepository: sourcebot-dev/sourcebot
Length of output: 220
🏁 Script executed:
cat -n ./packages/web/src/app/[domain]/browse/[...path]/components/404NotFound.tsxRepository: sourcebot-dev/sourcebot
Length of output: 5058
🏁 Script executed:
# Find where FileNotFound component is imported/used
rg "FileNotFound" --type tsx --type ts -A 3 -B 3 | head -100Repository: sourcebot-dev/sourcebot
Length of output: 94
🏁 Script executed:
# Also check the specific page that likely calls this component
fd "page.tsx" -path "*/[domain]/browse/[...path]/*" | head -5 | xargs cat -n 2>/dev/null | head -200Repository: sourcebot-dev/sourcebot
Length of output: 235
🏁 Script executed:
# Find where FileNotFound component is imported/used
rg "FileNotFound" -g "*.tsx" -g "*.ts" -A 3 -B 3 | head -150Repository: sourcebot-dev/sourcebot
Length of output: 2423
🏁 Script executed:
# Find the page.tsx in the browse directory
fd "page.tsx" | grep browse | grep pathRepository: sourcebot-dev/sourcebot
Length of output: 122
🏁 Script executed:
# Check codePreviewPanel to see if it's a client or server component
head -50 packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsxRepository: sourcebot-dev/sourcebot
Length of output: 1837
🏁 Script executed:
# Check the page.tsx to see the full context
head -150 packages/web/src/app/[domain]/browse/[...path]/page.tsxRepository: sourcebot-dev/sourcebot
Length of output: 3393
Remove 'use client' and unused useEffect import—this component doesn't need client-side rendering.
FileNotFound passes repoInfoResponse (which includes indexedAt: Date) from a server component to a client component, causing Next.js RSC serialization issues with the Date object. The component doesn't use any client-only hooks (useEffect is imported but never called), so it can be a server component.
🔧 Proposed fix
-'use client';
-
-import { useEffect } from "react";🤖 Prompt for AI Agents
In `@packages/web/src/app/`[domain]/browse/[...path]/components/404NotFound.tsx
around lines 1 - 22, Remove the client-side rendering directive and the unused
hook: delete the "'use client';" line at the top of 404NotFound.tsx and remove
the unused import "useEffect" so the component becomes a server component; keep
the RepoInfoResponse type (with indexedAt: Date | undefined) as-is since it will
now stay on the server, and ensure no client-only hooks or "use client"
directives are reintroduced in this file or in any components that receive
repoInfoResponse (refer to RepoInfoResponse and PathHeader to locate usages).
| <Link | ||
| href={getBrowsePath({ | ||
| repoName: !('statusCode' in repoInfoResponse) ? repoInfoResponse.name : repoName, | ||
| path: '/', | ||
| pathType: 'tree', | ||
| domain: SINGLE_TENANT_ORG_DOMAIN, | ||
| })} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all references to getBrowsePath to understand its usage
rg -n "getBrowsePath" --type ts --type tsx -B 2 -A 5 | head -100Repository: sourcebot-dev/sourcebot
Length of output: 94
🏁 Script executed:
# Look for the getBrowsePath definition/export
rg -n "export.*getBrowsePath|function.*getBrowsePath|const.*getBrowsePath" --type ts --type tsxRepository: sourcebot-dev/sourcebot
Length of output: 94
🏁 Script executed:
# Check the 404NotFound component for available variables
cat -n packages/web/src/app/[domain]/browse/[...path]/components/404NotFound.tsx | head -120Repository: sourcebot-dev/sourcebot
Length of output: 5058
🏁 Script executed:
# Search for getBrowsePath without file type restriction
rg -n "getBrowsePath" -B 2 -A 10 | head -150Repository: sourcebot-dev/sourcebot
Length of output: 13923
🏁 Script executed:
# Find the hooks/utils file
fd -t f "utils" | grep -E "(hooks|browse)" | head -20Repository: sourcebot-dev/sourcebot
Length of output: 175
🏁 Script executed:
# Read the getBrowsePath function definition
cat -n packages/web/src/app/[domain]/browse/hooks/utils.tsRepository: sourcebot-dev/sourcebot
Length of output: 3984
Pass revisionName to getBrowsePath in the "Return to the repository overview" link to preserve branch context.
The getBrowsePath function supports an optional revisionName parameter that affects the generated URL. Without it, the link will default to the HEAD branch regardless of which branch the user was browsing when the 404 occurred. Since revisionName is available in the component, it should be passed to maintain consistent navigation context.
Example adjustment
<Link
href={getBrowsePath({
repoName: !('statusCode' in repoInfoResponse) ? repoInfoResponse.name : repoName,
path: '/',
pathType: 'tree',
domain: SINGLE_TENANT_ORG_DOMAIN,
+ revisionName,
})}🤖 Prompt for AI Agents
In `@packages/web/src/app/`[domain]/browse/[...path]/components/404NotFound.tsx
around lines 101 - 107, The "Return to the repository overview" Link in
404NotFound builds its href via getBrowsePath but omits the optional
revisionName, causing the link to drop branch context; update the Link's
getBrowsePath call to pass the component's revisionName value (alongside
repoName/repoInfoResponse.name, path '/', pathType 'tree' and
SINGLE_TENANT_ORG_DOMAIN) so the generated URL preserves the current branch;
modify the Link invocation where getBrowsePath is called (inside the 404NotFound
component) to include revisionName as the appropriate argument.
1 participant
The below ss is the updated screen for 404 not found when trying to browse for file that doesnot exist.
inspiration: Github
issue link: #827