Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read ECONNRESET after upgrading #2599

Closed
niels-van-den-broeck opened this issue Apr 17, 2024 · 11 comments · Fixed by #2712
Closed

read ECONNRESET after upgrading #2599

niels-van-den-broeck opened this issue Apr 17, 2024 · 11 comments · Fixed by #2712

Comments

@niels-van-den-broeck
Copy link
Contributor

After introduction of #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
Copy link
Owner

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

@niels-van-den-broeck
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@niels-van-den-broeck
Copy link
Contributor Author

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
Copy link
Owner

Are you confident it's #2043 change that is causing the issue? Can you try to revert that change manually after installing latest version?

@niels-van-den-broeck
Copy link
Contributor Author

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
Copy link

asijoumi commented May 8, 2024

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
Copy link
Contributor Author

@sidorares It seems like setting the default for keepAliveInitialDelay 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
Copy link

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
Copy link
Contributor Author

Any feedback on this would be welcome

@sidorares
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants