Skip to content

⚡ Prevent cache stampede in OG image fetcher #480

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.

Sign up for GitHub

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

Closed
is0692vs wants to merge 1 commit into from
Closed

⚡ Prevent cache stampede in OG image fetcher #480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 32 additions & 14 deletions src/app/api/og/[username]/route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@
const ONE_DAY_IN_SECONDS = 24 * ONE_HOUR_IN_SECONDS;
const OG_CACHE_CONTROL = `public, max-age=${ONE_HOUR_IN_SECONDS}, s-maxage=${ONE_DAY_IN_SECONDS}, stale-while-revalidate=${ONE_DAY_IN_SECONDS}`;


// In-flight requests cache to prevent cache stampedes (thundering herd)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const inFlightRequests = new Map<string, Promise<any>>();
Comment on lines +16 to +18

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of using any and disabling the ESLint rule, we should define a strongly-typed interface for the GitHub profile data. This improves type safety, readability, and eliminates the need for the eslint-disable-next-line comment.

interface GitHubProfile {
  name?: string | null;
  bio?: string | null;
  avatar_url?: string | null;
  followers?: number;
  public_repos?: number;
}

// In-flight requests cache to prevent cache stampedes (thundering herd)
const inFlightRequests = new Map<string, Promise<GitHubProfile>>();


export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ username: string }> }
) {
const { username } = await params;


const ip = getClientIp(request);
const rateLimitResult = await rateLimiter.check(ip);

Expand All @@ -41,22 +47,34 @@
let followers = 0;
let publicRepos = 0;


try {
const res = await fetch(`https://api.github.com/users/${encodeURIComponent(username)}`, {
headers: {
Accept: "application/vnd.github.v3+json",
"User-Agent": "github-user-summary",
},
next: { revalidate: ONE_DAY_IN_SECONDS },
});
if (res.ok) {
const data = await res.json();
name = data.name ?? username;
bio = data.bio ?? "";
avatarUrl = data.avatar_url ?? "";
followers = data.followers ?? 0;
publicRepos = data.public_repos ?? 0;
let dataPromise = inFlightRequests.get(username);
if (!dataPromise) {
dataPromise = fetch(`https://api.github.com/users/${encodeURIComponent(username)}`, {
headers: {
Accept: "application/vnd.github.v3+json",
"User-Agent": "github-user-summary",
},
next: { revalidate: ONE_DAY_IN_SECONDS },
}).then(async (res) => {
if (!res.ok) {
throw new Error(`GitHub API responded with ${res.status}`);

@greptile-apps greptile-apps Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 想定内レスポンスのエラーログ化

!res.ok を例外にしたため、存在しないユーザーの 404 や GitHub の通常の rate limit 403 でも catch 側の logger.error が走るようになります。以前は非 2xx では静かにデフォルト表示へフォールバックしていたので、typo やスキャン、rate limit のような通常運用の入力で error ログが増え、監視ノイズになります。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/app/api/og/[username]/route.tsx
Line: 62

Comment:
**想定内レスポンスのエラーログ化**

`!res.ok` を例外にしたため、存在しないユーザーの 404 や GitHub の通常の rate limit 403 でも `catch` 側の `logger.error` が走るようになります。以前は非 2xx では静かにデフォルト表示へフォールバックしていたので、typo やスキャン、rate limit のような通常運用の入力で error ログが増え、監視ノイズになります。

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

}
return res.json();
});
Comment on lines +54 to +65

@greptile-apps greptile-apps Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 共有 Promise の失敗伝播

同じ username への同時リクエスト中に、先頭の GitHub fetch が一時的な 5xx/403 や JSON パース失敗になると、全 waiter が同じ rejection を受けてデフォルト値の OG 画像を返します。以前は各リクエストが独立していたため成功するリクエストが残る余地がありましたが、この変更後は 1 回の上流失敗で同時リクエスト全体が空の画像に寄り、成功時と同じ長いキャッシュ設定で配信されます。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/app/api/og/[username]/route.tsx
Line: 54-65

Comment:
**共有 Promise の失敗伝播**

同じ username への同時リクエスト中に、先頭の GitHub fetch が一時的な 5xx/403 や JSON パース失敗になると、全 waiter が同じ rejection を受けてデフォルト値の OG 画像を返します。以前は各リクエストが独立していたため成功するリクエストが残る余地がありましたが、この変更後は 1 回の上流失敗で同時リクエスト全体が空の画像に寄り、成功時と同じ長いキャッシュ設定で配信されます。

How can I resolve this? If you propose a fix, please make it concise.

inFlightRequests.set(username, dataPromise);
dataPromise.catch(() => {}).finally(() => {
inFlightRequests.delete(username);
});
Comment on lines +60 to +69

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The .catch(() => {}) block silently suppresses promise rejections on this detached promise chain. Since dataPromise is always awaited within the main try-catch block (where any errors are properly caught and logged), this .catch(() => {}) is redundant. Removing it avoids silent suppression and adheres to the repository's general rules. Additionally, we can remove the unnecessary async keyword from the .then callback and cast the JSON response to our strongly-typed GitHubProfile interface.

Suggested change
}).then(async (res) => {
if (!res.ok) {
throw new Error(`GitHub API responded with ${res.status}`);
}
return res.json();
});
inFlightRequests.set(username, dataPromise);
dataPromise.catch(() => {}).finally(() => {
inFlightRequests.delete(username);
});
}).then((res) => {
if (!res.ok) {
throw new Error(`GitHub API responded with ${res.status}`);
}
return res.json() as Promise<GitHubProfile>;
});
inFlightRequests.set(username, dataPromise);
dataPromise.finally(() => {
inFlightRequests.delete(username);
});
References
  1. When suppressing unhandled promise rejections, log the error instead of silently ignoring it to facilitate debugging.

}

const data = await dataPromise;
name = data.name ?? username;
bio = data.bio ?? "";
avatarUrl = data.avatar_url ?? "";
followers = data.followers ?? 0;
publicRepos = data.public_repos ?? 0;
} catch (error) {
logger.error(`Failed to fetch GitHub profile for OG image: ${username}`, error);
// fallback to defaults
Expand Down Expand Up @@ -84,7 +102,7 @@
}}
>
{avatarUrl && (
<img

Check warning on line 105 in src/app/api/og/[username]/route.tsx

View workflow job for this annotation

GitHub Actions / Lint 

Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` or a custom image loader to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element
src={sanitizeUrl(avatarUrl)}
alt=""
width={120}
Expand Down
Loading