Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions app/[owner]/[repo]/[postNumber]/not-found.tsx
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>
)
}
29 changes: 22 additions & 7 deletions app/[owner]/[repo]/[postNumber]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ export default async function PostPage({
}: {
params: Promise<{ owner: string; repo: string; postNumber: string }>
}) {
"use cache"

const { postNumber: postNumberStr, owner, repo } = await params

if (postNumberStr.endsWith(".md") || postNumberStr.endsWith(".txt")) {
Expand All @@ -136,6 +134,26 @@ export default async function PostPage({
notFound()
}

// Check post exists before cache boundary
const post = await getPostByNumber(owner, repo, postNumber)
if (!post) {
notFound()
}

return <PostPageContent owner={owner} postNumber={postNumber} repo={repo} />
}

async function PostPageContent({
owner,
repo,
postNumber,
}: {
owner: string
repo: string
postNumber: number
}) {
"use cache"

const [
postWithCategory,
allLlmUsers,
Expand Down Expand Up @@ -224,11 +242,8 @@ export default async function PostPage({
.where(and(eq(categories.owner, owner), eq(categories.repo, repo))),
])

if (!postWithCategory) {
notFound()
}

const { category, gitContexts, ...post } = postWithCategory
// Post existence already verified before cache boundary
const { category, gitContexts, ...post } = postWithCategory!
Comment on lines +245 to +246
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Post existence already verified before cache boundary
const { category, gitContexts, ...post } = postWithCategory!
// Defensive check: post should exist (verified before cache boundary),
// but we add this check to handle race conditions or data inconsistencies.
// If the post is not found, we notFound() even though we're in a cached function,
// as this is a data integrity issue that should be handled gracefully.
if (!postWithCategory) {
notFound()
}
const { category, gitContexts, ...post } = postWithCategory

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
diff --git a/app/[owner]/[repo]/[postNumber]/page.tsx b/app/[owner]/[repo]/[postNumber]/page.tsx
index 8450d34..bd795b2 100644
--- a/app/[owner]/[repo]/[postNumber]/page.tsx
+++ b/app/[owner]/[repo]/[postNumber]/page.tsx
@@ -242,8 +242,15 @@ async function PostPageContent({
       .where(and(eq(categories.owner, owner), eq(categories.repo, repo))),
   ])
 
-  // Post existence already verified before cache boundary
-  const { category, gitContexts, ...post } = postWithCategory!
+  // Defensive check: post should exist (verified before cache boundary),
+  // but we add this check to handle race conditions or data inconsistencies.
+  // If the post is not found, we notFound() even though we're in a cached function,
+  // as this is a data integrity issue that should be handled gracefully.
+  if (!postWithCategory) {
+    notFound()
+  }
+
+  const { category, gitContexts, ...post } = postWithCategory
   const gitContext = gitContexts?.[0] ?? null
 
   cacheTag(`post:${post.id}`)

Analysis

BUG ANALYSIS:

The code on line 245 uses a non-null assertion without a defensive null check:

const { category, gitContexts, ...post } = postWithCategory!

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 the PostPage function before the cache boundary, it will definitely exist in PostPageContent. However, this assumption breaks down for several reasons:

  1. Race Condition Between Queries: The PostPage function checks if the post exists using getPostByNumber(), but then PostPageContent queries the database again independently. Between these two queries, the post could be deleted from the database.

  2. 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.

  3. Unsafe Destructuring: If postWithCategory is 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.

  4. 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:

if (!postWithCategory) {
  notFound()
}

const { category, gitContexts, ...post } = postWithCategory

This handles the edge case where the post becomes unavailable due to:

  • Race conditions where post is deleted between queries
  • Data consistency issues between the two query structures
  • Any other unexpected conditions

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.

const gitContext = gitContexts?.[0] ?? null

cacheTag(`post:${post.id}`)
Expand Down
39 changes: 39 additions & 0 deletions app/[owner]/[repo]/category/[categorySlug]/not-found.tsx
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>
)
}
44 changes: 32 additions & 12 deletions app/[owner]/[repo]/category/[categorySlug]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,45 @@ export default async function CategoryPage({
}: {
params: Promise<{ owner: string; repo: string; categorySlug: string }>
}) {
"use cache"

const { owner, repo, categorySlug } = await params
cacheTag(`category:${categorySlug}`)

const [category, allLlmUsers, repoData] = await Promise.all([
// Check existence before cache boundary
const [category, repoData] = await Promise.all([
getCategoryBySlug(owner, repo, categorySlug),
getModelsForPicker(),
getGithubRepo(owner, repo),
])

if (!category) {
return notFound()
if (!category || !repoData) {
notFound()
}

if (!repoData) {
return notFound()
}
return (
<CategoryPageContent
category={category}
categorySlug={categorySlug}
owner={owner}
repo={repo}
/>
)
}

async function CategoryPageContent({
owner,
repo,
categorySlug,
category,
}: {
owner: string
repo: string
categorySlug: string
category: NonNullable<Awaited<ReturnType<typeof getCategoryBySlug>>>
}) {
"use cache"
cacheTag(`category:${categorySlug}`)

const categoryPosts = await db
const [allLlmUsers, categoryPosts] = await Promise.all([
getModelsForPicker(),
db
.select({
id: posts.id,
number: posts.number,
Expand All @@ -111,7 +130,8 @@ export default async function CategoryPage({
eq(posts.categoryId, category.id)
)
)
.orderBy(desc(posts.createdAt))
.orderBy(desc(posts.createdAt)),
])

const categoriesById = { [category.id]: category }

Expand Down
48 changes: 48 additions & 0 deletions app/[owner]/[repo]/not-found.tsx
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>
)
}
29 changes: 21 additions & 8 deletions app/[owner]/[repo]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,30 @@ export default async function RepoPage({
}: {
params: Promise<{ owner: string; repo: string }>
}) {
"use cache"

const { owner, repo } = await params

// Check repo exists before cache boundary
const repoData = await getGithubRepo(owner, repo)
if (!repoData) {
notFound()
}

return <RepoPageContent owner={owner} repo={repo} repoData={repoData} />
}

async function RepoPageContent({
owner,
repo,
repoData,
}: {
owner: string
repo: string
repoData: NonNullable<Awaited<ReturnType<typeof getGithubRepo>>>
}) {
"use cache"
cacheTag(`repo:${owner}:${repo}`)

const [repoPosts, repoCategories, allLlmUsers, repoData] = await Promise.all([
const [repoPosts, repoCategories, allLlmUsers] = await Promise.all([
db
.select({
id: posts.id,
Expand Down Expand Up @@ -91,13 +109,8 @@ export default async function RepoPage({
.from(categories)
.where(and(eq(categories.owner, owner), eq(categories.repo, repo))),
getModelsForPicker(),
getGithubRepo(owner, repo),
])

if (!repoData) {
return notFound()
}

const categoriesById = Object.fromEntries(
repoCategories.map((c) => [c.id, c])
)
Expand Down
21 changes: 21 additions & 0 deletions app/not-found.tsx
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>
)
}