From ec951bf36bc473efe00b83553285bd6af415e668 Mon Sep 17 00:00:00 2001 From: 2rdzy <232089760+2rdzy@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:14:05 +0200 Subject: [PATCH 1/2] Add a test harness, CI and current package metadata Add node:test tests against a small in-process Electrum server over TCP and TLS (with a committed, test-only certificate), covering requests, errors, concurrency, batches, notifications and close. Run them in CI on Node 18, 20 and 22. Point the repository, bugs and homepage fields at this fork, require Node 18 or later, and publish only index.js and lib. --- .github/workflows/test.yml | 19 ++++++++++ package.json | 31 +++++++++++----- test/client.test.js | 67 +++++++++++++++++++++++++++++++++ test/fixtures/cert.pem | 19 ++++++++++ test/fixtures/key.pem | 28 ++++++++++++++ test/helpers/client.js | 23 ++++++++++++ test/helpers/server.js | 76 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 254 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 test/client.test.js create mode 100644 test/fixtures/cert.pem create mode 100644 test/fixtures/key.pem create mode 100644 test/helpers/client.js create mode 100644 test/helpers/server.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..9df8ef9 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,19 @@ +name: test + +on: + push: + branches: [master] + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + node: [18, 20, 22] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + - run: npm test diff --git a/package.json b/package.json index d6bf75e..7e2cdbe 100644 --- a/package.json +++ b/package.json @@ -4,16 +4,16 @@ "description": "Electrum protocol client for React Native & Node.js", "main": "index.js", "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" + "test": "node --test test/*.test.js" }, "dependencies": {}, "devDependencies": {}, "repository": { "type": "git", - "url": "git://github.com/janoside/electrum-client.git" + "url": "git+https://github.com/2rdzy/electrum-client.git" }, "bugs": { - "url": "https://github.com/janoside/electrum-client/issues" + "url": "https://github.com/2rdzy/electrum-client/issues" }, "keywords": [ "bitcoin", @@ -21,13 +21,26 @@ "electrumx" ], "engines": { - "node": ">=6" + "node": ">=18" }, "contributors": [ - { "name": "Yuki Akiyama" }, - { "name": "7kharov" }, - { "name": "overtorment" }, - { "name": "janoside" } + { + "name": "Yuki Akiyama" + }, + { + "name": "7kharov" + }, + { + "name": "overtorment" + }, + { + "name": "janoside" + } ], - "license": "MIT" + "license": "MIT", + "homepage": "https://github.com/2rdzy/electrum-client#readme", + "files": [ + "index.js", + "lib" + ] } diff --git a/test/client.test.js b/test/client.test.js new file mode 100644 index 0000000..36653c9 --- /dev/null +++ b/test/client.test.js @@ -0,0 +1,67 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const { after, before, describe, test } = require('node:test'); + +const { startServer } = require('./helpers/server.js'); +const { withClient } = require('./helpers/client.js'); + +const handler = request => { + switch (request.method) { + case 'server.version': return ['test-server 1.0', '1.4']; + case 'echo': return request.params; + case 'blockchain.scripthash.get_balance': return { confirmed: request.params[0].length, unconfirmed: 0 }; + case 'fail': throw { code: 2, message: 'nope' }; + default: throw { code: -32601, message: `unknown method ${request.method}` }; + } +}; + +for (const [protocol, useTls] of [['tcp', false], ['tls', true]]) { + describe(`over ${protocol}`, () => { + let server; + + before(async () => { server = await startServer({ useTls, handler }); }); + after(() => server.close()); + + test('negotiates the protocol version', () => withClient(server, protocol, client => { + assert.deepEqual(client.versionInfo, ['test-server 1.0', '1.4']); + })); + + test('resolves a request with its result', () => withClient(server, protocol, async client => { + assert.deepEqual(await client.request('echo', ['a', 1]), ['a', 1]); + })); + + test('rejects a request with the server error', () => withClient(server, protocol, async client => { + await assert.rejects(client.request('fail', []), { code: 2, message: 'nope' }); + })); + + test('keeps concurrent requests apart', () => withClient(server, protocol, async client => { + const results = await Promise.all([1, 2, 3, 4, 5].map(n => client.request('echo', [n]))); + + assert.deepEqual(results, [[1], [2], [3], [4], [5]]); + })); + + test('resolves a batch with one entry per parameter, tagged with it', () => withClient(server, protocol, async client => { + const results = await client.blockchainScripthash_getBalanceBatch(['aa', 'bbbb', 'cccccc']); + + assert.deepEqual(results.map(r => r.param), ['aa', 'bbbb', 'cccccc']); + assert.deepEqual(results.map(r => r.result.confirmed), [2, 4, 6]); + })); + + test('emits subscription notifications', () => withClient(server, protocol, async client => { + const notified = new Promise(resolve => client.subscribe.once('blockchain.headers.subscribe', resolve)); + + server.broadcast(JSON.stringify({ jsonrpc: '2.0', method: 'blockchain.headers.subscribe', params: [{ height: 7 }] })); + + assert.deepEqual(await notified, [{ height: 7 }]); + })); + + test('refuses requests after close', async () => { + await withClient(server, protocol, async client => { + client.close(); + + await assert.rejects(client.request('echo', []), /Connection to server lost/); + }); + }); + }); +} diff --git a/test/fixtures/cert.pem b/test/fixtures/cert.pem new file mode 100644 index 0000000..a2ae783 --- /dev/null +++ b/test/fixtures/cert.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDJzCCAg+gAwIBAgIUC8QQdK2UWC7Gzx1+oiCJs2ear6wwDQYJKoZIhvcNAQEL +BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MCAXDTI2MTAwNTIwMTMyNVoYDzIxMjYw +OTExMjAxMzI1WjAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwggEiMA0GCSqGSIb3DQEB +AQUAA4IBDwAwggEKAoIBAQCpRxHcBqjb/VShru2qOjUGM+B9owDhiwKzM47xQ/iF +G7k0mtbFv9lHTL9NWTTEHy7prg461Z2XCP3QeNYYjV4udy9G2U9YuJuHHT+gx7br +Cr1Q6WT6kqc7RXe0R79GggqHpi02s3g/qbVO/OalBSvVBFpxxw9uMdG9Q7V5y7Jn +WI9NmYB/WRkBQS4dygw+wjS4igUF3j6vwR40gXm3QA66RxdSDNTOLnWLp6On92wj +DpmGNPciXCxhXquwW0o7ylRnQYsXpafHEMHl1uGjJQpXBuvgV/q1VmLFNKbp00T0 +YxiXV2gQA+IgnFMSQ7XKoKHPVJx1gPr97ERTiLYe/ci7AgMBAAGjbzBtMB0GA1Ud +DgQWBBT+7ktr7S5GLg39hICE95f36RZXzTAfBgNVHSMEGDAWgBT+7ktr7S5GLg39 +hICE95f36RZXzTAPBgNVHRMBAf8EBTADAQH/MBoGA1UdEQQTMBGCCWxvY2FsaG9z +dIcEfwAAATANBgkqhkiG9w0BAQsFAAOCAQEAhSP9Gxcxde1s8jfTStMn+VNmKGG3 +cWIVnw1IYf8PCTUx5z2gi1MjBpEV4QyL0tWNvfk0yfDCZJ24kXqLbgo338T7A5pQ +SfJJ1XMP/INRWtcsrnQZA4MdflD+9LzDjSaTNw8rLTQqemrRGLcDXfwmk+rAsGSJ +N20d/Txy1Ixlu7bBE95vMS5nHMK3P6XtlbZPGdDyZnN7JoomUYghWUkmtM00w1Ck +WPqNBCVsTRJZgyhgzvIXEBD7XcN4vMR6wWoTUhRxpW9QqlSQyzVr/Ik9DBh+QZ/n +DVdsD2+EMFtg0jzOJ9+fn+dMYpNJOjis8D1hAJZiku500/R2xjlmVdwYeg== +-----END CERTIFICATE----- diff --git a/test/fixtures/key.pem b/test/fixtures/key.pem new file mode 100644 index 0000000..03bc7b3 --- /dev/null +++ b/test/fixtures/key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCpRxHcBqjb/VSh +ru2qOjUGM+B9owDhiwKzM47xQ/iFG7k0mtbFv9lHTL9NWTTEHy7prg461Z2XCP3Q +eNYYjV4udy9G2U9YuJuHHT+gx7brCr1Q6WT6kqc7RXe0R79GggqHpi02s3g/qbVO +/OalBSvVBFpxxw9uMdG9Q7V5y7JnWI9NmYB/WRkBQS4dygw+wjS4igUF3j6vwR40 +gXm3QA66RxdSDNTOLnWLp6On92wjDpmGNPciXCxhXquwW0o7ylRnQYsXpafHEMHl +1uGjJQpXBuvgV/q1VmLFNKbp00T0YxiXV2gQA+IgnFMSQ7XKoKHPVJx1gPr97ERT +iLYe/ci7AgMBAAECggEABiIzx3aGT9JSFWRdQDnJChHUeeOaIzys5bQMjeQZ8S4b ++91QbZc8rD9fqdHS/k2S9EIA9KjLg67TuOYSpo5uHIjnsaLWI9oskyzAIsyZwtJ/ +dvBotEZJa6BiF1Tc3WqA2dbE9KJ0Ik+9PhYegAQ787AAsaFg3Gv5tD3r1B9JYO00 +I2nQ36zxw1YlOwHS5LBgYSePHlVP2BYo6M21C/JUs6oECL0DshNRkbE0qLv95trL +JHq0PRx/0ABwpcfgjEDftvA/kFsxDrP3HEnoHIdvGawO/wiURQQiIEEIwtGkMmTP +/7hXSUOf8XWrRk1+9ljxHpIhMEizbVgW0vjvQi9a4QKBgQDQSQhmzef9zM677qvs +xmjdP4S5XQMQXxCRXRYBVs15OURM+MRvWG+zhuhnMMI8tNe9jNCci3jpk5O5NKUN +Bn3P/SLGkr9odzD2wZ+9dw/KHpaCwuGqnngTGKgTaBWA0yyl6Kk59lUbABz0T9vN +87xREtUF2x752Vugpb7wZjt6RwKBgQDQDmr6fA/tUlrPxyb5QINXmFzNrv2WYCQn +L4mZi8fAaSUIb04+LGaKSEr8JhGKir5Vgy7OLQQT5AuPWeBkJcP324jddvJ03cdO +WSVGQ2r1pOjKz+N8egFVGCgi+YE21H2g6LbpXRH4vm/pLv1m7Agp+qy/mJkWe7kn +4GkVKStD7QKBgBhLLGuxQIndxY8t+2eWjMyhh/2G18ClFe2eid6GHVkyUMEZku/Z +xM2PwrOmgEFMZo15Syu3KNc1m1lSxp2KUEpeIBxBKYopvNBLLTKQHItanfuKd3/L +E/5GNfRtg3SPLPn89SAJl0ua68WjOuP+tcxPAgG42RgTPhiyvF6VzgIpAoGBAMJf +pewr+YlDNloFedDFR6+PRaxXPsy9NylwzQGpSE3nLVTq/9MwEgBAt+VH/31KT7V+ +ID04ayXWGEKtueh2K9YkwIH+vVcXVztIJ320QHoLvfy0KZhCN3pzXhQGu4QpEtFA +eJoefErCR7U3VEBSZPIrIjbBbsxFfF7X1fSSCLylAoGBAKZ652nLK3Tctg5F1PVm +PMO0K4Vs4rcXBURFq6vLB5QEs0pSC3kjYhrbT8cW6L8zThueFnGrK9SopGing/e8 +d6zhhkx5C+OzkDxbyirJKOzCtjrssVTEZId17dDPfr06YDXsD6+6pvFYxeDRxL5J +WhiL3lHrR0+npjrBWe9zJjpy +-----END PRIVATE KEY----- diff --git a/test/helpers/client.js b/test/helpers/client.js new file mode 100644 index 0000000..7b16b53 --- /dev/null +++ b/test/helpers/client.js @@ -0,0 +1,23 @@ +'use strict'; + +// The client expects net and tls to be injected as globals (see lib/client.js). +global.net = require('net'); +global.tls = require('tls'); + +const ElectrumClient = require('../../index.js'); + +// Connect a client to `server`, run `fn(client)`, and always close the client. +async function withClient(server, protocol, fn, { options, callbacks } = {}) { + const client = new ElectrumClient(server.port, '127.0.0.1', protocol, options, callbacks); + + await client.initElectrum({ client: 'electrum-client-test', version: '1.4' }, { retryPeriod: 100000, maxRetry: 0, callback: () => {} }); + + try { + return await fn(client); + + } finally { + client.close(); + } +} + +module.exports = { ElectrumClient, withClient }; diff --git a/test/helpers/server.js b/test/helpers/server.js new file mode 100644 index 0000000..fb88787 --- /dev/null +++ b/test/helpers/server.js @@ -0,0 +1,76 @@ +'use strict'; + +// A minimal Electrum-protocol server for tests: newline-delimited JSON-RPC over +// TCP or TLS. `handler(request)` returns the result for a request, or throws +// `{code, message}` to send an error. + +const fs = require('fs'); +const net = require('net'); +const path = require('path'); +const tls = require('tls'); + +const fixtures = path.join(__dirname, '..', 'fixtures'); + +const tlsOptions = { + key: fs.readFileSync(path.join(fixtures, 'key.pem')), + cert: fs.readFileSync(path.join(fixtures, 'cert.pem')), +}; + +function reply(request, handler) { + try { + return { jsonrpc: '2.0', id: request.id, result: handler(request) }; + + } catch (err) { + return { jsonrpc: '2.0', id: request.id, error: { code: err.code || -1, message: err.message || String(err) } }; + } +} + +async function startServer({ useTls = false, handler = req => req.params } = {}) { + const sockets = new Set(); + const received = []; + + const onConnection = socket => { + sockets.add(socket); + socket.on('close', () => sockets.delete(socket)); + socket.on('error', () => {}); + socket.setEncoding('utf8'); + + let buffer = ''; + socket.on('data', chunk => { + buffer += chunk; + + let index; + while ((index = buffer.indexOf('\n')) !== -1) { + const line = buffer.slice(0, index); + buffer = buffer.slice(index + 1); + + const message = JSON.parse(line); + received.push(message); + + const out = Array.isArray(message) + ? message.map(request => reply(request, handler)) + : reply(message, handler); + + socket.write(JSON.stringify(out) + '\n'); + } + }); + }; + + const server = useTls ? tls.createServer(tlsOptions, onConnection) : net.createServer(onConnection); + + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + + return { + port: server.address().port, + received, + sockets, + // send a raw line to every connected client (for notifications and malformed input) + broadcast: line => sockets.forEach(socket => socket.write(line + '\n')), + close: () => new Promise(resolve => { + sockets.forEach(socket => socket.destroy()); + server.close(resolve); + }), + }; +} + +module.exports = { startServer, tlsOptions }; From 5a51c3d0a02257e5f8e084aab52742647434b893 Mon Sep 17 00:00:00 2001 From: 2rdzy <232089760+2rdzy@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:25:36 +0200 Subject: [PATCH 2/2] Bound connecting and requests, and clean up on close - Enforce the connect timeout: the socket's 'timeout' event was never handled (and not forwarded by the TLS wrapper), so a stalled connection or TLS handshake hung. Destroy the socket on timeout, fail connect if the socket closes first, and keep the timeout armed until the connection (for TLS, the handshake) is established. Option connectTimeout, default 5000 ms. - Give up if the server accepts the connection but never answers server.version, instead of waiting for ever. - Add an optional requestTimeout (milliseconds, off by default) that fails a request which gets no reply and forgets its callback. - close() now rejects pending requests, cancels the reconnect and keep-alive timers, and stops reconnecting through a flag instead of replacing methods. Timers no longer keep the process alive. - TlsSocketWrapper: write before connect throws a clear error, end and destroy are harmless, removing an unknown listener no longer removes all listeners, and 'timeout' is forwarded. --- index.js | 65 +++++++++++++++- lib/TlsSocketWrapper.js | 30 ++++---- lib/client.js | 64 +++++++++++++--- test/helpers/server.js | 38 +++++++++- test/lifecycle.test.js | 161 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 325 insertions(+), 33 deletions(-) create mode 100644 test/lifecycle.test.js diff --git a/index.js b/index.js index 056cd73..86d0dbf 100644 --- a/index.js +++ b/index.js @@ -13,6 +13,7 @@ class ElectrumClient extends Client { }; this.timeLastCall = 0; + this.closed = false; } initElectrum(electrumConfig, persistencePolicy = { retryPeriod: 10000, maxRetry: 1000, pingPeriod: 120000, callback: null }) { @@ -22,7 +23,7 @@ class ElectrumClient extends Client { return new Promise((resolve, reject) => { this.connect().then(() => { - this.server_version(this.electrumConfig.client, this.electrumConfig.version).then((versionInfo) => { + this.withHandshakeTimeout(this.server_version(this.electrumConfig.client, this.electrumConfig.version)).then((versionInfo) => { this.versionInfo = versionInfo; if (this.onConnectCallback != null) { @@ -40,6 +41,31 @@ class ElectrumClient extends Client { }); } + // A server that accepts the connection but never answers server.version would otherwise + // leave initElectrum waiting for ever: give up after connectTimeout and drop the connection. + withHandshakeTimeout(promise) { + if (!(this.connectTimeout > 0)) { + return promise; + } + + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + this.conn.destroy(); + + reject(new Error('Timed out waiting for the server version')); + }, this.connectTimeout); + + if (timer.unref) { + timer.unref(); + } + + promise.then( + result => { clearTimeout(timer); resolve(result); }, + err => { clearTimeout(timer); reject(err); } + ); + }); + } + // Override parent request(method, params) { this.timeLastCall = new Date().getTime(); @@ -68,6 +94,10 @@ class ElectrumClient extends Client { onClose() { super.onClose(); + if (this.closed) { + return; + } + const list = [ 'server.peers.subscribe', 'blockchain.numblocks.subscribe', @@ -86,7 +116,15 @@ class ElectrumClient extends Client { this.onCloseCallback(this); } - setTimeout(() => { + if (this.reconnectTimer != null) { + clearTimeout(this.reconnectTimer); + } + + this.reconnectTimer = setTimeout(() => { + if (this.closed) { + return; + } + if (this.persistencePolicy != null && this.persistencePolicy.maxRetry > 0) { this.reconnect().catch((err) => { this.onError(err); @@ -103,10 +141,19 @@ class ElectrumClient extends Client { }); } }, retryPeriod); + + // a pending reconnect must not keep the process alive on its own + if (this.reconnectTimer.unref) { + this.reconnectTimer.unref(); + } } // ElectrumX persistancy keepAlive() { + if (this.closed) { + return; + } + if (this.timeout != null) { clearTimeout(this.timeout); } @@ -123,19 +170,31 @@ class ElectrumClient extends Client { }); } }, pingPeriod); + + if (this.timeout.unref) { + this.timeout.unref(); + } } close() { + this.closed = true; + super.close(); if (this.timeout != null) { clearTimeout(this.timeout); } - this.reconnect = this.reconnect = this.onClose = this.keepAlive = () => {}; // dirty hack to make it stop reconnecting + if (this.reconnectTimer != null) { + clearTimeout(this.reconnectTimer); + } } reconnect() { + if (this.closed) { + return Promise.reject(new Error('Client is closed')); + } + this.log("Electrum attempting reconnect..."); this.initSocket(); diff --git a/lib/TlsSocketWrapper.js b/lib/TlsSocketWrapper.js index 13c373f..b896716 100644 --- a/lib/TlsSocketWrapper.js +++ b/lib/TlsSocketWrapper.js @@ -46,24 +46,15 @@ class TlsSocketWrapper { this._listeners[event] = this._listeners[event] || []; let newListeners = []; - let found = false; for (let savedListener of this._listeners[event]) { - if (savedListener == listener) { - // found our listener - found = true; - // we just skip it - } else { + if (savedListener != listener) { // other listeners should go back to original array newListeners.push(savedListener); } } - if (found) { - this._listeners[event] = newListeners; - } else { - // something went wrong, lets just cleanup all listeners - this._listeners[event] = []; - } + // removing a listener that is not there leaves the others alone + this._listeners[event] = newListeners; } connect(port, host, callback) { @@ -96,6 +87,9 @@ class TlsSocketWrapper { this._socket.on('connection', data => { this._passOnEvent('connection', data); }); + this._socket.on('timeout', data => { + this._passOnEvent('timeout', data); + }); } _passOnEvent(event, data) { @@ -106,18 +100,22 @@ class TlsSocketWrapper { } emit(event, data) { - this._socket.emit(event, data); + if (this._socket) this._socket.emit(event, data); } end() { - this._socket.end(); + if (this._socket) this._socket.end(); } - destroy() { - this._socket.destroy(); + destroy(err) { + if (this._socket) this._socket.destroy(err); } write(data) { + if (!this._socket) { + throw new Error('Not connected'); + } + this._socket.write(data); } } diff --git a/lib/client.js b/lib/client.js index 7900023..ef1ed6e 100644 --- a/lib/client.js +++ b/lib/client.js @@ -11,7 +11,7 @@ * */ let net = global.net; let tls = global.tls; -const TIMEOUT = 5000; +const DEFAULT_CONNECT_TIMEOUT = 5000; const TlsSocketWrapper = require('./TlsSocketWrapper.js'); const EventEmitter = require('events').EventEmitter; @@ -30,6 +30,8 @@ class Client { }); this._protocol = protocol; // saving defaults this._options = options; + this.connectTimeout = (options && options.connectTimeout !== undefined) ? options.connectTimeout : DEFAULT_CONNECT_TIMEOUT; + this.requestTimeout = (options && options.requestTimeout) || 0; // 0: wait as long as it takes this.onErrorCallback = (callbacks && callbacks.onError) ? callbacks.onError : null; @@ -56,14 +58,17 @@ class Client { throw new Error('unknown protocol'); } - this.conn.setTimeout(TIMEOUT); + this.conn.setTimeout(this.connectTimeout); this.conn.setEncoding('utf8'); this.conn.setKeepAlive(true, 0); this.conn.setNoDelay(true); this.conn.on('connect', () => { - this.conn.setTimeout(0); this.onConnect(); }); + this.conn.on('timeout', () => { + // only armed until the connection is up (see connectSocket) or data arrives + this.conn.destroy(new Error('Connection timed out')); + }); this.conn.on('close', e => { this.onClose(e); }); @@ -88,11 +93,17 @@ class Client { connectSocket(conn, port, host) { return new Promise((resolve, reject) => { const errorHandler = e => reject(e); - + const closeHandler = () => reject(new Error('Connection closed before it was established')); + conn.on('error', errorHandler); + conn.on('close', closeHandler); conn.connect(port, host, () => { conn.removeListener('error', errorHandler); + conn.removeListener('close', closeHandler); + + // connected (for TLS: the handshake is done), so stop the connect timeout + conn.setTimeout(0); resolve(); }); @@ -106,6 +117,42 @@ class Client { this.conn.end(); this.conn.destroy(); this.status = 0; + + this.rejectPending(new Error('Connection closed')); + } + + rejectPending(err) { + Object.keys(this.callback_message_queue).forEach(key => { + const callback = this.callback_message_queue[key]; + delete this.callback_message_queue[key]; + + callback(err); + }); + } + + // Register `callback` for the response to request `id`. With a requestTimeout set, fail + // it (and forget it) when no response arrives in time. + registerCallback(id, callback) { + if (!(this.requestTimeout > 0)) { + this.callback_message_queue[id] = callback; + return; + } + + const timer = setTimeout(() => { + delete this.callback_message_queue[id]; + + callback(new Error(`Request timed out after ${this.requestTimeout} ms`)); + }, this.requestTimeout); + + if (timer.unref) { + timer.unref(); + } + + this.callback_message_queue[id] = (err, result) => { + clearTimeout(timer); + + callback(err, result); + }; } request(method, params) { @@ -115,7 +162,7 @@ class Client { return new Promise((resolve, reject) => { const id = ++this.id; const content = util.makeRequest(method, params, id); - this.callback_message_queue[id] = util.createPromiseResult(resolve, reject); + this.registerCallback(id, util.createPromiseResult(resolve, reject)); this.conn.write(content + '\n'); }); } @@ -137,7 +184,7 @@ class Client { arguments_far_calls[id] = param; } const content = '[' + contents.join(',') + ']'; - this.callback_message_queue[this.id] = util.createPromiseResultBatch(resolve, reject, arguments_far_calls); + this.registerCallback(this.id, util.createPromiseResultBatch(resolve, reject, arguments_far_calls)); // callback will exist only for max id this.conn.write(content + '\n'); }); @@ -188,10 +235,7 @@ class Client { onClose(e) { this.status = 0; - Object.keys(this.callback_message_queue).forEach(key => { - this.callback_message_queue[key](new Error('close connect')); - delete this.callback_message_queue[key]; - }); + this.rejectPending(new Error('close connect')); } onRecv(chunk) { diff --git a/test/helpers/server.js b/test/helpers/server.js index fb88787..ca2babc 100644 --- a/test/helpers/server.js +++ b/test/helpers/server.js @@ -16,9 +16,18 @@ const tlsOptions = { cert: fs.readFileSync(path.join(fixtures, 'cert.pem')), }; +// a handler may return NO_REPLY to leave a request unanswered +const NO_REPLY = Symbol('no reply'); + function reply(request, handler) { try { - return { jsonrpc: '2.0', id: request.id, result: handler(request) }; + const result = handler(request); + + if (result === NO_REPLY) { + return NO_REPLY; + } + + return { jsonrpc: '2.0', id: request.id, result }; } catch (err) { return { jsonrpc: '2.0', id: request.id, error: { code: err.code || -1, message: err.message || String(err) } }; @@ -48,10 +57,12 @@ async function startServer({ useTls = false, handler = req => req.params } = {}) received.push(message); const out = Array.isArray(message) - ? message.map(request => reply(request, handler)) + ? message.map(request => reply(request, handler)).filter(r => r !== NO_REPLY) : reply(message, handler); - socket.write(JSON.stringify(out) + '\n'); + if (out !== NO_REPLY && !(Array.isArray(out) && out.length === 0)) { + socket.write(JSON.stringify(out) + '\n'); + } } }); }; @@ -73,4 +84,23 @@ async function startServer({ useTls = false, handler = req => req.params } = {}) }; } -module.exports = { startServer, tlsOptions }; +// a TCP server that accepts connections and never says anything (so a TLS handshake stalls) +async function startSilentServer() { + const sockets = new Set(); + const server = net.createServer(socket => { + sockets.add(socket); + socket.on('error', () => {}); + }); + + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + + return { + port: server.address().port, + close: () => new Promise(resolve => { + sockets.forEach(socket => socket.destroy()); + server.close(resolve); + }), + }; +} + +module.exports = { startServer, startSilentServer, tlsOptions, NO_REPLY }; diff --git a/test/lifecycle.test.js b/test/lifecycle.test.js new file mode 100644 index 0000000..e9bd269 --- /dev/null +++ b/test/lifecycle.test.js @@ -0,0 +1,161 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const { spawn } = require('node:child_process'); +const path = require('node:path'); +const { describe, test } = require('node:test'); + +const { NO_REPLY, startServer, startSilentServer } = require('./helpers/server.js'); +const { ElectrumClient, withClient } = require('./helpers/client.js'); +const TlsSocketWrapper = require('../lib/TlsSocketWrapper.js'); + +const handler = request => { + switch (request.method) { + case 'server.version': return ['test-server 1.0', '1.4']; + case 'hang': return NO_REPLY; + default: return request.params; + } +}; + +const initConfig = { client: 'electrum-client-test', version: '1.4' }; +const noRetry = { retryPeriod: 10, maxRetry: 0, callback: () => {} }; + +describe('timeouts', () => { + for (const protocol of ['tcp', 'tls']) { + test(`a stalled ${protocol} connection fails after connectTimeout`, async () => { + const silent = await startSilentServer(); + + try { + const client = new ElectrumClient(silent.port, '127.0.0.1', protocol, { connectTimeout: 150 }, { onError: () => {} }); + const started = Date.now(); + + await assert.rejects(client.initElectrum(initConfig, noRetry), /[Tt]imed out/); + assert.ok(Date.now() - started < 2000); + + client.close(); + + } finally { + await silent.close(); + } + }); + } + + test('a request with no reply fails after requestTimeout and is forgotten', async () => { + const server = await startServer({ handler }); + + try { + await withClient(server, 'tcp', async client => { + await assert.rejects(client.request('hang', []), /timed out after 100 ms/); + + assert.equal(Object.keys(client.callback_message_queue).length, 0); + assert.deepEqual(await client.request('echo', ['still works']), ['still works']); + }, { options: { requestTimeout: 100 } }); + + } finally { + await server.close(); + } + }); + + test('without requestTimeout a request waits', async () => { + const server = await startServer({ handler }); + + try { + await withClient(server, 'tcp', async client => { + const outcome = await Promise.race([ + client.request('hang', []).then(() => 'answered', () => 'failed'), + new Promise(resolve => setTimeout(() => resolve('still waiting'), 300)), + ]); + + assert.equal(outcome, 'still waiting'); + }); + + } finally { + await server.close(); + } + }); +}); + +describe('close', () => { + test('rejects requests that are still pending', async () => { + const server = await startServer({ handler }); + + try { + await withClient(server, 'tcp', async client => { + const pending = client.request('hang', []); + + client.close(); + + await assert.rejects(pending, /Connection closed/); + }); + + } finally { + await server.close(); + } + }); + + test('does not reconnect afterwards', async () => { + const server = await startServer({ handler }); + + try { + const client = new ElectrumClient(server.port, '127.0.0.1', 'tcp', null, {}); + await client.initElectrum(initConfig, { retryPeriod: 20, maxRetry: 5, callback: null }); + + client.close(); + await new Promise(resolve => setTimeout(resolve, 200)); + + assert.equal(server.sockets.size, 0); + await assert.rejects(client.reconnect(), /closed/); + + } finally { + await server.close(); + } + }); + + test('leaves nothing running that keeps the process alive', async () => { + const server = await startServer({ handler }); + + try { + const script = ` + global.net = require('net'); global.tls = require('tls'); + const ElectrumClient = require(${JSON.stringify(path.join(__dirname, '..', 'index.js'))}); + const client = new ElectrumClient(${server.port}, '127.0.0.1', 'tcp', null, {}); + client.initElectrum({ client: 'x', version: '1.4' }, { retryPeriod: 600000, pingPeriod: 600000, maxRetry: 10, callback: null }) + .then(() => client.request('echo', [1])) + .then(() => server_side_close()); + function server_side_close() { client.close(); } + `; + + const child = spawn(process.execPath, ['-e', script], { stdio: 'ignore' }); + const exited = new Promise(resolve => child.on('exit', resolve)); + const timer = setTimeout(() => child.kill(), 5000); + + assert.equal(await exited, 0, 'the process should exit by itself after close()'); + clearTimeout(timer); + + } finally { + await server.close(); + } + }); +}); + +describe('TlsSocketWrapper', () => { + test('write before connect throws a clear error; end and destroy are harmless', () => { + const wrapper = new TlsSocketWrapper(require('tls')); + + assert.throws(() => wrapper.write('x'), /Not connected/); + wrapper.end(); + wrapper.destroy(); + }); + + test('removing an unknown listener keeps the others', () => { + const wrapper = new TlsSocketWrapper(require('tls')); + const seen = []; + const keep = data => seen.push(data); + + wrapper.on('data', keep); + wrapper.removeListener('data', () => {}); + wrapper._passOnEvent('data', 1); + + assert.deepEqual(seen, [1]); + }); +});