From 92c389ef077186cb5d7626c3e175f8577d814ddc Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Sun, 8 Nov 2026 03:45:11 +0800 Subject: [PATCH] fix(social): honour timeoutMs in the REST fallback The REST helpers behind grant and profile calls passed no signal to fetch, so a Bot Social server that accepted the connection and never answered left rest() pending forever -- past the timeoutMs that only the MCP client used. Pass AbortSignal.timeout(timeoutMs) and map AbortError/TimeoutError to the same "no answer in Ns" wording callTool uses, so the fallback reports a timeout instead of hanging. A normal answer inside the deadline is unchanged. --- changelog.d/2633.fixed.md | 1 + docs/subsystems/social.md | 9 ++-- plugins/social/src/mcp-client.js | 33 ++++++++---- plugins/social/test/rest-timeout.test.js | 64 ++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 changelog.d/2633.fixed.md create mode 100644 plugins/social/test/rest-timeout.test.js diff --git a/changelog.d/2633.fixed.md b/changelog.d/2633.fixed.md new file mode 100644 index 000000000..8b8248fc3 --- /dev/null +++ b/changelog.d/2633.fixed.md @@ -0,0 +1 @@ +A grant or profile change that falls back to Bot Social's REST routes now gives up at the client's deadline instead of waiting forever, so a Bot Social server that accepts the connection and never answers shows a timeout error rather than a hung action ([#2633](https://github.com/ThinkFlowLab/FlyBot/issues/2633)). diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 3014f2257..dd91cc1d3 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -78,9 +78,12 @@ Bot Social accepts the FlyBot connector token on `/mcp`, every face-room REST ro now reads and writes through MCP `whoami` / `set_auto_approve` (#109). `mcp-client.js` enforces the same list in `connectorTokenAllowed`: a `rest()` path outside it fails locally with `route_not_allowed` before the token is sent, so a new call cannot quietly -start using a route Bot Social rejects. `plugins/social/test/connector-scope.test.js` -pins the list. The authority for the list is Bot Social's README and `docs/FACE_ROOMS.md` -§Auth. +start using a route Bot Social rejects. `rest()` runs on the same `timeoutMs` deadline as +the MCP client (`AbortSignal.timeout`, covering the body read): a server that accepts and +never answers rejects with `Bot Social : no answer in s` instead of +leaving the fallback pending (#2633). `plugins/social/test/connector-scope.test.js` pins +the list, and `plugins/social/test/rest-timeout.test.js` pins the deadline. The authority +for the list is Bot Social's README and `docs/FACE_ROOMS.md` §Auth. ### Inline approvals and compact Friends' Bots (#114, #115) diff --git a/plugins/social/src/mcp-client.js b/plugins/social/src/mcp-client.js index bbdaa9f0f..c8a3d13ed 100644 --- a/plugins/social/src/mcp-client.js +++ b/plugins/social/src/mcp-client.js @@ -337,16 +337,29 @@ export function createSocialApi({ server, token, fetch: fetchImpl = socialFetch, if (!connectorTokenAllowed(path)) { throw new BotSocialError(`Bot Social ${method} ${path}: a connector token is not accepted there`, 'route_not_allowed') } - const response = await fetchImpl(`${base}${path}`, { - method, - headers: { - authorization: `Bearer ${token}`, - accept: 'application/json', - ...(body !== undefined ? { 'content-type': 'application/json' } : {}), - }, - body: body !== undefined ? JSON.stringify(body) : undefined, - }) - const text = await response.text() + // The REST fallback shares the MCP client's deadline: a server that accepts and never answers + // is a timeout here too, and the signal covers reading the body (#2633). + const signal = AbortSignal.timeout(timeoutMs) + let response + let text + try { + response = await fetchImpl(`${base}${path}`, { + method, + headers: { + authorization: `Bearer ${token}`, + accept: 'application/json', + ...(body !== undefined ? { 'content-type': 'application/json' } : {}), + }, + body: body !== undefined ? JSON.stringify(body) : undefined, + signal, + }) + text = await response.text() + } catch (error) { + if (signal.aborted || error?.name === 'TimeoutError' || error?.name === 'AbortError') { + throw new BotSocialError(`Bot Social ${method} ${path}: no answer in ${Math.round(timeoutMs / 1000)}s`, 'timeout') + } + throw error + } let data try { data = text ? JSON.parse(text) : {} } catch { data = { message: text } } if (!response.ok) { diff --git a/plugins/social/test/rest-timeout.test.js b/plugins/social/test/rest-timeout.test.js new file mode 100644 index 000000000..49f23d0c8 --- /dev/null +++ b/plugins/social/test/rest-timeout.test.js @@ -0,0 +1,64 @@ +import assert from 'node:assert/strict' +import { createServer } from 'node:http' +import { test } from 'node:test' +import { createSocialApi } from '../src/mcp-client.js' + +// A Bot Social that accepts the connection and never answers, the way a server hung +// mid-deploy does. The sockets are tracked so the test can close over a held-open request. +function heldOpenServer(t) { + const sockets = new Set() + const server = createServer(() => { /* accepted, never answered */ }) + server.on('connection', socket => { + sockets.add(socket) + socket.on('close', () => sockets.delete(socket)) + }) + return new Promise(resolve => { + server.listen(0, '127.0.0.1', () => { + t.after(() => { + for (const socket of sockets) socket.destroy() + server.close() + }) + resolve(`http://127.0.0.1:${server.address().port}`) + }) + }) +} + +test('#2633: the REST fallback rejects at its deadline instead of hanging', async t => { + const server = await heldOpenServer(t) + const api = createSocialApi({ server, token: 'otk_test', timeoutMs: 200 }) + const started = Date.now() + + const outcome = await Promise.race([ + api.rest('GET', '/v1/bots/b1/grants').then( + value => ({ settled: 'resolved', value }), + error => ({ settled: 'rejected', error }), + ), + new Promise(resolve => { + const timer = setTimeout(() => resolve({ settled: 'pending' }), 1500) + timer.unref?.() + }), + ]) + const elapsed = Date.now() - started + + assert.equal(outcome.settled, 'rejected', `rest() was still ${outcome.settled} at ${elapsed}ms with a 200ms deadline`) + assert.equal(outcome.error?.code, 'timeout') + assert.equal(outcome.error?.message, 'Bot Social GET /v1/bots/b1/grants: no answer in 0s') + assert.ok(elapsed < 1000, `rest() took ${elapsed}ms with a 200ms deadline`) +}) + +test('#2633: a REST answer inside the deadline still returns its body', async t => { + const server = createServer((req, res) => { + setTimeout(() => { + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ ok: true, path: req.url })) + }, 20) + }) + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)) + t.after(() => server.close()) + + const api = createSocialApi({ server: `http://127.0.0.1:${server.address().port}`, token: 'otk_test', timeoutMs: 1000 }) + assert.deepEqual( + await api.rest('PATCH', '/v1/bots/b1/profile', { display_name: 'Chief' }), + { ok: true, path: '/v1/bots/b1/profile' }, + ) +})