diff --git a/server/model/links/links.db.js b/server/model/links/links.db.js index 295bb36..b25d33f 100644 --- a/server/model/links/links.db.js +++ b/server/model/links/links.db.js @@ -28,14 +28,26 @@ async function getBySteamId(steamId) { return rows[0] } -/** Every Steam account one website user holds, newest first. */ +/** + * Every Steam account one website user holds, newest first. + * + * **It joins `rust_players` for the name the game last saw**, and that is not a + * convenience. The name on the LINK is what the player was called at the moment + * they linked, which is a Rust name and changes on a whim — so a player who has + * renamed since sees a name they no longer use, on the one page of the site that + * is about who they are. The admin panel already preferred the newer one; this + * is the same rule applied where the person themselves is reading. + * + * A LEFT JOIN, because a player can link an account and never play on it. + */ async function listForUser(userId) { return core.query( - `SELECT steam_id AS steamId, user_id AS userId, name, server_id AS serverId, - linked_at AS linkedAt - FROM ${LINKS} - WHERE user_id = ? - ORDER BY linked_at DESC`, + `SELECT l.steam_id AS steamId, l.user_id AS userId, l.name, l.server_id AS serverId, + l.linked_at AS linkedAt, p.name AS playerName + FROM ${LINKS} l + LEFT JOIN ${PLAYERS} p ON p.steam_id = l.steam_id + WHERE l.user_id = ? + ORDER BY l.linked_at DESC`, [userId], ) } diff --git a/server/model/links/links.model.js b/server/model/links/links.model.js index 9962cab..3d76b8b 100644 --- a/server/model/links/links.model.js +++ b/server/model/links/links.model.js @@ -34,9 +34,20 @@ function shape(row) { } } -/** The Steam accounts one website user holds. */ +/** + * The Steam accounts one website user holds. + * + * The name is the one the GAME last saw, falling back to the one recorded when + * they linked — the rule the admin panel already used, applied on the page the + * player themselves reads. A browser walk found the two disagreeing: staff saw + * `Wanderer` and the player saw `Wanderer-old`, for the same person on the same + * site. + */ async function listForUser(userId) { - return (await db.listForUser(userId)).map(shape) + return (await db.listForUser(userId)).map((row) => ({ + ...shape(row), + name: row.playerName || row.name || null, + })) } /** True when this user holds this Steam id. The ownership gate every player read uses. */ diff --git a/server/router/admin/rust.controller.js b/server/router/admin/rust.controller.js index 6d006dd..34210d1 100644 --- a/server/router/admin/rust.controller.js +++ b/server/router/admin/rust.controller.js @@ -23,7 +23,7 @@ async function listServers(req, res) { res.json({ servers: await servers.listForAdmin() }) } catch (err) { log.error('failed to read the server list', { error: err.message }) - res.status(500).json({ error: 'Failed to read the server list' }) + res.status(500).json({ message: 'Failed to read the server list' }) } } @@ -39,7 +39,7 @@ async function putServer(req, res) { // Refusing it up front costs one round trip and saves that hunt. An EXISTING // row is a different case: omitting the token is how you say "leave it". if (!existing && !sidecarToken) { - return res.status(400).json({ error: 'A new server needs its sidecar token' }) + return res.status(400).json({ message: 'A new server needs its sidecar token' }) } await db.upsertServer({ @@ -71,7 +71,7 @@ async function putServer(req, res) { return res.status(204).end() } catch (err) { log.error('failed to save a server', { server: id, error: err.message }) - return res.status(500).json({ error: 'Failed to save the server' }) + return res.status(500).json({ message: 'Failed to save the server' }) } } @@ -80,7 +80,7 @@ async function deleteServer(req, res) { try { const existing = await db.getServer(id) - if (!existing) return res.status(404).json({ error: 'No such server' }) + if (!existing) return res.status(404).json({ message: 'No such server' }) await db.deleteServer(id) await core.activity.log({ req, action: 'rust.server.delete', detail: { server: id } }) @@ -88,7 +88,7 @@ async function deleteServer(req, res) { return res.status(204).end() } catch (err) { log.error('failed to delete a server', { server: id, error: err.message }) - return res.status(500).json({ error: 'Failed to delete the server' }) + return res.status(500).json({ message: 'Failed to delete the server' }) } } @@ -106,7 +106,7 @@ async function testServer(req, res) { try { const row = await db.getServer(id) - if (!row) return res.status(404).json({ error: 'No such server' }) + if (!row) return res.status(404).json({ message: 'No such server' }) const result = await sidecar.health(servers.withToken(row)) @@ -125,7 +125,7 @@ async function testServer(req, res) { }) } catch (err) { log.error('failed to probe a sidecar', { server: id, error: err.message }) - return res.status(500).json({ error: 'Failed to probe the sidecar' }) + return res.status(500).json({ message: 'Failed to probe the sidecar' }) } } diff --git a/server/router/admin/usersRust.controller.js b/server/router/admin/usersRust.controller.js index f26590d..6fb2592 100644 --- a/server/router/admin/usersRust.controller.js +++ b/server/router/admin/usersRust.controller.js @@ -27,7 +27,7 @@ async function listLinks(req, res) { res.json({ links: await links.forAdmin(req.params.id) }) } catch (err) { log.error('failed to read a user’s Rust links', { error: err.message }) - res.status(500).json({ error: 'Failed to read this user’s Rust accounts' }) + res.status(500).json({ message: 'Failed to read this user’s Rust accounts' }) } } @@ -50,7 +50,7 @@ async function removeLink(req, res) { try { const removed = await links.unlinkOwned(steamId, userId) - if (!removed) return res.status(404).json({ error: 'That account is not linked to this user' }) + if (!removed) return res.status(404).json({ message: 'That account is not linked to this user' }) // The one write this panel has, so it is the one thing here worth an audit // row: after phase 7 a link is what permissions are granted against, and @@ -64,7 +64,7 @@ async function removeLink(req, res) { return res.json({ unlinked: true }) } catch (err) { log.error('failed to unlink a Steam account', { error: err.message }) - return res.status(500).json({ error: 'Failed to unlink that account' }) + return res.status(500).json({ message: 'Failed to unlink that account' }) } } diff --git a/server/router/player/rust.controller.js b/server/router/player/rust.controller.js index 7240169..77161d2 100644 --- a/server/router/player/rust.controller.js +++ b/server/router/player/rust.controller.js @@ -31,7 +31,7 @@ async function listServers(req, res) { res.json({ servers: await servers.listPublic() }) } catch (err) { log.error('failed to read the server list', { error: err.message }) - res.status(500).json({ error: 'Failed to read the server list' }) + res.status(500).json({ message: 'Failed to read the server list' }) } } @@ -41,7 +41,7 @@ async function listLinks(req, res) { res.json({ links: await links.listForUser(req.user.id) }) } catch (err) { log.error('failed to read a player’s links', { error: err.message }) - res.status(500).json({ error: 'Failed to read your linked accounts' }) + res.status(500).json({ message: 'Failed to read your linked accounts' }) } } @@ -79,34 +79,34 @@ async function confirmLink(req, res) { // signed in, the account named is one they may well own, and without the // name the advice ("sign in as that account, or ask staff") is unusable. return res.status(409).json({ - error: result.username + message: result.username ? `That Steam account is already linked to ${result.username}. Run /unlink in game to release it.` : 'That Steam account is already linked to another website account. Run /unlink in game to release it.', }) case 'unsure': return res.status(503).json({ - error: + message: 'One of the servers could not be reached, so that code could not be checked. ' + 'Your code is still good — try again in a minute.', }) case 'offline': return res.status(503).json({ - error: 'The game servers are unreachable right now — try again in a minute.', + message: 'The game servers are unreachable right now — try again in a minute.', }) case 'no-servers': - return res.status(503).json({ error: 'No Rust servers are configured on this site yet.' }) + return res.status(503).json({ message: 'No Rust servers are configured on this site yet.' }) default: return res.status(400).json({ - error: 'That code is unknown or has expired. Type /link in game for a new one.', + message: 'That code is unknown or has expired. Type /link in game for a new one.', }) } } catch (err) { log.error('failed to confirm a link code', { error: err.message }) - return res.status(500).json({ error: 'Failed to confirm that code' }) + return res.status(500).json({ message: 'Failed to confirm that code' }) } } @@ -123,14 +123,14 @@ async function removeLink(req, res) { try { const removed = await links.unlinkOwned(steamId, req.user.id) - if (!removed) return res.status(404).json({ error: 'That account is not linked to you' }) + if (!removed) return res.status(404).json({ message: 'That account is not linked to you' }) await core.activity.log({ req, action: 'rust.account.unlink', detail: { steamId } }) return res.json({ unlinked: true }) } catch (err) { log.error('failed to unlink', { error: err.message }) - return res.status(500).json({ error: 'Failed to unlink that account' }) + return res.status(500).json({ message: 'Failed to unlink that account' }) } } diff --git a/server/router/public/rust.controller.js b/server/router/public/rust.controller.js index 8b799b6..b208378 100644 --- a/server/router/public/rust.controller.js +++ b/server/router/public/rust.controller.js @@ -21,7 +21,7 @@ async function listServers(req, res) { res.json({ servers: await servers.listPublic() }) } catch (err) { log.error('failed to read the server list', { error: err.message }) - res.status(500).json({ error: 'Failed to read the server list' }) + res.status(500).json({ message: 'Failed to read the server list' }) } } @@ -39,13 +39,13 @@ async function getServer(req, res) { try { const server = await servers.getPublic(req.params.id) if (!server) { - res.status(404).json({ error: 'No such server' }) + res.status(404).json({ message: 'No such server' }) return } res.json({ server }) } catch (err) { log.error('failed to read a server', { server: req.params.id, error: err.message }) - res.status(500).json({ error: 'Failed to read the server' }) + res.status(500).json({ message: 'Failed to read the server' }) } } @@ -69,7 +69,7 @@ async function listEvents(req, res) { }) } catch (err) { log.error('failed to read events', { server: req.params.id, error: err.message }) - res.status(500).json({ error: 'Failed to read events' }) + res.status(500).json({ message: 'Failed to read events' }) } } @@ -85,7 +85,7 @@ async function listLeaderboard(req, res) { }) } catch (err) { log.error('failed to read the leaderboard', { server: req.params.id, error: err.message }) - res.status(500).json({ error: 'Failed to read the leaderboard' }) + res.status(500).json({ message: 'Failed to read the leaderboard' }) } } @@ -94,7 +94,7 @@ async function listWipes(req, res) { res.json({ wipes: await events.wipes(req.params.id) }) } catch (err) { log.error('failed to read wipes', { server: req.params.id, error: err.message }) - res.status(500).json({ error: 'Failed to read wipes' }) + res.status(500).json({ message: 'Failed to read wipes' }) } } @@ -103,7 +103,7 @@ async function listOnline(req, res) { res.json({ players: await events.online(req.params.id) }) } catch (err) { log.error('failed to read presence', { server: req.params.id, error: err.message }) - res.status(500).json({ error: 'Failed to read who is online' }) + res.status(500).json({ message: 'Failed to read who is online' }) } } diff --git a/server/test/errorShape.test.js b/server/test/errorShape.test.js new file mode 100644 index 0000000..7b77f74 --- /dev/null +++ b/server/test/errorShape.test.js @@ -0,0 +1,134 @@ +// ── The field an error has to be in ─────────────────────────────────────── +// +// **The walk found this, and no test could have.** Core's request primitive is +// the only thing that reads a module's failures: +// +// const message = (data && data.message) || res.statusText || 'Request failed' +// +// So a body shaped `{ error: '…' }` is not rendered as a worse message — it is +// not rendered at all. The player sees `Service Unavailable`, which is what the +// link page showed for every one of the four sentences this phase exists to +// write, until a browser said so. +// +// This module answered `{ error }` from its first phase and got away with it, +// because until now every failure landed in `ErrorState` on a page whose whole +// content was missing — where a generic sentence is honest. A form is different: +// the sentence IS the outcome, and the four are not interchangeable. +// +// The rule is core's `Error` schema (`{ message }`), which every one of this +// module's `#swagger.responses` already pointed at. So this suite is the schema +// those annotations claim, asserted against what the handlers actually send. + +const test = require('node:test') +const assert = require('node:assert') + +const { fakeCtx } = require('./_fakes') + +function withCore() { + require('../core')._reset() + require('../core').init(fakeCtx()) +} + +/** A response double that records the status and the body. */ +function fakeRes() { + const res = { + statusCode: 200, + body: null, + status(code) { + res.statusCode = code + return res + }, + json(body) { + res.body = body + return res + }, + } + return res +} + +/** Every outcome `redeem` can answer, and the status each has to become. */ +const OUTCOMES = [ + [{ ok: false, reason: 'taken', username: 'someone-else' }, 409, /already linked to someone-else/], + [{ ok: false, reason: 'unsure' }, 503, /still good/], + [{ ok: false, reason: 'offline' }, 503, /unreachable/], + [{ ok: false, reason: 'no-servers' }, 503, /No Rust servers/], + [{ ok: false, reason: 'rejected' }, 400, /unknown or has expired/], +] + +test('every refusal reaches the player as a sentence, in the field core reads', async () => { + for (const [outcome, status, matches] of OUTCOMES) { + withCore() + const links = require('../model/links/links.model') + const controller = require('../router/player/rust.controller') + + links.redeem = async () => outcome + + const res = fakeRes() + await controller.confirmLink({ body: { code: 'K7M2PQ' }, user: { id: 4 } }, res) + + assert.equal(res.statusCode, status, `${outcome.reason} must be ${status}`) + assert.equal(typeof res.body.message, 'string', `${outcome.reason} sent no \`message\``) + assert.match(res.body.message, matches) + + // The half that is easy to leave behind while fixing this: a body carrying + // BOTH fields reads correctly in a browser and keeps the wrong shape alive + // for the next route that copies it. + assert.equal(res.body.error, undefined, `${outcome.reason} still carries an \`error\` field`) + } +}) + +test('the five outcomes are five different statuses-and-sentences, not one', async () => { + const seen = new Set() + + for (const [outcome] of OUTCOMES) { + withCore() + const links = require('../model/links/links.model') + const controller = require('../router/player/rust.controller') + + links.redeem = async () => outcome + + const res = fakeRes() + await controller.confirmLink({ body: { code: 'K7M2PQ' }, user: { id: 4 } }, res) + seen.add(res.body.message) + } + + // "That code is wrong" and "we could not reach the server that has it" send a + // player to do different things, and one of the two is a dead end when it is + // wrong — they run /link again on the server that is down and get the same + // answer for as long as it stays down. + assert.equal(seen.size, OUTCOMES.length, 'two outcomes tell the player the same thing') +}) + +test('no handler in this module answers in a field core cannot read', async () => { + // The other controllers, the same way — driven rather than grepped, because the + // shape that matters is what a handler SENDS. Each is given a model that throws, + // which is every controller's own 500 path and the one branch they all have. + withCore() + + const cases = [ + ['public', '../router/public/rust.controller', 'listServers', { params: {}, query: {} }], + ['player', '../router/player/rust.controller', 'listServers', { params: {}, query: {}, user: { id: 4 } }], + ['player', '../router/player/rust.controller', 'listLinks', { params: {}, user: { id: 4 } }], + ['admin', '../router/admin/rust.controller', 'listServers', { params: {}, query: {} }], + ['slot', '../router/admin/usersRust.controller', 'listLinks', { params: { id: '4' } }], + ] + + for (const [tier, modulePath, handler, req] of cases) { + withCore() + + // Core's `query` is the fake's spy; make it throw so every handler takes its + // failure branch. + require('../core')._reset() + require('../core').init(fakeCtx({ + db: { query: () => Promise.reject(new Error('the database is not there')), pool: {} }, + })) + + const controller = require(modulePath) + const res = fakeRes() + await controller[handler](req, res) + + assert.equal(res.statusCode, 500, `${tier}.${handler} did not fail`) + assert.equal(typeof res.body.message, 'string', `${tier}.${handler} sent no \`message\``) + assert.equal(res.body.error, undefined, `${tier}.${handler} answers in \`error\``) + } +}) diff --git a/server/test/links.test.js b/server/test/links.test.js index a6a5731..3a47cb5 100644 --- a/server/test/links.test.js +++ b/server/test/links.test.js @@ -274,3 +274,21 @@ test('the in-game unlink is scoped by Steam id alone, because that is the author assert.ok(!del.sql.includes('user_id')) assert.deepEqual(del.params, ['7656']) }) + +test('a player sees the name the GAME last saw, not the one they linked under', async () => { + withCore({ select: [[{ steamId: '7656', userId: 4, name: 'Wanderer-old', playerName: 'Wanderer', serverId: 'a', linkedAt: 'x' }]] }) + const links = require('../model/links/links.model') + + const [link] = await links.listForUser(4) + + // Found in a browser: staff saw `Wanderer` on the admin panel and the player + // saw `Wanderer-old` on their own page — the same person, labelled two ways on + // one site, because a Rust name changes on a whim and only one of the two reads + // was joining `rust_players`. + assert.equal(link.name, 'Wanderer') + + // And the fallback still holds for a link whose account has never played. + withCore({ select: [[{ steamId: '7656', userId: 4, name: 'Wanderer-old', playerName: null, linkedAt: 'x' }]] }) + const again = require('../model/links/links.model') + assert.equal((await again.listForUser(4))[0].name, 'Wanderer-old') +})