From 33da6f768dbb8f37cb450402644fac315a30364e Mon Sep 17 00:00:00 2001 From: Thomas Praxl Date: Fri, 27 Sep 2024 15:43:10 +0200 Subject: [PATCH 1/3] fix(lib): circular dependency index.js <-> lib pool.js and pool_connection.js required index.js, and index.js required pool.js and pool_connection.js. Circular dependency can cause all sorts of problems. This fix aims to provide a step towards fixing a problem with @opentelemetry/instrumentation-mysql2, which looses several exports when using its patched require. For instance, `format` is lost and can cause an instrumented application to crash. --- lib/pool.js | 9 ++++----- lib/pool_connection.js | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/pool.js b/lib/pool.js index dc638d743e..7f73db8b00 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -1,8 +1,7 @@ 'use strict'; const process = require('process'); -const mysql = require('../index.js'); - +const SqlString = require('sqlstring'); const EventEmitter = require('events').EventEmitter; const PoolConnection = require('./pool_connection.js'); const Queue = require('denque'); @@ -214,7 +213,7 @@ class Pool extends EventEmitter { } format(sql, values) { - return mysql.format( + return SqlString.format( sql, values, this.config.connectionConfig.stringifyObjects, @@ -223,7 +222,7 @@ class Pool extends EventEmitter { } escape(value) { - return mysql.escape( + return SqlString.escape( value, this.config.connectionConfig.stringifyObjects, this.config.connectionConfig.timezone @@ -231,7 +230,7 @@ class Pool extends EventEmitter { } escapeId(value) { - return mysql.escapeId(value, false); + return SqlString.escapeId(value, false); } } diff --git a/lib/pool_connection.js b/lib/pool_connection.js index 78aac6d6b8..5760f9a468 100644 --- a/lib/pool_connection.js +++ b/lib/pool_connection.js @@ -1,6 +1,6 @@ 'use strict'; -const Connection = require('../index.js').Connection; +const Connection = require('./connection.js'); class PoolConnection extends Connection { constructor(pool, options) { From 40053ad8097289a736f59fdb3457dea1c14f5e98 Mon Sep 17 00:00:00 2001 From: Thomas Praxl Date: Fri, 27 Sep 2024 18:01:07 +0200 Subject: [PATCH 2/3] fix: circular dependency index.js <-> promise.js index.js and promise.js require each other. Circular dependency can cause all sorts of problems. This commit aims to fix a problem with @opentelemetry/instrumentation-mysql2, which looses several exports when using its patched require. For instance, `format` is lost and can cause an instrumented application to crash. --- common.js | 19 +++++++++++++++++++ index.js | 20 ++++++-------------- package.json | 1 + promise.js | 34 +++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 29 deletions(-) create mode 100644 common.js diff --git a/common.js b/common.js new file mode 100644 index 0000000000..66cd0b2bbb --- /dev/null +++ b/common.js @@ -0,0 +1,19 @@ +'use strict'; + +const Connection = require('./lib/connection.js'); +const ConnectionConfig = require('./lib/connection_config.js'); +const Pool = require('./lib/pool'); +const PoolConfig = require('./lib/pool_config'); +const PoolCluster = require('./lib/pool_cluster'); + +exports.Connection = Connection; +exports.createConnection = function(opts) { + return new Connection({ config: new ConnectionConfig(opts) }); +}; +exports.createPool = function(config) { + return new Pool({ config: new PoolConfig(config) }); +}; +exports.createPoolCluster = function(config) { + const PoolCluster = require('./lib/pool_cluster.js'); + return new PoolCluster(config); +}; diff --git a/index.js b/index.js index f33455e3f4..928eec4d42 100644 --- a/index.js +++ b/index.js @@ -2,32 +2,24 @@ const SqlString = require('sqlstring'); -const Connection = require('./lib/connection.js'); const ConnectionConfig = require('./lib/connection_config.js'); const parserCache = require('./lib/parsers/parser_cache'); -exports.createConnection = function(opts) { - return new Connection({ config: new ConnectionConfig(opts) }); -}; +const common = require('./common'); +exports.createConnection = common.createConnection exports.connect = exports.createConnection; -exports.Connection = Connection; +exports.Connection = common.Connection; exports.ConnectionConfig = ConnectionConfig; const Pool = require('./lib/pool.js'); const PoolCluster = require('./lib/pool_cluster.js'); -exports.createPool = function(config) { - const PoolConfig = require('./lib/pool_config.js'); - return new Pool({ config: new PoolConfig(config) }); -}; +exports.createPool = common.createPool -exports.createPoolCluster = function(config) { - const PoolCluster = require('./lib/pool_cluster.js'); - return new PoolCluster(config); -}; +exports.createPoolCluster = common.createPoolCluster; -exports.createQuery = Connection.createQuery; +exports.createQuery = common.Connection.createQuery; exports.Pool = Pool; diff --git a/package.json b/package.json index b05dd0b628..07ab95d93f 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "typings/mysql", "index.js", "index.d.ts", + "common.js", "promise.js", "promise.d.ts" ], diff --git a/promise.js b/promise.js index 5d80e21e72..04eaff6433 100644 --- a/promise.js +++ b/promise.js @@ -1,8 +1,12 @@ 'use strict'; -const core = require('./index.js'); +const SqlString = require('sqlstring'); +const common = require('./common.js'); const EventEmitter = require('events').EventEmitter; const parserCache = require('./lib/parsers/parser_cache.js'); +const PoolConnection = require('./lib/pool_connection.js'); +const Pool = require('./lib/pool.js'); +const PoolCluster = require('./lib/pool_cluster.js'); function makeDoneCb(resolve, reject, localErr) { return function (err, rows, fields) { @@ -249,7 +253,7 @@ class PromiseConnection extends EventEmitter { } function createConnection(opts) { - const coreConnection = core.createConnection(opts); + const coreConnection = common.createConnection(opts); const createConnectionErr = new Error(); const thePromise = opts.Promise || Promise; if (!thePromise) { @@ -286,12 +290,12 @@ function createConnection(opts) { const func = functionsToWrap[i]; if ( - typeof core.Connection.prototype[func] === 'function' && + typeof common.Connection.prototype[func] === 'function' && PromiseConnection.prototype[func] === undefined ) { PromiseConnection.prototype[func] = (function factory(funcName) { return function () { - return core.Connection.prototype[funcName].apply( + return common.Connection.prototype[funcName].apply( this.connection, arguments ); @@ -319,7 +323,7 @@ class PromisePoolConnection extends PromiseConnection { } destroy() { - return core.PoolConnection.prototype.destroy.apply( + return PoolConnection.prototype.destroy.apply( this.connection, arguments ); @@ -408,7 +412,7 @@ class PromisePool extends EventEmitter { } function createPool(opts) { - const corePool = core.createPool(opts); + const corePool = common.createPool(opts); const thePromise = opts.Promise || Promise; if (!thePromise) { throw new Error( @@ -426,12 +430,12 @@ function createPool(opts) { const func = functionsToWrap[i]; if ( - typeof core.Pool.prototype[func] === 'function' && + typeof Pool.prototype[func] === 'function' && PromisePool.prototype[func] === undefined ) { PromisePool.prototype[func] = (function factory(funcName) { return function () { - return core.Pool.prototype[funcName].apply(this.pool, arguments); + return Pool.prototype[funcName].apply(this.pool, arguments); }; })(func); } @@ -527,12 +531,12 @@ class PromisePoolCluster extends EventEmitter { const func = functionsToWrap[i]; if ( - typeof core.PoolCluster.prototype[func] === 'function' && + typeof PoolCluster.prototype[func] === 'function' && PromisePoolCluster.prototype[func] === undefined ) { PromisePoolCluster.prototype[func] = (function factory(funcName) { return function () { - return core.PoolCluster.prototype[funcName].apply(this.poolCluster, arguments); + return PoolCluster.prototype[funcName].apply(this.poolCluster, arguments); }; })(func); } @@ -542,7 +546,7 @@ class PromisePoolCluster extends EventEmitter { ]); function createPoolCluster(opts) { - const corePoolCluster = core.createPoolCluster(opts); + const corePoolCluster = common.createPoolCluster(opts); const thePromise = (opts && opts.Promise) || Promise; if (!thePromise) { throw new Error( @@ -557,10 +561,10 @@ function createPoolCluster(opts) { exports.createConnection = createConnection; exports.createPool = createPool; exports.createPoolCluster = createPoolCluster; -exports.escape = core.escape; -exports.escapeId = core.escapeId; -exports.format = core.format; -exports.raw = core.raw; +exports.escape = SqlString.escape; +exports.escapeId = SqlString.escapeId; +exports.format = SqlString.format; +exports.raw = SqlString.raw; exports.PromisePool = PromisePool; exports.PromiseConnection = PromiseConnection; exports.PromisePoolConnection = PromisePoolConnection; From fbb8ab11333b075a098093328e06bdc59fdef87a Mon Sep 17 00:00:00 2001 From: Thomas Praxl Date: Sat, 28 Sep 2024 13:50:55 +0200 Subject: [PATCH 3/3] refactor: split common.js into lib modules The proposal to put common.js into lib was good, but it felt weird to arbitrarily stuff just some exported functions in lib/common.js. This made sense when common.js was meant to provide commons for index.js and promise.js. But in the lib, this felt like a weird scope. I therefore split common.js into - lib/create_connection.js - lib/create_pool.js - lib/create_pool_cluster.js Also made `require` more consistent in all affected files: all `require` files now have a js suffix when they refer to a single local file. --- common.js | 19 ------------------- index.js | 18 ++++++++++-------- lib/create_connection.js | 10 ++++++++++ lib/create_pool.js | 10 ++++++++++ lib/create_pool_cluster.js | 9 +++++++++ package.json | 1 - promise.js | 27 +++++++++++++++------------ 7 files changed, 54 insertions(+), 40 deletions(-) delete mode 100644 common.js create mode 100644 lib/create_connection.js create mode 100644 lib/create_pool.js create mode 100644 lib/create_pool_cluster.js diff --git a/common.js b/common.js deleted file mode 100644 index 66cd0b2bbb..0000000000 --- a/common.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; - -const Connection = require('./lib/connection.js'); -const ConnectionConfig = require('./lib/connection_config.js'); -const Pool = require('./lib/pool'); -const PoolConfig = require('./lib/pool_config'); -const PoolCluster = require('./lib/pool_cluster'); - -exports.Connection = Connection; -exports.createConnection = function(opts) { - return new Connection({ config: new ConnectionConfig(opts) }); -}; -exports.createPool = function(config) { - return new Pool({ config: new PoolConfig(config) }); -}; -exports.createPoolCluster = function(config) { - const PoolCluster = require('./lib/pool_cluster.js'); - return new PoolCluster(config); -}; diff --git a/index.js b/index.js index 928eec4d42..0d7179c2a7 100644 --- a/index.js +++ b/index.js @@ -3,23 +3,25 @@ const SqlString = require('sqlstring'); const ConnectionConfig = require('./lib/connection_config.js'); -const parserCache = require('./lib/parsers/parser_cache'); +const parserCache = require('./lib/parsers/parser_cache.js'); -const common = require('./common'); -exports.createConnection = common.createConnection +const Connection = require('./lib/connection.js'); +exports.createConnection = require('./lib/create_connection.js'); exports.connect = exports.createConnection; -exports.Connection = common.Connection; +exports.Connection = Connection exports.ConnectionConfig = ConnectionConfig; const Pool = require('./lib/pool.js'); const PoolCluster = require('./lib/pool_cluster.js'); +const createPool = require('./lib/create_pool.js'); +const createPoolCluster = require('./lib/create_pool_cluster.js'); -exports.createPool = common.createPool +exports.createPool = createPool -exports.createPoolCluster = common.createPoolCluster; +exports.createPoolCluster = createPoolCluster; -exports.createQuery = common.Connection.createQuery; +exports.createQuery = Connection.createQuery; exports.Pool = Pool; @@ -34,7 +36,7 @@ exports.createServer = function(handler) { return s; }; -exports.PoolConnection = require('./lib/pool_connection'); +exports.PoolConnection = require('./lib/pool_connection.js'); exports.authPlugins = require('./lib/auth_plugins'); exports.escape = SqlString.escape; exports.escapeId = SqlString.escapeId; diff --git a/lib/create_connection.js b/lib/create_connection.js new file mode 100644 index 0000000000..d39618f78f --- /dev/null +++ b/lib/create_connection.js @@ -0,0 +1,10 @@ +'use strict'; + +const Connection = require('./connection.js'); +const ConnectionConfig = require('./connection_config.js'); + +function createConnection(opts) { + return new Connection({ config: new ConnectionConfig(opts) }); +} + +module.exports = createConnection; diff --git a/lib/create_pool.js b/lib/create_pool.js new file mode 100644 index 0000000000..7f782aff9a --- /dev/null +++ b/lib/create_pool.js @@ -0,0 +1,10 @@ +'use strict'; + +const Pool = require('./pool.js'); +const PoolConfig = require('./pool_config.js'); + +function createPool(config) { + return new Pool({ config: new PoolConfig(config) }); +} + +module.exports = createPool diff --git a/lib/create_pool_cluster.js b/lib/create_pool_cluster.js new file mode 100644 index 0000000000..4ded78b783 --- /dev/null +++ b/lib/create_pool_cluster.js @@ -0,0 +1,9 @@ +'use strict'; + +const PoolCluster = require('./pool_cluster.js'); + +function createPoolCluster(config) { + return new PoolCluster(config); +} + +module.exports = createPoolCluster; diff --git a/package.json b/package.json index 07ab95d93f..b05dd0b628 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,6 @@ "typings/mysql", "index.js", "index.d.ts", - "common.js", "promise.js", "promise.d.ts" ], diff --git a/promise.js b/promise.js index 04eaff6433..32cd8dc4c2 100644 --- a/promise.js +++ b/promise.js @@ -1,12 +1,15 @@ 'use strict'; const SqlString = require('sqlstring'); -const common = require('./common.js'); const EventEmitter = require('events').EventEmitter; const parserCache = require('./lib/parsers/parser_cache.js'); const PoolConnection = require('./lib/pool_connection.js'); const Pool = require('./lib/pool.js'); const PoolCluster = require('./lib/pool_cluster.js'); +const createConnection = require('./lib/create_connection.js'); +const Connection = require('./lib/connection.js'); +const createPool = require('./lib/create_pool.js'); +const createPoolCluster = require('./lib/create_pool_cluster.js'); function makeDoneCb(resolve, reject, localErr) { return function (err, rows, fields) { @@ -252,8 +255,8 @@ class PromiseConnection extends EventEmitter { } } -function createConnection(opts) { - const coreConnection = common.createConnection(opts); +function createConnectionPromise(opts) { + const coreConnection = createConnection(opts); const createConnectionErr = new Error(); const thePromise = opts.Promise || Promise; if (!thePromise) { @@ -290,12 +293,12 @@ function createConnection(opts) { const func = functionsToWrap[i]; if ( - typeof common.Connection.prototype[func] === 'function' && + typeof Connection.prototype[func] === 'function' && PromiseConnection.prototype[func] === undefined ) { PromiseConnection.prototype[func] = (function factory(funcName) { return function () { - return common.Connection.prototype[funcName].apply( + return Connection.prototype[funcName].apply( this.connection, arguments ); @@ -411,8 +414,8 @@ class PromisePool extends EventEmitter { } } -function createPool(opts) { - const corePool = common.createPool(opts); +function createPromisePool(opts) { + const corePool = createPool(opts); const thePromise = opts.Promise || Promise; if (!thePromise) { throw new Error( @@ -545,8 +548,8 @@ class PromisePoolCluster extends EventEmitter { 'add' ]); -function createPoolCluster(opts) { - const corePoolCluster = common.createPoolCluster(opts); +function createPromisePoolCluster(opts) { + const corePoolCluster = createPoolCluster(opts); const thePromise = (opts && opts.Promise) || Promise; if (!thePromise) { throw new Error( @@ -558,9 +561,9 @@ function createPoolCluster(opts) { return new PromisePoolCluster(corePoolCluster, thePromise); } -exports.createConnection = createConnection; -exports.createPool = createPool; -exports.createPoolCluster = createPoolCluster; +exports.createConnection = createConnectionPromise; +exports.createPool = createPromisePool; +exports.createPoolCluster = createPromisePoolCluster; exports.escape = SqlString.escape; exports.escapeId = SqlString.escapeId; exports.format = SqlString.format;