diff --git a/backend/controllers/cln/network.js b/backend/controllers/cln/network.js index 89137447..2e7db86b 100644 --- a/backend/controllers/cln/network.js +++ b/backend/controllers/cln/network.js @@ -99,9 +99,20 @@ export const getAlias = (selNode, peer, id) => { peer.alias = cached.alias; return Promise.resolve(peer); } - options.url = selNode.settings.lnServerUrl + '/v1/listnodes'; - options.body = { id: peerId }; - return request.post(options).then((body) => { + // Build a self-contained request from the selected node's own auth options rather than the + // shared module-level 'options', which is only set by a prior network.ts endpoint call. That + // coupling meant a cold Peers/route lookup (no prior network call) dereferenced a null 'options' + // and threw; now that the limiter swallows per-task throws, that surfaced as a 200 with every + // alias unset (#1501 review F1). selNode.authentication.options is guaranteed present here + // because every caller runs getOptions() first. + const nodeOptions = selNode.authentication?.options; + if (!nodeOptions || !nodeOptions.headers) { + peer.alias = peerId.substring(0, 20); + return Promise.resolve(peer); + } + const aliasOptions = { ...nodeOptions, method: 'POST', url: selNode.settings.lnServerUrl + '/v1/listnodes', body: { id: peerId }, json: true, qs: {} }; + delete aliasOptions.form; + return request.post(aliasOptions).then((body) => { logger.log({ selectedNode: selNode, level: 'DEBUG', fileName: 'Network', msg: 'Peer Alias Finished', data: body }); const alias = body.nodes?.[0]?.alias || peerId.substring(0, 20); // Re-insert so a refreshed entry moves to the most-recent position, then evict the diff --git a/backend/controllers/lnd/peers.js b/backend/controllers/lnd/peers.js index 104dc753..6a8ebf4c 100644 --- a/backend/controllers/lnd/peers.js +++ b/backend/controllers/lnd/peers.js @@ -25,7 +25,10 @@ export const getPeers = (req, res, next) => { request(options).then((body) => { logger.log({ selectedNode: req.session.selectedNode, level: 'DEBUG', fileName: 'Peers', msg: 'Peers List Received', data: body }); const peers = !body.peers ? [] : body.peers; - return Promise.all(peers?.map((peer) => getAliasForPeers(req.session.selectedNode, peer))).then((values) => { + // Bound concurrent alias lookups so a node with many peers can't fire one graph/node + // 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); }); @@ -51,15 +54,14 @@ export const postPeer = (req, res, next) => { options.url = req.session.selectedNode.settings.lnServerUrl + '/v1/peers'; request(options).then((body) => { const peers = (!body.peers) ? [] : body.peers; - return Promise.all(peers?.map((peer) => getAliasForPeers(req.session.selectedNode, peer))).then((values) => { + // 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 }); } res.status(201).json(body.peers); - }).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 }); }); }).catch((errRes) => { const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode); diff --git a/backend/utils/common.js b/backend/utils/common.js index 1a985d92..eff684d9 100644 --- a/backend/utils/common.js +++ b/backend/utils/common.js @@ -642,7 +642,10 @@ export class CommonService { runNext(); }); }; - for (let i = 0; i < limit && i < tasks.length; i++) { + // Normalize to at least 1: a non-positive limit would start no tasks, so 'done' (only + // reached from a task's finally) would never fire and the response would hang. + const startCount = Math.max(1, limit); + for (let i = 0; i < startCount && i < tasks.length; i++) { runNext(); } }; diff --git a/server/controllers/cln/network.ts b/server/controllers/cln/network.ts index a2448c07..18ef1065 100644 --- a/server/controllers/cln/network.ts +++ b/server/controllers/cln/network.ts @@ -100,10 +100,21 @@ export const getAlias = (selNode: SelectedNode, peer: any, id: string) => { return Promise.resolve(peer); } - options.url = selNode.settings.lnServerUrl + '/v1/listnodes'; - options.body = { id: peerId }; + // Build a self-contained request from the selected node's own auth options rather than the + // shared module-level 'options', which is only set by a prior network.ts endpoint call. That + // coupling meant a cold Peers/route lookup (no prior network call) dereferenced a null 'options' + // and threw; now that the limiter swallows per-task throws, that surfaced as a 200 with every + // alias unset (#1501 review F1). selNode.authentication.options is guaranteed present here + // because every caller runs getOptions() first. + const nodeOptions = selNode.authentication?.options; + if (!nodeOptions || !nodeOptions.headers) { + peer.alias = peerId.substring(0, 20); + return Promise.resolve(peer); + } + const aliasOptions = { ...nodeOptions, method: 'POST', url: selNode.settings.lnServerUrl + '/v1/listnodes', body: { id: peerId }, json: true, qs: {} }; + delete aliasOptions.form; - return request.post(options).then((body) => { + return request.post(aliasOptions).then((body) => { logger.log({ selectedNode: selNode, level: 'DEBUG', fileName: 'Network', msg: 'Peer Alias Finished', data: body }); const alias = body.nodes?.[0]?.alias || peerId.substring(0, 20); // Re-insert so a refreshed entry moves to the most-recent position, then evict the diff --git a/server/controllers/lnd/peers.ts b/server/controllers/lnd/peers.ts index 0723ea03..1816e6fe 100644 --- a/server/controllers/lnd/peers.ts +++ b/server/controllers/lnd/peers.ts @@ -26,7 +26,10 @@ export const getPeers = (req, res, next) => { request(options).then((body) => { logger.log({ selectedNode: req.session.selectedNode, level: 'DEBUG', fileName: 'Peers', msg: 'Peers List Received', data: body }); const peers = !body.peers ? [] : body.peers; - return Promise.all(peers?.map((peer) => getAliasForPeers(req.session.selectedNode, peer))).then((values) => { + // Bound concurrent alias lookups so a node with many peers can't fire one graph/node + // 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); }); @@ -51,15 +54,14 @@ export const postPeer = (req, res, next) => { options.url = req.session.selectedNode.settings.lnServerUrl + '/v1/peers'; request(options).then((body) => { const peers = (!body.peers) ? [] : body.peers; - return Promise.all(peers?.map((peer) => getAliasForPeers(req.session.selectedNode, peer))).then((values) => { + // 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 }); } res.status(201).json(body.peers); - }).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 }); }); }).catch((errRes) => { const err = common.handleError(errRes, 'Peers', 'Connect Peer Error', req.session.selectedNode); diff --git a/server/utils/common.ts b/server/utils/common.ts index 265a584f..c8020486 100644 --- a/server/utils/common.ts +++ b/server/utils/common.ts @@ -618,7 +618,10 @@ export class CommonService { }); }; - for (let i = 0; i < limit && i < tasks.length; i++) { + // Normalize to at least 1: a non-positive limit would start no tasks, so 'done' (only + // reached from a task's finally) would never fire and the response would hang. + const startCount = Math.max(1, limit); + for (let i = 0; i < startCount && i < tasks.length; i++) { runNext(); } };