Unhandled rejection in dbCleanup

[rkaw92]

It appears that the periodic session clean-up function, dbCleanup, misses error handling. Also, there is no point in the API that would allow the user of the library to add such handling. This causes an unhandled promise rejection in case the clean-up fails for any reason (connection error, statement failure, etc.):

Unhandled rejection Error: connect ECONNREFUSED 127.0.0.1:3306
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)
    --------------------
    at Protocol._enqueue (/home/dev/mypackage/node_modules/mysql/lib/protocol/Protocol.js:144:48)
    at Protocol.handshake (/home/dev/mypackage/node_modules/mysql/lib/protocol/Protocol.js:51:23)
    at Connection.connect (/home/dev/mypackage/node_modules/mysql/lib/Connection.js:116:18)
    at /home/dev/mypackage/node_modules/knex/lib/dialects/mysql/index.js:69:18
    at Promise._execute (/home/dev/mypackage/node_modules/bluebird/js/release/debuggability.js:384:9)
    at Promise._resolveFromExecutor (/home/dev/mypackage/node_modules/bluebird/js/release/promise.js:518:18)
    at new Promise (/home/dev/mypackage/node_modules/bluebird/js/release/promise.js:103:10)
    at Client_MySQL.acquireRawConnection (/home/dev/mypackage/node_modules/knex/lib/dialects/mysql/index.js:64:12)
    at create (/home/dev/mypackage/node_modules/knex/lib/client.js:290:39)

Sure, the timeout does get re-registered via .finally(). However, it seems like having unhandled promise rejections is a bad idea in general, and Node.js always complains that it's going to cause process crashes in the future.

Errors need to be caught, and preferably handled via a user-defined function; that would allow one to inject a logger, for example.

[ivarconr]

Any update on this?

With the current way of doing this by not handling the promise-rejection ends up crashing the application if the database is unavailable for a short period of time. In many scenarios it is better to yield 500 and stop the application due to failing health checks.

[ivarconr]

Would you be open to a PR where the user can specify an option to not yeild an unresolved promise rejection when the db is unavailable?

[spaghettiguru]

We are also having occasional crashes because of this issue. Any updates on this?