fix: gracefully close pool connections

[dstankovd]

Fixes #3148

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.15%. Comparing base (de94ba9) to head (65fda1e). Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3180      +/-   ##
==========================================
+ Coverage   89.08%   89.15%   +0.07%     
==========================================
  Files          86       86              
  Lines       13531    13528       -3     
  Branches     1569     1570       +1     
==========================================
+ Hits        12054    12061       +7     
+ Misses       1477     1467      -10     

Flag	Coverage Δ
compression-0	89.15% <100.00%> (+0.07%)	:arrow_up:
compression-1	89.15% <100.00%> (+0.07%)	:arrow_up:
static-parser-0	86.73% <100.00%> (+0.07%)	:arrow_up:
static-parser-1	87.51% <100.00%> (+0.07%)	:arrow_up:
tls-0	88.57% <100.00%> (+0.07%)	:arrow_up:
tls-1	88.92% <100.00%> (+0.07%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

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

[wellwelwel]

Thanks, @dstankovd 🙋🏻‍♂️

Do you think a test can be added to cover these changes?

[dstankovd]

Thanks, @dstankovd 🙋🏻‍♂️

Do you think a test can be added to cover these changes?

Hey, I've added a new test covering theses changes :)

[wellwelwel]

@dstankovd, in #3081 we moved the lib/pool.js and lib/pool_connection.js to lib/base/pool.js and lib/base/pool_connection.js. This led to the need for a rebase.

Sorry for the inconvenience 🙋🏻‍♂️

Also, thanks for the tests 🤝

[dstankovd]

Hey @wellwelwel. I've rebased the branch, not sure why there are failed checks tho..

[wellwelwel]

@dstankovd, I don't know if it's a bug or if it's intentional, but it seems that Deno has completely removed support for local CommonJS in the canary version.

In any case, the failed workflows are unrelated to your changes 🙋🏻‍♂️

---

Edit: Not a bug, but a breaking change (already fixed in #3209).

[wellwelwel]

I've rebased the branch, not sure why there are failed checks tho..

@sidorares, could you enable the following option in the repository settings?

• This will display a button that allows us to automatically rebase outdated branches without conflict.

[dstankovd]

Hey @wellwelwel ,

rebased again, so all checks should pass now :)

[wellwelwel]

Hello, any plans on when will this fix be released?

Hi @dygabo, this PR seems to remove a depreciation:

• Original warning

https://github.com/sidorares/node-mysql2/blob/e455b6b23e7ce8dcd91cbb3fa39b616c8fb8479d/lib/base/pool_connection.js#L32-L42

---

For this reason, I prefer to keep the "merge" action to @sidorares 🙋🏻‍♂️

[Zikoel]

We currently have the problem solved by this PR... seems to me that all steps are gone well. I see that remains some conflicts (but I can't see them) any chances to go further with this? Is not clear to me if any other help is useful at this point.

[SavvasSunilBelakeri]

Hi @wellwelwel @sidorares : any plans on when will this fix be released?

[ZoRDoK]

We are also waiting for this to be added.