sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.07k stars 619 forks source link

read ECONNRESET after upgrading #2599

Closed niels-van-den-broeck closed 5 months ago

niels-van-den-broeck commented 6 months ago

After introduction of https://github.com/sidorares/node-mysql2/pull/2043, our application seems to be getting the following error (seemingly at random):

"stack": "Error: read ECONNRESET\n    at TCP.onStreamRead (node:internal/stream_base_commons:217:20)\n    at runInContextCb (/usr/src/app/node_modules/newrelic/lib/shim/shim.js:1168:22)\n    at AsyncLocalStorage.run (node:async_hooks:335:14)\n    at AsyncLocalContextManager.runInContext (/usr/src/app/node_modules/newrelic/lib/context-manager/async-local-context-manager.js:65:36)\n    at Shim.applySegment (/usr/src/app/node_modules/newrelic/lib/shim/shim.js:1158:25)\n    at TCP.wrapper (/usr/src/app/node_modules/newrelic/lib/shim/shim.js:1760:17)\n    at TCP.callbackTrampoline (node:internal/async_hooks:130:17)\n    at MysqlConnection.executeQuery (/usr/src/app/node_modules/kysely/dist/cjs/dialect/mysql/mysql-driver.js:119:69)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async /usr/src/app/node_modules/kysely/dist/cjs/query-executor/query-executor-base.js:37:28\n    at async DefaultConnectionProvider.provideConnection (/usr/src/app/node_modules/kysely/dist/cjs/driver/default-connection-provider.js:12:20)\n    at async DefaultQueryExecutor.executeQuery (/usr/src/app/node_modules/kysely/dist/cjs/query-executor/query-executor-base.js:36:16)\n    at async SelectQueryBuilderImpl.execute (/usr/src/app/node_modules/kysely/dist/cjs/query-builder/select-query-builder.js:311:24)",

We use the mysql dialect for kysely which receives a pool created in this library. the pool is created as follows:

import { createPool } from 'mysql2';

const pool = createPool({
    host, // host ip string
    user, // user string
    database, // db string
    password, // password string
    port, // port
    timezone: 'Z',
  });

All versions before 3.3.5 work just fine. Right now this happens relatively often, but we can't seem to find a pattern of when/how it triggers.

Any potential help / input would be welcome.

sidorares commented 6 months ago

does it help if you pass enableKeepAlive: false to connection config?

niels-van-den-broeck commented 6 months ago

does it help if you pass enableKeepAlive: false to connection config?

Unfortunately the error still occurs after adding it to the config.

niels-van-den-broeck commented 6 months ago

To give some additional context, we are running on Node 20.11.1.

niels-van-den-broeck commented 6 months ago

This is blocking us from updating to a version which no longer has security vulnerabilities. Is there any way we can look into what's causing this issue?

sidorares commented 6 months ago

Are you confident it's https://github.com/sidorares/node-mysql2/pull/2043 change that is causing the issue? Can you try to revert that change manually after installing latest version?

niels-van-den-broeck commented 6 months ago

After patching manually unfortunately that seemed to not be the issue. We've decided to roll down our versions 1 by 1, and the last one that seems to not have the issue was 3.3.1

So after updating to 3.3.2 we started seeing the issue again. Which when looking at release notes only introduced this

asijoumi commented 6 months ago

Hello!

I recently discussed issue #10872: QueryFailedError: read ECONNRESET with @niels-van-den-broeck. It appears that changes between the last version and version 3.3.1 have introduced an ECONNRESET error.

I haven't pinpointed the exact cause yet, but I conducted tests on both AWS RDS and an on-premise server. Interestingly, the error does not occur on AWS RDS, but it does manifest in the on-premise environment.

Could you help us understand what might be causing this discrepancy?

Thank you!

niels-van-den-broeck commented 5 months ago

@sidorares It seems like setting the default forkeepAliveInitialDelay to 0 is what is causing this issue.

The node documentation is also kind of vague about this default. (note the or previous part of the scentence).

Would it be ok if I'd create a PR to remove this default on the mysql2 side again? As of right now it's not possible to pass in undefined since it's overwritten.

benyaminl commented 5 months ago

Hello!

I recently discussed issue #10872: QueryFailedError: read ECONNRESET with @niels-van-den-broeck. It appears that changes between the last version and version 3.3.1 have introduced an ECONNRESET error.

I haven't pinpointed the exact cause yet, but I conducted tests on both AWS RDS and an on-premise server. Interestingly, the error does not occur on AWS RDS, but it does manifest in the on-premise environment.

Could you help us understand what might be causing this discrepancy?

Thank you!

Sorry for jumpin, we also face this problem on our on prem server (typeORM on nestjs), and we check that mysql2 thinks that the connection still alive even it's broken, so we resort with setting maxIdle: 0 on the typeORM layer to mysql2 config, so any open connection after used is closed. This make no error after that. I do know that it make a performance hit to the application as a roundtrip to re-establish connection to the DB takes time, but well.. that's what we can only think for now as we rely on the framework.

niels-van-den-broeck commented 5 months ago

Any feedback on this would be welcome

sidorares commented 5 months ago

Would it be ok if I'd create a PR to remove this default on the mysql2 side again? As of right now it's not possible to pass in undefined since it's overwritten.

Sorry, missed that message @niels-van-den-broeck . If thats the case this is probably a bug, feel fee to open a PR