GlcoinCore/RTL
1
0
Fork 0
mirror of https://github.com/Ride-The-Lightning/RTL.git synced 2026-08-13 12:33:07 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Address 2nd review: guard limiter callbacks, CLN postPeer aliases, one-shot done

Browse source 
Follow-up to the second #1629 review:

- F4: the limiter invokes its done callback outside the surrounding .then/.catch,
  so a throw in the response-send body became an unhandled rejection with no
  response (a 500 -> hang regression, notably on LND postPeer where the inner
  .catch was removed). Wrap each converted done body in try/catch that sends the
  error response, guarded by res.headersSent.
- F5: CLN postPeer re-listed peers but never resolved their aliases, so a freshly
  connected CLN peer came back with a raw node id (the frontend uses this response
  directly). Resolve aliases through the same bounded limiter, matching LND postPeer.
- F6: make runWithConcurrencyLimit fire 'done' exactly once via a one-shot guard,
  so multiple synchronous completions (e.g. non-function task elements) can't
  double-send the response.
This commit is contained in:
saubyk 2026-07-17 22:50:54 -07:00 • committed by Suheb
parent e11899a051
commit e0fce065d5
8 changed files with 133 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files

13
backend/controllers/cln/network.js
View file

@ -23,8 +23,17 @@ export const getRoute = (req, res, next) => {
// peers/channels paths, so a long route can't storm clnrest (#1501).
const getRouteAliasesTasks = (body.route || []).map((rt) => () => getAlias(req.session.selectedNode, rt, 'id'));
common.runWithConcurrencyLimit(getRouteAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Network Routes with Alias Received', data: body });
res.status(200).json(body || []);
// Guard the response-send: the limiter invokes this outside the surrounding .catch.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Network Routes with Alias Received', data: body });
res.status(200).json(body || []);
}
catch (e) {
const err = common.handleError(e, 'Network', 'Query Routes Error', req.session.selectedNode);
if (!res.headersSent) {
res.status(err.statusCode).json({ message: err.message, error: err.error });
}
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Network', 'Query Routes Error', req.session.selectedNode);

31
backend/controllers/cln/peers.js
View file

@ -20,8 +20,18 @@ export const getPeers = (req, res, next) => {
// with many peers and fails with "Resource temporarily unavailable (os error 11)" (#1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAlias(req.session.selectedNode, peer, 'id'));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers || []);
// The limiter invokes this outside the surrounding .then/.catch chain, so guard the
// response-send: a throw here would otherwise be an unhandled rejection with no response.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers || []);
}
catch (e) {
const err = common.handleError(e, 'Peers', 'List Peers Error', req.session.selectedNode);
if (!res.headersSent) {
res.status(err.statusCode).json({ message: err.message, error: err.error });
}
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'List Peers Error', req.session.selectedNode);
@ -42,8 +52,21 @@ export const postPeer = (req, res, next) => {
listOptions.url = req.session.selectedNode.settings.lnServerUrl + '/v1/listpeers';
request.post(listOptions).then((listPeersRes) => {
const peers = listPeersRes && listPeersRes.peers ? common.newestOnTop(listPeersRes.peers, 'id', connectRes.id) : [];
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: peers });
res.status(201).json(peers);
// Resolve aliases (bounded) for the returned peers so a freshly connected peer shows its
// alias rather than a raw node id, matching getPeers and the LND postPeer path (#1629 F5).
const getPeerAliasesTasks = peers.map((peer) => () => getAlias(req.session.selectedNode, peer, 'id'));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: peers });
res.status(201).json(peers);
}
catch (e) {
const err = common.handleError(e, 'Peers', 'Connect Peer Error', req.session.selectedNode);
if (!res.headersSent) {
res.status(err.statusCode).json({ message: err.message, error: err.error });
}
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode);
return res.status(err.statusCode).json({ message: err.message, error: err.error });

31
backend/controllers/lnd/peers.js
View file

@ -29,8 +29,17 @@ export const getPeers = (req, res, next) => {
// request per peer at once and overwhelm the backend (parity with the CLN fix, #1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAliasForPeers(req.session.selectedNode, peer));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers);
// Guard the response-send: the limiter invokes this outside the surrounding .catch.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers);
}
catch (e) {
const err = common.handleError(e, 'Peers', 'List Peers Error', req.session.selectedNode);
if (!res.headersSent) {
res.status(err.statusCode).json({ message: err.message, error: err.error });
}
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'List Peers Error', req.session.selectedNode);
@ -57,11 +66,21 @@ export const postPeer = (req, res, next) => {
// Bound concurrent alias lookups (parity with the CLN fix, #1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAliasForPeers(req.session.selectedNode, peer));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
if (body.peers) {
body.peers = common.newestOnTop(body.peers, 'pub_key', pubkey);
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: body });
// Guard the response-send: the limiter invokes this outside the surrounding .catch, and
// this replaced an explicit inner .catch — a throw here must not hang the POST (#1629 F4).
try {
if (body.peers) {
body.peers = common.newestOnTop(body.peers, 'pub_key', pubkey);
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: body });
}
res.status(201).json(body.peers);
}
catch (e) {
const err = common.handleError(e, 'Peers', 'Connect Peer Error', req.session.selectedNode);
if (!res.headersSent) {
res.status(err.statusCode).json({ message: err.message, error: err.error });
}
}
res.status(201).json(body.peers);
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode);

14
backend/utils/common.js
View file

@ -610,17 +610,27 @@ export class CommonService {
};
this.runWithConcurrencyLimit = (tasks, limit, done) => {
const results = new Array(tasks?.length || 0);
// 'done' must fire exactly once. Guard it: multiple runNext() completions (e.g. several
// non-function task elements draining synchronously) must not send the response twice.
let finished = false;
const finish = () => {
if (finished) {
return;
}
finished = true;
done(results);
};
// No tasks: the start loop below never runs, so 'done' would never fire and the
// response would hang. Resolve immediately for empty lists (e.g. a node with no peers).
if (!tasks || tasks.length === 0) {
return done(results);
return finish();
}
let nextIndex = 0;
let activeCount = 0;
const runNext = () => {
if (nextIndex >= tasks.length) {
if (activeCount === 0) {
done(results); // all tasks are finished
finish(); // all tasks are finished
}
return;
}

10
server/controllers/cln/network.ts
View file

@ -24,8 +24,14 @@ export const getRoute = (req, res, next) => {
// peers/channels paths, so a long route can't storm clnrest (#1501).
const getRouteAliasesTasks = (body.route || []).map((rt) => () => getAlias(req.session.selectedNode, rt, 'id'));
common.runWithConcurrencyLimit(getRouteAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Network Routes with Alias Received', data: body });
res.status(200).json(body || []);
// Guard the response-send: the limiter invokes this outside the surrounding .catch.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Network Routes with Alias Received', data: body });
res.status(200).json(body || []);
} catch (e) {
const err = common.handleError(e, 'Network', 'Query Routes Error', req.session.selectedNode);
if (!res.headersSent) { res.status(err.statusCode).json({ message: err.message, error: err.error }); }
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Network', 'Query Routes Error', req.session.selectedNode);

25
server/controllers/cln/peers.ts
View file

@ -20,8 +20,15 @@ export const getPeers = (req, res, next) => {
// with many peers and fails with "Resource temporarily unavailable (os error 11)" (#1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAlias(req.session.selectedNode, peer, 'id'));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers || []);
// The limiter invokes this outside the surrounding .then/.catch chain, so guard the
// response-send: a throw here would otherwise be an unhandled rejection with no response.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers || []);
} catch (e) {
const err = common.handleError(e, 'Peers', 'List Peers Error', req.session.selectedNode);
if (!res.headersSent) { res.status(err.statusCode).json({ message: err.message, error: err.error }); }
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'List Peers Error', req.session.selectedNode);
@ -41,8 +48,18 @@ export const postPeer = (req, res, next) => {
listOptions.url = req.session.selectedNode.settings.lnServerUrl + '/v1/listpeers';
request.post(listOptions).then((listPeersRes) => {
const peers = listPeersRes && listPeersRes.peers ? common.newestOnTop(listPeersRes.peers, 'id', connectRes.id) : [];
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: peers });
res.status(201).json(peers);
// Resolve aliases (bounded) for the returned peers so a freshly connected peer shows its
// alias rather than a raw node id, matching getPeers and the LND postPeer path (#1629 F5).
const getPeerAliasesTasks = peers.map((peer) => () => getAlias(req.session.selectedNode, peer, 'id'));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: peers });
res.status(201).json(peers);
} catch (e) {
const err = common.handleError(e, 'Peers', 'Connect Peer Error', req.session.selectedNode);
if (!res.headersSent) { res.status(err.statusCode).json({ message: err.message, error: err.error }); }
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode);
return res.status(err.statusCode).json({ message: err.message, error: err.error });

25
server/controllers/lnd/peers.ts
View file

@ -30,8 +30,14 @@ export const getPeers = (req, res, next) => {
// request per peer at once and overwhelm the backend (parity with the CLN fix, #1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAliasForPeers(req.session.selectedNode, peer));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers);
// Guard the response-send: the limiter invokes this outside the surrounding .catch.
try {
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Sorted Peers List Received', data: body.peers });
res.status(200).json(body.peers);
} catch (e) {
const err = common.handleError(e, 'Peers', 'List Peers Error', req.session.selectedNode);
if (!res.headersSent) { res.status(err.statusCode).json({ message: err.message, error: err.error }); }
}
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'List Peers Error', req.session.selectedNode);
@ -57,11 +63,18 @@ export const postPeer = (req, res, next) => {
// Bound concurrent alias lookups (parity with the CLN fix, #1501).
const getPeerAliasesTasks = peers.map((peer) => () => getAliasForPeers(req.session.selectedNode, peer));
common.runWithConcurrencyLimit(getPeerAliasesTasks, 20, () => {
if (body.peers) {
body.peers = common.newestOnTop(body.peers, 'pub_key', pubkey);
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: body });
// Guard the response-send: the limiter invokes this outside the surrounding .catch, and
// this replaced an explicit inner .catch — a throw here must not hang the POST (#1629 F4).
try {
if (body.peers) {
body.peers = common.newestOnTop(body.peers, 'pub_key', pubkey);
logger.log({ selectedNode: req.session.selectedNode, level: 'INFO', fileName: 'Peers', msg: 'Peers List after Connect Received', data: body });
}
res.status(201).json(body.peers);
} catch (e) {
const err = common.handleError(e, 'Peers', 'Connect Peer Error', req.session.selectedNode);
if (!res.headersSent) { res.status(err.statusCode).json({ message: err.message, error: err.error }); }
}
res.status(201).json(body.peers);
});
}).catch((errRes) => {
const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode);

12
server/utils/common.ts
View file

@ -583,16 +583,24 @@ export class CommonService {
public runWithConcurrencyLimit = (tasks, limit, done) => {
const results = new Array(tasks?.length || 0);
// 'done' must fire exactly once. Guard it: multiple runNext() completions (e.g. several
// non-function task elements draining synchronously) must not send the response twice.
let finished = false;
const finish = () => {
if (finished) { return; }
finished = true;
done(results);
};
// No tasks: the start loop below never runs, so 'done' would never fire and the
// response would hang. Resolve immediately for empty lists (e.g. a node with no peers).
if (!tasks || tasks.length === 0) { return done(results); }
if (!tasks || tasks.length === 0) { return finish(); }
let nextIndex = 0;
let activeCount = 0;
const runNext = () => {
if (nextIndex >= tasks.length) {
if (activeCount === 0) {
done(results); // all tasks are finished
finish(); // all tasks are finished
}
return;
}