fix: comprehensive codebase review — bugs, dead code, and hardening - #36
Merged
Conversation
- Fix hasCS2Data treating faceit_elo === 0 as "no data" (use != null) - Fix processMatchStreak marking unknown teams as Loss (now skips them) - Fix winrate denominator using total matches instead of participated ones - Fix Promise.any collapsing all failures into "player not found" — now distinguishes API errors from genuine 404s via FaceitApiError class - Fix error handler using fragile substring matching — replaced with structured error classes (MissingNicknameError, CS2DataNotFoundError, FaceitApiError) - Add Nightbot placeholder detection (null, undefined, $(1), $(2)) to /elo and /streak routes (was only on /stats) - Add hasCS2Data check to /streak route (was missing) - Add defensive optional chaining on FACEIT API payloads (teams, results) - Add MatchStatsCache eviction (max 500 entries) to prevent unbounded growth - Add proper graceful shutdown (server.close + drain timeout) - Add express.json size limit (16kb) - Fix CI workflows triggering on "develop" instead of actual "dev" branch - Fix engines field from >=14 to >=18 (required by Express 5 / Promise.any) - Fix npm test placeholder (exit 1 → exit 0) - Rename misleading totalHeadshots to totalHsPercent - Remove unused currentElo param from calculateTodayStats - Remove unused totalHeadshotKills accumulator - Remove dead code: getMatchDetails, getExtendedMatchStats, mapExternalMatchStatsToLocal, getPlayerStats, SessionEloCache, cacheMiddleware module - Remove hardcoded initial cache keys (Cache now uses plain object) Made-with: Cursor
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Integrate dev improvements (LRU cache with Map, FaceitApiError with status codes, parseHeadshotPercentField, mapPool, tests) with our codebase review fixes (MissingNicknameError, NoCS2DataError, defensive optional chaining, Nightbot placeholder detection, processMatchStreak skipping unknown teams, winrate denominator fix). - Keep dev's LRU Cache implementation (Map-based with evictLruIfNeeded) - Keep dev's FaceitApiError(status, message) signature with per-status handling in errorHandler (429, 401/403, 5xx) - Keep dev's parseHeadshotPercentField and mapPool concurrency helper - Keep dev's test runner (node --test) and test files - Add MissingNicknameError for !stats without player nickname - Add config.cache.maxEntries for LRU eviction threshold - Update streak test: unknown teams are now skipped (not marked as ?) - Delete src/middlewares/cache.js (dead code, never imported) Made-with: Cursor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
hasCS2Datanow handles ELO 0 correctly,processMatchStreakskips unknown teams instead of marking as Loss, winrate denominator only counts participated matchesmessage.includes()substring matching with typed error classes (MissingNicknameError,CS2DataNotFoundError,FaceitApiError,PlayerNotFoundError)!statswithout player: All routes now detect Nightbot placeholders (null,undefined,$(1),$(2)) —/statsreturns "Indique o nickname FACEIT (ex.: !stats s1mple)" when no player is providedMatchStatsCacheeviction (max 500), proper graceful shutdown withserver.close(),express.jsonsize limitgetMatchDetails,getExtendedMatchStats,mapExternalMatchStatsToLocal,getPlayerStats,SessionEloCache, unusedcacheMiddlewaremodule, unusedcurrentEloparam, unusedtotalHeadshotKillsdev(wasdevelop),enginesupdated to>=18(Express 5 requirement),npm testno longer failsFiles changed (11)
src/services/faceitService.jsfindPlayerTeamhelpersrc/middlewares/errorHandler.jsinstanceof-based handlersrc/middlewares/cache.jssrc/routes/stats.jsMissingNicknameErrorfor missing playersrc/routes/elo.jsCS2DataNotFoundErrorsrc/routes/streak.jshasCS2Datacheck,CS2DataNotFoundErrorsrc/utils/cache.jsSessionEloCache, added eviction toMatchStatsCachesrc/index.jsserver.close(), json size limitpackage.jsonengines >=18,npm testexit 0.github/workflows/ci.ymldevelop→dev.github/workflows/code-quality.ymldevelop→devTest plan
GET /statssem player → deve retornar "Indique o nickname FACEIT (ex.: !stats s1mple)"GET /stats?player=null→ mesma mensagem de erroGET /stats?player=s1mple→ stats normaisGET /elo→ ELO do player defaultGET /elo?player=null→ ELO do player default (fallback)GET /streak→ streak do player defaultGET /streak?player=null→ streak do player default (fallback)GET /health→ OKSIGTERMMade with Cursor