add automatic connection release with Symbol.asyncDispose after execu…

[luiz-rissardi]

ADD: automatic connection release with Symbol.asyncDispose after execution

[codecov]

Codecov Report

Attention: 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 39 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 🤝