diff --git a/README.md b/README.md index 81f9c48..ade1612 100644 --- a/README.md +++ b/README.md @@ -82,24 +82,24 @@ A `queryTimeout` (in milliseconds) set here applies to **every** query run throu #### Configuration params: -| Parameter | Description | -| :----------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| user | The MySQL user to authenticate as. | -| password | The password of that MySQL user. | -| database | Name of the database to use for this connection. (Optional) | -| host | The hostname of the database you are connecting to. | -| port | The port number to connect to. (Default: 3306) | -| socketPath | The path to a unix domain socket to connect to. When used host and port are ignored. (Optional) | -| charset | The charset for the connection (collation). (Default: 'UTF8_GENERAL_CI') | -| timezone | The timezone configured on the MySQL server. (Default: 'local') | -| insecureAuth | Allow connecting to MySQL instances that ask for the old (insecure) authentication method. (Default: false) | -| flags | Connection flags. More information [here](https://github.com/mysqljs/mysql#connection-flags). | -| multipleStatements | Allow multiple mysql statements per query. (Default: true) | -| connectTimeout | Milliseconds before a timeout occurs during the initial connection to the MySQL server. (Default: 60000) | -| queryTimeout | Milliseconds before an unresponsive query is aborted with a timeout error. Prevents a hung query from blocking the chain forever — e.g. when the server silently drops an idle/pooled connection and mysql2 reports it only as a non-fatal "packets out of order" warning. Disabled by default (no timeout). Applies to every query of this executor and can be overridden per query in the plan. (Optional) | -| ssl/ca | SSL CA File (Optional) | -| ssl/cert | SSL CERT File (Optional) | -| ssl/key | SSL KEY File (Optional) | +| Parameter | Description | +| :----------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| user | The MySQL user to authenticate as. | +| password | The password of that MySQL user. | +| database | Name of the database to use for this connection. (Optional) | +| host | The hostname of the database you are connecting to. | +| port | The port number to connect to. (Default: 3306) | +| socketPath | The path to a unix domain socket to connect to. When used host and port are ignored. (Optional) | +| charset | The charset for the connection (collation). (Default: 'UTF8_GENERAL_CI') | +| timezone | The timezone configured on the MySQL server. (Default: 'local') | +| insecureAuth | Allow connecting to MySQL instances that ask for the old (insecure) authentication method. (Default: false) | +| flags | Connection flags. More information [here](https://github.com/mysqljs/mysql#connection-flags). | +| multipleStatements | Allow multiple mysql statements per query. (Default: true) | +| connectTimeout | Milliseconds before a timeout occurs during the initial connection to the MySQL server. (Default: 60000) | +| queryTimeout | Milliseconds before an unresponsive operation is aborted with a timeout error. It bounds both phases of the operation independently: the connection acquisition (connect + handshake, where mysql2 has no working timer once the first byte arrives) and the query execution itself. Prevents a hung connection or query from blocking the chain forever — e.g. when the server stalls mid-handshake or silently drops a connection and mysql2 reports it only as a non-fatal "packets out of order" warning. Disabled by default (no timeout). Applies to every query of this executor and can be overridden per query in the plan. (Optional) | +| ssl/ca | SSL CA File (Optional) | +| ssl/cert | SSL CERT File (Optional) | +| ssl/key | SSL KEY File (Optional) | ### Plan sample: diff --git a/index.js b/index.js index 7669e6d..bfb49a0 100644 --- a/index.js +++ b/index.js @@ -62,6 +62,16 @@ class mysqlExecutor extends Executor { } try { + // The query `timeout` above only arms once the command starts on an + // already-established connection: mysql2 disarms `connectTimeout` as + // soon as the FIRST byte arrives from the server, so a connection that + // stalls halfway through the handshake (e.g. after an out-of-sequence + // auth packet) would hang forever with no timer covering it. Bound the + // acquisition phase (connect + handshake) with the same deadline. + if (params.queryTimeout) { + await this.acquireConnection(connection, params.queryTimeout); + } + const queryStream = connection.query(queryOptions); const author = 'Runnerty'; const sheetName = 'Sheet'; @@ -216,6 +226,41 @@ class mysqlExecutor extends Executor { } } + // Proves the pool can deliver a fully-handshaken connection within + // `timeoutMs`. On success the connection is released back to the + // (per-execution) pool, where the subsequent `pool.query()` reuses it. + // On expiry every connection held by the pool is destroyed — a connection + // stuck mid-handshake is checked out, so `pool.end()` alone would never + // close it — and the promise rejects so the process always ends. + acquireConnection(pool, timeoutMs) { + return new Promise((resolve, reject) => { + let settled = false; + const timer = setTimeout(() => { + settled = true; + if (pool._allConnections && typeof pool._allConnections.toArray === 'function') { + for (const pending of pool._allConnections.toArray()) { + pending.destroy(); + } + } + reject(new Error(`connection acquire timeout after ${timeoutMs}ms (server stalled during connect/handshake)`)); + }, timeoutMs); + pool.getConnection((err, connection) => { + if (settled) { + if (connection) connection.destroy(); + return; + } + settled = true; + clearTimeout(timer); + if (err) { + reject(err); + } else { + connection.release(); + resolve(); + } + }); + }); + } + prepareEndOptions(firstRow, rowCounter, resultSetHeader, results) { //STANDARD OUPUT: this.endOptions.data_output = results || ''; diff --git a/test/acquire-timeout-repro.js b/test/acquire-timeout-repro.js new file mode 100644 index 0000000..300d6e4 --- /dev/null +++ b/test/acquire-timeout-repro.js @@ -0,0 +1,104 @@ +/* eslint-disable no-console */ +'use strict'; + +/** + * Manual reproduction script (not wired to `npm test`). + * + * Reproduces a production failure mode observed on AWS RDS MySQL 8.0: + * the server stalls halfway through the connection handshake and later + * emits an out-of-sequence auth packet. mysql2 prints + * `Warning: got packets out of order. Expected 2 but received 1`, + * disarms nothing and the connection hangs forever: + * - `connectTimeout` was already disarmed by the FIRST byte received. + * - the query `timeout` never arms, because it is armed in + * `Query.start()` and the command never starts. + * Without a bound on the acquisition phase, the executor never settles + * and the whole Runnerty chain stays locked until the host is restarted. + * + * Run: node test/acquire-timeout-repro.js + * PASS: the executor settles with a connection-acquire-timeout error + * shortly after `queryTimeout` expires. + * FAIL: nothing settles within 15s (behaviour prior to this fix). + */ + +const net = require('net'); +const MysqlExecutor = require('../index.js'); + +// Real handshake-init packet captured from MySQL 8.0.46 (seq 0). +const GREETING = Buffer.from( + '4a0000000a382e302e3436000b0000004668526a4064266400ffffff0200ffdf1500000000000000000000' + + '49697a0d1e43726870615e2f0063616368696e675f736861325f70617373776f726400', + 'hex' +); + +// AuthMoreData (0x01) + 0x04 = caching_sha2 "perform full authentication", +// sent with sequence id 1 while the client expects 2 → triggers the exact +// "got packets out of order. Expected 2 but received 1" warning seen in +// production, then the client waits forever for the server public key. +const DESYNCED_AUTH_MORE_DATA = Buffer.from([0x02, 0x00, 0x00, 0x01, 0x01, 0x04]); + +const QUERY_TIMEOUT_MS = 3000; +const VERDICT_AFTER_MS = 15000; + +function startStallServer(cb) { + const server = net.createServer(socket => { + socket.write(GREETING); + // Ignore the client HandshakeResponse entirely (stalled server) and + // after 1s send the desynced auth packet. Keep the socket open. + setTimeout(() => socket.write(DESYNCED_AUTH_MORE_DATA), 1000); + }); + server.listen(0, '127.0.0.1', () => cb(server)); +} + +startStallServer(server => { + const port = server.address().port; + + const executor = new MysqlExecutor({ + logger: { log: () => {} }, + checkExecutorParams: () => {}, + runtime: {}, + process: { + id: 'ACQUIRE-TIMEOUT-REPRO', + name: 'Acquire timeout repro', + uId: 'repro-uid', + exec: {}, + values: () => ({}) + } + }); + + const t0 = Date.now(); + let settled = false; + + // Capture the settle instead of going through the Runnerty runtime. + executor.end = options => { + settled = true; + const elapsed = Date.now() - t0; + const isError = options.end === 'error'; + const isAcquireTimeout = /acquire timeout/i.test(String(options.err_output || '')); + const inTime = elapsed < QUERY_TIMEOUT_MS + 3000; + console.log(`settled after ${elapsed}ms → end="${options.end}" err="${options.err_output}"`); + if (isError && isAcquireTimeout && inTime) { + console.log('PASS: acquisition phase bounded, process settled with an error'); + process.exit(0); + } + console.log('FAIL: settled, but not with the expected acquire-timeout error'); + process.exit(1); + }; + + executor.exec({ + user: 'root', + password: 'whatever', + database: 'whatever', + host: '127.0.0.1', + port, + command: 'SELECT 1', + queryTimeout: QUERY_TIMEOUT_MS + }); + + setTimeout(() => { + if (!settled) { + console.log(`FAIL: nothing settled after ${VERDICT_AFTER_MS}ms — the chain would hang forever`); + process.exit(1); + } + }, VERDICT_AFTER_MS); +});