diff --git a/src/server/app.js b/src/server/app.js index 666970f..a053086 100644 --- a/src/server/app.js +++ b/src/server/app.js @@ -31,6 +31,23 @@ const websocketProxyOption = { changeOrigin: true, } +function sendJson(res, statusCode, payload) { + res.statusCode = statusCode + res.setHeader('Content-Type', 'application/json') + res.end(JSON.stringify(payload)) +} + +function readJsonFileOr500(res, filePath, callback) { + jsonfile.readFile(filePath, (err, obj) => { + if (err) { + console.log('[ERROR]' + err) + sendJson(res, 500, { message: 'Failed to read server data' }) + return + } + callback(obj) + }) +} + Object.defineProperty(Array.prototype, 'flat', { value: function (depth = 1) { return this.reduce(function (flat, toFlatten) { @@ -64,6 +81,12 @@ const refresh = spawn('node', ['refresh.js'], { shell: true, stdio: 'inherit', }) +refresh.on('exit', (code) => { + console.log(`[ERROR] refresh.js exited with code ${code}. Leaderboard data may become stale.`) +}) +refresh.on('error', (err) => { + console.log(`[ERROR] Failed to start refresh.js: ${err.message}`) +}) process.on('exit', () => { refresh.kill() // kill it when exit }) @@ -75,15 +98,13 @@ const server = http switch (route) { case '/data': res.setHeader('Cache-Control', 'no-store') - jsonfile.readFile(dataPath, (err, obj) => { - if (err) console.log('[ERROR]' + err) + readJsonFileOr500(res, dataPath, (obj) => { res.end(JSON.stringify(obj)) }) break case '/log': res.setHeader('Cache-Control', 'no-store') - jsonfile.readFile(logPath, (err, obj) => { - if (err) console.log('[ERROR]' + err) + readJsonFileOr500(res, logPath, (obj) => { res.end(JSON.stringify(obj)) }) break @@ -107,7 +128,7 @@ const server = http ) var contributorsList = [] - Util.post(req, async (params) => { + Util.post(req, res, async (params) => { const { token } = params if (token === adminPassword) { await Promise.all( @@ -179,7 +200,7 @@ const server = http return } - Util.post(req, (params) => { + Util.post(req, res, (params) => { const { token, includedRepositories } = params if (token !== adminPassword) { @@ -200,7 +221,7 @@ const server = http res.end('Permission denied\n') return } - Util.post(req, (params) => { + Util.post(req, res, (params) => { const { token, startDate } = params if (token !== adminPassword) { @@ -222,7 +243,7 @@ const server = http return } - Util.post(req, (params) => { + Util.post(req, res, (params) => { const { token, interval } = params if (token !== adminPassword) { @@ -244,7 +265,7 @@ const server = http return } - Util.post(req, (params) => { + Util.post(req, res, (params) => { const { token, username } = params if (token !== adminPassword) { @@ -275,7 +296,7 @@ const server = http return } - Util.post(req, (params) => { + Util.post(req, res, (params) => { const { token, username } = params if (token !== adminPassword) { @@ -291,6 +312,10 @@ const server = http API.getContributorAvatar(username).then((result) => { if (result === '') { res.end(JSON.stringify({ message: 'Not found' })) + } else if (!result) { + res.end(JSON.stringify({ + message: 'Unable to fetch contributor profile. Check GitHub token and network connectivity.', + })) } else { // Add this contributor in config.json Config.contributors.unshift(username) @@ -336,8 +361,7 @@ const server = http return } - jsonfile.readFile(dataPath, async (err, obj) => { - if (err) console.log('[ERROR]' + err) + readJsonFileOr500(res, dataPath, async (obj) => { res.end(JSON.stringify(await API.getStats(obj))) }) break @@ -347,8 +371,7 @@ const server = http return } - jsonfile.readFile(dataPath, async (err, obj) => { - if (err) console.log('[ERROR]' + err) + readJsonFileOr500(res, dataPath, async (obj) => { const query = url.parse(req.url, true).query // Gets list of contributors sorted by parameter if provided @@ -385,8 +408,7 @@ const server = http return } - jsonfile.readFile(dataPath, async (err, obj) => { - if (err) console.log('[ERROR]' + err) + readJsonFileOr500(res, dataPath, async (obj) => { const query = url.parse(req.url, true).query if (query.username) { diff --git a/src/server/refresh.js b/src/server/refresh.js index 59f6c43..cfd5972 100644 --- a/src/server/refresh.js +++ b/src/server/refresh.js @@ -28,8 +28,9 @@ if (fs.existsSync(logPath)) { async function getAllContributorsInfo() { let Config = jsonfile.readFileSync(configPath) - let contributors = Config.contributors - let includedRepositories = Config.includedRepositories + let organization = process.env.ORGANIZATION || Config.organization + let contributors = Array.isArray(Config.contributors) ? Config.contributors : [] + let includedRepositories = Array.isArray(Config.includedRepositories) ? Config.includedRepositories : [] let startDate = Config.startDate interval = contributors.length < 150 ? 150 : (contributors.length + 10) // update interval @@ -41,11 +42,17 @@ async function getAllContributorsInfo() { await Promise.delay(delay * 1000) - API.getContributorInfo(process.env.ORGANIZATION, contributor, includedRepositories, startDate).then( res => { + try { + const res = await API.getContributorInfo( + organization, + contributor, + includedRepositories, + startDate + ) Config = jsonfile.readFileSync(configPath) // update Config delay = Config.delay // update delay - if (res.avatarUrl !== '' && res.issuesNumber !== -1 && res.mergedPRsNumber !== -1 && res.openPRsNumber != -1) { + if (res && res.avatarUrl !== '' && res.issuesNumber !== -1 && res.mergedPRsNumber !== -1 && res.openPRsNumber !== -1) { dataBuffer = jsonfile.readFileSync(dataPath) @@ -58,8 +65,12 @@ async function getAllContributorsInfo() { if (err) console.error(err) }) } + } else { + console.log(`[WARNING] Skipped update for ${contributor}. Check GitHub token, rate limit, or network status.`) } - }) + } catch (err) { + console.log(`[ERROR] Failed to update ${contributor}: ${err && err.message ? err.message : err}`) + } // Record time logBuffer.endtime = Date.now() @@ -71,4 +82,4 @@ async function getAllContributorsInfo() { } getAllContributorsInfo() -setInterval(getAllContributorsInfo, interval * delay * 1000) \ No newline at end of file +setInterval(getAllContributorsInfo, interval * delay * 1000) diff --git a/src/server/util/API.js b/src/server/util/API.js index 71b3c8d..ca59727 100644 --- a/src/server/util/API.js +++ b/src/server/util/API.js @@ -3,6 +3,19 @@ const chalk = require('chalk') const BASEURL = 'https://github.com' const APIHOST = 'https://api.github.com' +let hasLoggedBadCredentials = false + +function getRateLimitResetMessage(headers) { + const resetHeader = headers && headers['x-ratelimit-reset'] + if (!resetHeader) { + return '' + } + const resetTime = Number(resetHeader) * 1000 + if (!Number.isFinite(resetTime)) { + return '' + } + return ` Retry after ${new Date(resetTime).toISOString()}.` +} async function get(url, _authToken) { try { @@ -22,25 +35,40 @@ async function get(url, _authToken) { }) } catch (err) { if (err.code === 'ECONNABORTED') { - console.log(chalk.yellow('[WARNING] Time Out.')) + console.log(chalk.yellow('[WARNING] GitHub API request timed out.')) return } if (err.response !== undefined) { const message = err.response.data.message - switch (message) { - case 'Bad credentials': + if (message === 'Bad credentials') { + if (!hasLoggedBadCredentials) { + console.log( + chalk.red( + '[ERROR] GitHub token is invalid or expired. Please update the AUTH_TOKEN env variable and restart the server.' + ) + ) + hasLoggedBadCredentials = true + } + return + } + if (message && message.indexOf('API rate limit exceeded') === 0) { console.log( - chalk.red( - '[ERROR] Your GitHub Token is not correct! Please check the AUTH_TOKEN env variable.' + chalk.yellow( + '[WARNING] GitHub API rate limit exceeded.' + + getRateLimitResetMessage(err.response.headers) ) ) - process.exit() - break - default: - console.log(chalk.yellow('[WARNING] ' + message)) + return } + console.log(chalk.yellow('[WARNING] ' + message)) } else { - console.log(err) + console.log( + chalk.yellow( + `[WARNING] GitHub API request failed: ${ + err.message || 'Unknown network error' + }` + ) + ) } } } @@ -130,6 +158,9 @@ async function getContributorInfo( includedRepositories, startDate ) { + if (!Array.isArray(includedRepositories)) { + includedRepositories = [] + } const home = BASEURL + '/' + contributor const avatarUrl = await getContributorAvatar(contributor) let OpenPRsURL = `/search/issues?q=is:pr+author:${contributor}+is:Open+created:>=${startDate}` diff --git a/src/server/util/Util.js b/src/server/util/Util.js index 20eb983..c1f7263 100644 --- a/src/server/util/Util.js +++ b/src/server/util/Util.js @@ -1,4 +1,4 @@ -function post(req, callback) { +function post(req, res, callback) { if(req.method === 'POST') { let body = '' @@ -10,6 +10,9 @@ function post(req, callback) { try { callback(JSON.parse(body)) } catch (ex) { + res.statusCode = 400 + res.setHeader('Content-Type', 'application/json') + res.end(JSON.stringify({ message: 'Invalid JSON body' })) return } }) diff --git a/test/.eslintrc b/test/.eslintrc new file mode 100644 index 0000000..1f5ea06 --- /dev/null +++ b/test/.eslintrc @@ -0,0 +1,8 @@ +{ + "env": { + "node": true + }, + "globals": { + "test": "readonly" + } +} diff --git a/test/README.md b/test/README.md index ab94c72..8bad0ea 100644 --- a/test/README.md +++ b/test/README.md @@ -1,10 +1,15 @@ # Regression tests -This directory holds automated regression tests for stable leaderboard behavior. +This directory holds automated regression tests for server behavior. -`leaderboard-e2e.test.js` starts the current server code against a fixed Rocket.Chat snapshot and verifies that `/stats`, `/rank`, and selected `/contributor` and `/rank?username=` responses still match the checked-in expected output. +Current suites: -Node version used: +- `util-post.test.js` covers invalid JSON handling for admin `POST` bodies and related error-handling behavior. +- `leaderboard-e2e.test.js` starts the current server code against a fixed Rocket.Chat snapshot and verifies that `/stats`, `/rank`, and selected `/contributor` and `/rank?username=` responses still match the checked-in expected output. + +Tests are run with Node's built-in test runner (`node --test`). Node.js 18+ is required. + +Node version used for the leaderboard regression test: - Node.js `v25.4.0` @@ -13,7 +18,7 @@ Fixtures: - `../contrib/rocketchat/gsoc/2025/gsoc2025final.json` is the canonical snapshot used as the fixed leaderboard input. - `fixtures/gsoc2025final.expected.json` is the checked-in golden output generated from the current stable ranking logic and used for regression comparisons. -Run from the repo root: +From the repo root: ```bash npm i @@ -21,6 +26,6 @@ npm --prefix src/server install npm test ``` -Note: `npm i` at the repo root installs only root dependencies. The regression test boots `src/server/app.js`, so `src/server` dependencies must also be installed before running `npm test`. +Note: `npm i` at the repo root installs only root dependencies. The leaderboard regression test boots `src/server/app.js`, so `src/server` dependencies must also be installed before running `npm test`. -The test itself uses the env/path override support already available in the upstream dotenv-based server setup (`CONFIG_PATH`, `DATA_PATH`, `LOG_PATH`, `ADMINDATA_PATH`, `CONFIG_BACKUP_PATH`, `SERVER_PORT`) and does not require additional source changes under `src/server`. +The leaderboard test uses the env/path override support already available in the upstream dotenv-based server setup (`CONFIG_PATH`, `DATA_PATH`, `LOG_PATH`, `ADMINDATA_PATH`, `CONFIG_BACKUP_PATH`, `SERVER_PORT`) and does not require additional source changes under `src/server`. diff --git a/test/leaderboard-e2e.test.js b/test/leaderboard-e2e.test.js index b91cdd5..874b58a 100644 --- a/test/leaderboard-e2e.test.js +++ b/test/leaderboard-e2e.test.js @@ -86,9 +86,16 @@ before(async () => { childProcess.spawn = function (command, args) { if (command === 'node' && Array.isArray(args) && args[0] === 'refresh.js') { - return { + const mockChild = { kill() {}, + on() { + return mockChild + }, + once() { + return mockChild + }, } + return mockChild } return originalSpawn.apply(this, arguments) diff --git a/test/util-post.test.js b/test/util-post.test.js new file mode 100644 index 0000000..90f8b50 --- /dev/null +++ b/test/util-post.test.js @@ -0,0 +1,39 @@ +const { test } = require('node:test') +const assert = require('assert') +const { EventEmitter } = require('events') +const Util = require('../src/server/util/Util') + +test('Util.post sends 400 JSON when body is not valid JSON', async () => { + const req = new EventEmitter() + req.method = 'POST' + + let settle + const done = new Promise((resolve) => { + settle = resolve + }) + + const res = { + statusCode: 200, + headers: {}, + setHeader(name, value) { + this.headers[name] = value + }, + end(payload) { + this.body = payload + settle() + } + } + + Util.post(req, res, () => { + assert.fail('callback must not run when JSON.parse throws') + }) + + req.emit('data', Buffer.from('{not-json')) + req.emit('end') + + await done + + assert.strictEqual(res.statusCode, 400) + assert.strictEqual(res.headers['Content-Type'], 'application/json') + assert.deepStrictEqual(JSON.parse(res.body), { message: 'Invalid JSON body' }) +})