Conversation
|
@dapperdodger is attempting to deploy a commit to the Producdevity's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request integrates Steam API key configuration, updates schema validation for mobile Steam stats to accept null values, and refactors Steam game data fetching with a new API endpoint, pagination support, request deduplication, timeout handling, and enhanced error logging. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/schemas/mobile.ts (1)
82-82: Consider applying the same.nullish()treatment to the Switch/3DS stats schemas for consistency.
GetSteamGamesStatsMobileSchemawas changed to.nullish()because tRPC was sending explicitnulland.optional()rejected it with BAD_REQUEST. The sibling schemasGetSwitchGamesStatsMobileSchema(line 82) andGetThreeDsGamesStatsMobileSchema(line 93) are still.optional()and their handlers presumably ignore the input in the same way. If any mobile client ever passesnullfor those stats endpoints, they will hit the same validation failure. Aligning all three reduces future surprises.♻️ Proposed consistency fix
-export const GetSwitchGamesStatsMobileSchema = z.object({}).optional() +export const GetSwitchGamesStatsMobileSchema = z.object({}).nullish() @@ -export const GetThreeDsGamesStatsMobileSchema = z.object({}).optional() +export const GetThreeDsGamesStatsMobileSchema = z.object({}).nullish()Also applies to: 93-93, 104-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/mobile.ts` at line 82, Change the two sibling schemas GetSwitchGamesStatsMobileSchema and GetThreeDsGamesStatsMobileSchema from .optional() to .nullish() so they accept explicit nulls the same way GetSteamGamesStatsMobileSchema does; locate their declarations and replace the .optional() call with .nullish() to avoid BAD_REQUEST when clients send null.src/server/utils/steamGameSearch.ts (1)
76-117: Add a safety cap on the pagination loop.The loop terminates only when the API sets
have_more_resultsto false (or omitslast_appid). If the Steam API ever misbehaves — e.g. returnshave_more_results: truewhile repeating the samelast_appid— this will spin forever, tying up a request and eventually exhausting the 30s timeout on each iteration (or not, if pages succeed quickly). Given the 50k page size and the ~164k total referenced in the PR, even a generous cap of ~20 iterations is more than enough headroom, and it also protects against a non-advancinglast_appid.🛡️ Proposed defensive guard
const allApps: SteamAppEntry[] = [] let lastAppId: number | undefined + let previousLastAppId: number | undefined + const MAX_PAGES = 20 + let pages = 0 do { + if (++pages > MAX_PAGES) { + throw new Error(`Steam GetAppList exceeded ${MAX_PAGES} pages; aborting to avoid infinite loop`) + } const url = new URL(STEAM_STORE_API_URL) // ... lastAppId = data.response.have_more_results ? data.response.last_appid : undefined + if (lastAppId !== undefined && lastAppId === previousLastAppId) { + throw new Error(`Steam GetAppList pagination did not advance (last_appid=${lastAppId})`) + } + previousLastAppId = lastAppId } while (lastAppId !== undefined)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/utils/steamGameSearch.ts` around lines 76 - 117, The pagination loop using lastAppId in the do/while (inside function handling SteamStoreApiResponse) can spin indefinitely if the API misreports have_more_results or repeats last_appid; add a defensive max-iteration cap (e.g. const MAX_PAGES = 20) and a page counter variable that increments each loop iteration inside the do block, and break or throw a clear error/log (using existing logging mechanism) when the counter exceeds MAX_PAGES; ensure you reference and preserve existing symbols like lastAppId, PAGE_SIZE, FETCH_TIMEOUT_MS and SteamStoreApiResponse and perform the check after updating lastAppId so the loop cannot continue beyond the cap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server/utils/steamGameSearch.ts`:
- Around line 131-149: The catch block mutates and re-logs the same Error
instance shared by concurrent awaiters of inflightFetch, causing duplicated logs
and nested messages; instead create and throw a new Error (wrapping the
original) and log only once: capture the original error from fetchSteamGamesData
in the catch, call processLogger/console.error with a single descriptive message
plus the original error, then throw a new Error like `new Error("Steam games
data fetch failed: " + originalMessage)` (or preserve original via Error.cause
if available) rather than modifying error.message; keep inflightFetch,
fetchSteamGamesData, and steamGamesDataCache usage unchanged.
---
Nitpick comments:
In `@src/schemas/mobile.ts`:
- Line 82: Change the two sibling schemas GetSwitchGamesStatsMobileSchema and
GetThreeDsGamesStatsMobileSchema from .optional() to .nullish() so they accept
explicit nulls the same way GetSteamGamesStatsMobileSchema does; locate their
declarations and replace the .optional() call with .nullish() to avoid
BAD_REQUEST when clients send null.
In `@src/server/utils/steamGameSearch.ts`:
- Around line 76-117: The pagination loop using lastAppId in the do/while
(inside function handling SteamStoreApiResponse) can spin indefinitely if the
API misreports have_more_results or repeats last_appid; add a defensive
max-iteration cap (e.g. const MAX_PAGES = 20) and a page counter variable that
increments each loop iteration inside the do block, and break or throw a clear
error/log (using existing logging mechanism) when the counter exceeds MAX_PAGES;
ensure you reference and preserve existing symbols like lastAppId, PAGE_SIZE,
FETCH_TIMEOUT_MS and SteamStoreApiResponse and perform the check after updating
lastAppId so the loop cannot continue beyond the cap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c044a1d6-95cb-4ef3-b103-a60682b571be
📒 Files selected for processing (3)
.env.examplesrc/schemas/mobile.tssrc/server/utils/steamGameSearch.ts
| // Deduplicate concurrent fetches so only one in-flight request runs at a time | ||
| if (!inflightFetch) { | ||
| inflightFetch = fetchSteamGamesData().finally(() => { | ||
| inflightFetch = null | ||
| }) | ||
| } | ||
|
|
||
| try { | ||
| const freshData = await fetchSteamGamesData() | ||
| const freshData = await inflightFetch | ||
| steamGamesDataCache.set(cacheKey, freshData) | ||
| return freshData | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error) | ||
| console.error('[steamGameSearch] fetchSteamGamesData failed:', message) | ||
| if (error instanceof Error) { | ||
| error.message = `Steam games data fetch failed: ${error.message}` | ||
| error.message = `Steam games data fetch failed: ${message}` | ||
| } | ||
| throw error | ||
| } |
There was a problem hiding this comment.
Concurrent awaiters of inflightFetch will mutate the same Error and double-log.
With the new dedup, multiple callers share the same rejected promise, so on failure every caller runs this catch: each one logs [steamGameSearch] fetchSteamGamesData failed: and each one prepends "Steam games data fetch failed: " to the same Error instance (the message getter now already contains the previous prefix). Net effect: duplicate error logs per cold start and a nested message like Steam games data fetch failed: Steam games data fetch failed: <original>. Mutating a caller-visible Error is also generally risky (loses original stack context for downstream callers like getSteamAppInfo and getSteamGamesStats).
Prefer wrapping in a fresh Error and logging only once (either in fetchSteamGamesData itself, or by checking a flag).
🛠️ Proposed fix — wrap instead of mutate
try {
const freshData = await inflightFetch
steamGamesDataCache.set(cacheKey, freshData)
return freshData
} catch (error) {
const message = error instanceof Error ? error.message : String(error)
console.error('[steamGameSearch] fetchSteamGamesData failed:', message)
- if (error instanceof Error) {
- error.message = `Steam games data fetch failed: ${message}`
- }
- throw error
+ throw new Error(`Steam games data fetch failed: ${message}`, {
+ cause: error instanceof Error ? error : undefined,
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Deduplicate concurrent fetches so only one in-flight request runs at a time | |
| if (!inflightFetch) { | |
| inflightFetch = fetchSteamGamesData().finally(() => { | |
| inflightFetch = null | |
| }) | |
| } | |
| try { | |
| const freshData = await fetchSteamGamesData() | |
| const freshData = await inflightFetch | |
| steamGamesDataCache.set(cacheKey, freshData) | |
| return freshData | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error) | |
| console.error('[steamGameSearch] fetchSteamGamesData failed:', message) | |
| if (error instanceof Error) { | |
| error.message = `Steam games data fetch failed: ${error.message}` | |
| error.message = `Steam games data fetch failed: ${message}` | |
| } | |
| throw error | |
| } | |
| // Deduplicate concurrent fetches so only one in-flight request runs at a time | |
| if (!inflightFetch) { | |
| inflightFetch = fetchSteamGamesData().finally(() => { | |
| inflightFetch = null | |
| }) | |
| } | |
| try { | |
| const freshData = await inflightFetch | |
| steamGamesDataCache.set(cacheKey, freshData) | |
| return freshData | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error) | |
| console.error('[steamGameSearch] fetchSteamGamesData failed:', message) | |
| throw new Error(`Steam games data fetch failed: ${message}`, { | |
| cause: error instanceof Error ? error : undefined, | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/utils/steamGameSearch.ts` around lines 131 - 149, The catch block
mutates and re-logs the same Error instance shared by concurrent awaiters of
inflightFetch, causing duplicated logs and nested messages; instead create and
throw a new Error (wrapping the original) and log only once: capture the
original error from fetchSteamGamesData in the catch, call
processLogger/console.error with a single descriptive message plus the original
error, then throw a new Error like `new Error("Steam games data fetch failed: "
+ originalMessage)` (or preserve original via Error.cause if available) rather
than modifying error.message; keep inflightFetch, fetchSteamGamesData, and
steamGamesDataCache usage unchanged.
1 participant
Description
Fixes the
games.findSteamAppId,games.getBestSteamAppId, andgames.batchBySteamAppIdstRPC endpoints returning empty results on every request.Root cause: Valve removed the
ISteamApps/GetAppList/v2endpoint — it now returns HTTP 404 with the messageMethod 'GetAppList' not found in interface 'ISteamApps'. The catch block ingetSteamGamesStatsswallowed the error silently, so the failure was invisible in logs and the cache remained perpetually empty.Changes:
ISteamApps/GetAppList/v2toIStoreService/GetAppList/v1, which requires aSTEAM_API_KEYenv var and supports pagination (up to 50k apps per page). The fetch now paginates untilhave_more_resultsis false, returning ~164k games filtered to games only (DLC, software, videos, hardware excluded).AbortControllertimeout to prevent silent hangs in serverless environments, and module-level in-flight deduplication so concurrent cold starts share one fetch instead of each firing independently.console.errorbefore returning/rethrowing, so the actual failure reason appears in server logs.GetSteamGamesStatsMobileSchemachanged from.optional()to.nullish()to accept thenulltRPC sends when there's no input (was causing aBAD_REQUESTvalidation error).STEAM_API_KEYadded to.env.examplewith a link tosteamcommunity.com/dev/apikey.Type of change
How Has This Been Tested?
Tested locally with
npx tsx --env-file=.env.local:getSteamGamesData()returns 163,962 games from the new paginated endpointgetSteamGamesStatstRPC endpoint returns{ success: true, totalGames: 163962, cacheStatus: "miss" }batchBySteamAppIdswith Half-Life 2 (steamAppId: "220") returnsmatchStrategy: "exact"and the full game recordfindSteamAppIdfuzzy search returns correct results with scoresChecklist
Notes for reviewers
STEAM_API_KEYmust be added to the production environment (Vercel / hosting provider) before deploying — the feature is completely broken without it. The key is free from https://steamcommunity.com/dev/apikey.Summary by CodeRabbit
Bug Fixes
Chores