-
Notifications
You must be signed in to change notification settings - Fork 3
Fix repo not found crash and improve 404 page #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
julianbenegas
wants to merge
2
commits into
main
Choose a base branch
from
claude/fix-repo-404-error-V70WA
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+221
−27
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| "use client" | ||
|
|
||
| import Link from "next/link" | ||
| import { usePathname } from "next/navigation" | ||
| import { Container } from "@/components/container" | ||
| import { Title } from "@/components/typography" | ||
|
|
||
| export default function PostNotFound() { | ||
| const pathname = usePathname() | ||
| const parts = pathname.split("/").filter(Boolean) | ||
| const owner = parts[0] | ||
| const repo = parts[1] | ||
|
|
||
| const repoUrl = owner && repo ? `/${owner}/${repo}` : "/" | ||
|
|
||
| return ( | ||
| <Container> | ||
| <div className="py-12"> | ||
| <Title>404 — Post Not Found</Title> | ||
| <p className="mt-4 text-muted"> | ||
| This post doesn't exist in{" "} | ||
| <span className="font-medium text-dim"> | ||
| {owner}/{repo} | ||
| </span> | ||
| . | ||
| </p> | ||
| <p className="mt-6 flex items-center gap-4 text-sm"> | ||
| <Link className="text-accent hover:underline" href={repoUrl}> | ||
| View all posts in {owner}/{repo} | ||
| </Link> | ||
| <Link className="text-accent hover:underline" href="/"> | ||
| Go home | ||
| </Link> | ||
| </p> | ||
| </div> | ||
| </Container> | ||
| ) | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| "use client" | ||
|
|
||
| import { ExternalLinkIcon } from "lucide-react" | ||
| import Link from "next/link" | ||
| import { usePathname } from "next/navigation" | ||
| import { Container } from "@/components/container" | ||
| import { Title } from "@/components/typography" | ||
|
|
||
| export default function CategoryNotFound() { | ||
| const pathname = usePathname() | ||
| const parts = pathname.split("/").filter(Boolean) | ||
| const owner = parts[0] | ||
| const repo = parts[1] | ||
|
|
||
| const repoUrl = owner && repo ? `/${owner}/${repo}` : "/" | ||
|
|
||
| return ( | ||
| <Container> | ||
| <div className="py-12"> | ||
| <Title>404 — Category Not Found</Title> | ||
| <p className="mt-4 text-muted"> | ||
| This category doesn't exist in{" "} | ||
| <span className="font-medium text-dim"> | ||
| {owner}/{repo} | ||
| </span> | ||
| . | ||
| </p> | ||
| <p className="mt-6 flex items-center gap-4 text-sm"> | ||
| <Link className="text-accent hover:underline" href={repoUrl}> | ||
| View all posts in {owner}/{repo} | ||
| </Link> | ||
| <Link className="text-accent hover:underline" href="/"> | ||
| Go home | ||
| </Link> | ||
| </p> | ||
| </div> | ||
| </Container> | ||
| ) | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| "use client" | ||
|
|
||
| import { ExternalLinkIcon } from "lucide-react" | ||
| import Link from "next/link" | ||
| import { usePathname } from "next/navigation" | ||
| import { Container } from "@/components/container" | ||
| import { Title } from "@/components/typography" | ||
|
|
||
| export default function RepoNotFound() { | ||
| const pathname = usePathname() | ||
| const parts = pathname.split("/").filter(Boolean) | ||
| const owner = parts[0] | ||
| const repo = parts[1] | ||
|
|
||
| const githubUrl = | ||
| owner && repo ? `https://github.com/${owner}/${repo}` : "https://github.com" | ||
|
|
||
| return ( | ||
| <Container> | ||
| <div className="py-12"> | ||
| <Title>404 — Repository Not Found</Title> | ||
| <p className="mt-4 text-muted"> | ||
| We couldn't find a repository at{" "} | ||
| <span className="font-medium text-dim"> | ||
| {owner}/{repo} | ||
| </span> | ||
| . | ||
| </p> | ||
| <p className="mt-6 flex items-center gap-2 text-sm"> | ||
| <span className="text-faint">Does this repo exist on GitHub?</span> | ||
| <Link | ||
| className="flex items-center gap-1 text-accent hover:underline" | ||
| href={githubUrl} | ||
| target="_blank" | ||
| > | ||
| Check on GitHub | ||
| <ExternalLinkIcon className="h-3.5 w-3.5" /> | ||
| </Link> | ||
| </p> | ||
| <p className="mt-6"> | ||
| <Link className="text-accent hover:underline" href="/"> | ||
| Go back home | ||
| </Link> | ||
| </p> | ||
| </div> | ||
| </Container> | ||
| ) | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import Link from "next/link" | ||
| import { Container } from "@/components/container" | ||
| import { Title } from "@/components/typography" | ||
|
|
||
| export default function NotFound() { | ||
| return ( | ||
| <Container> | ||
| <div className="py-12"> | ||
| <Title>404 — Page Not Found</Title> | ||
| <p className="mt-4 text-muted"> | ||
| The page you're looking for doesn't exist. | ||
| </p> | ||
| <p className="mt-6"> | ||
| <Link className="text-accent hover:underline" href="/"> | ||
| Go back home | ||
| </Link> | ||
| </p> | ||
| </div> | ||
| </Container> | ||
| ) | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-null assertion on postWithCategory! without null check could crash with TypeError if post is deleted between queries or data becomes inconsistent
View Details
📝 Patch Details
Analysis
BUG ANALYSIS:
The code on line 245 uses a non-null assertion without a defensive null check:
This was changed from a previous version that had an explicit null check (
if (!postWithCategory) { notFound() }). The developer's assumption is that because the post exists was verified in thePostPagefunction before the cache boundary, it will definitely exist inPostPageContent. However, this assumption breaks down for several reasons:Race Condition Between Queries: The
PostPagefunction checks if the post exists usinggetPostByNumber(), but thenPostPageContentqueries the database again independently. Between these two queries, the post could be deleted from the database.Different Query Structures: The first check uses
getPostByNumber()(from lib/data/posts), while the second uses a direct Drizzle query with a LEFT JOIN. These queries could diverge if there's data inconsistency or timing issues.Unsafe Destructuring: If
postWithCategoryis undefined at runtime, TypeScript's non-null assertion doesn't prevent a crash. Instead, the destructuring will throw:TypeError: Cannot destructure property 'category' of 'undefined' (reading 'category'). This is worse than a graceful 404.Edge Cases: While race conditions are rare, they're possible especially in high-concurrency scenarios or if the post is deleted between the initial check and the cache query.
FIX EXPLANATION:
The fix restores the null check that was previously present:
This handles the edge case where the post becomes unavailable due to:
By calling
notFound(), users get a proper 404 page instead of a server error. The check also removes the type-unsafe non-null assertion, making the code more robust.