add automatic connection release with Symbol.asyncDispose after execu…

[luiz-rissardi]

ADD: automatic connection release with Symbol.asyncDispose after execution

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.06%. Comparing base (108fa5d) to head (3aca520).
⚠️ Report is 347 commits behind head on master.

Files with missing lines	Patch %	Lines
lib/commands/query.js	33.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3590      +/-   ##
==========================================
- Coverage   89.08%   89.06%   -0.03%     
==========================================
  Files          86       86              
  Lines       13538    13544       +6     
  Branches     1569     1569              
==========================================
+ Hits        12061    12063       +2     
- Misses       1477     1481       +4     

Flag	Coverage Δ
compression-0	89.06% <33.33%> (-0.03%)	⬇️
compression-1	89.06% <33.33%> (-0.03%)	⬇️
static-parser-0	86.64% <33.33%> (-0.03%)	⬇️
static-parser-1	87.42% <33.33%> (-0.03%)	⬇️
tls-0	88.48% <33.33%> (-0.03%)	⬇️
tls-1	88.83% <33.33%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[luiz-rissardi]

What could I do? I didn't understand why the errors were occurring, where could I put this change to release a connection after a query?

[wellwelwel]

What could I do? I didn't understand why the errors were occurring, where could I put this change to release a connection after a query?

Hi, @luiz-rissardi! Firstly, thank you for your contribution 🙋🏻‍♂️

Regarding the error returned, it's due to the lint check (Prettier). You can fix it running the command npm run lint:fix.

About the changes, the release method usually comes from pool connections, for example:

import mysql from 'mysql2/promise';

const pool = mysql.createPool('mysql://root:password@localhost:3306/test');
const connection = await pool.getConnection();

// ... some queries

connection.release();

You can check out the detailed documentation and examples at docs/examples/connections/create-pool and docs/examples/connections/createPoolCluster.

---

I'm attempting to visualize the scenarios. Is this feature related to the new using keyword in JavaScript? If so, I think we can create a test for the runtime versions that support this feature, including examples with use cases. I can help with this part if needed.

In this case, I believe it makes more sense to adopt this approach to close connections. What do you think?

[luiz-rissardi]

Primeiramente, obrigado a me ajudar na comunidade open source, essa é minha primeira vez tentando agir diretamente em uma biblioteca, a minha sugestão para as próximas versões seria implementar a liberação das conexões de createPool e createConnection automaticamente, sem a necessidade do .end() ou o .release().

[wellwelwel]

Primeiramente, obrigado a me ajudar na comunidade open source, essa é minha primeira vez tentando agir diretamente em uma biblioteca, a minha sugestão para as próximas versões seria implementar a liberação das conexões de createPool e createConnection automaticamente, sem a necessidade do .end() ou o .release().

You're welcome, @luiz-rissardi! I can also talk in Portuguese, but I'll continue talking in English here. Feel free to contact me directly if you prefer in Portuguese only.

As I haven't tested the possible conflicts, I'd like to understand if asyncDispose offers some breaking change for those who use .end/release() (e.g., closing/releasing connections twice). Or if it's only triggered in combination with await using (in this case, offering no conflict or breaking change).

[luiz-rissardi]

@wellwelwel teria que ver se ele realmente está afetando os testes, eu não encontrei o motivo de tantos testes quebrarem

[luiz-rissardi]

Alguém conseguiu ver o porque dos testes terem dado errado? não tive muito tempo pra ver essa feature no projeto, de implementar o uso das novas "using" do javascript

[wellwelwel]

Alguém conseguiu ver o porque dos testes terem dado errado? não tive muito tempo pra ver essa feature no projeto, de implementar o uso das novas "using" do javascript

@luiz-rissardi, you can check the solution for the failed workflow in #3590 (comment) and about implementation (also for tests implementation based in Node.js version) in #3590 (comment). Feel free to ask if you have any questions 🙋🏻‍♂️

[wellwelwel]

🇺🇸 Hey @luiz-rissardi, if you can't follow the conversation in English, feel free to call me on LinkedIn and chat in Portuguese 🤝

🇧🇷 Fala @luiz-rissardi, se você não puder seguir a conversa em inglês, fique à vontade para me chamar no LinkedIn e conversar em português 🤝

[sakurai-ryo]

Hi @luiz-rissardi @wellwelwel ,

Is there any update on this PR?
Now that Node.js v24 has reached LTS (making Symbol.dispose/asyncDispose stable APIs), I believe this PR is valuable.

If you'd like, I can take this over and open a new PR.

[wellwelwel]

Is there any update on this PR?

@sakurai-ryo, in this PR, the implementation is in the wrong place (.query), as it forces a release after every query instead of only when the .getConnection or pool scope ends. It also doesn't consider the synchronous API and doesn't implement disposal for Connection, PoolConnection, and PoolCluster, including docs, typings and tests.

• Please, check feat: add Symbol.dispose and Symbol.asyncDispose support for Connections, Pools, and Pool Clusters #4112 🙋🏻‍♂️

[sakurai-ryo]

@wellwelwel
Thanks for creating the new PR!
I'm looking forward to seeing it merged.

[wellwelwel]

Closing in favor of #4112.