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

Bug fix: Mark connection as not good when error on cancellation confirmation #773

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zcolleen
Copy link

@zcolleen zcolleen commented Nov 9, 2022

This pr fixes bug when we were not able to get cancelation confirmation but connection is not marked as bad and returns to pool, so next time this connection is taken from pool, we get the same error instead of opening new good connection

Example:

r, err := db.QueryContext(ctx, query) <- context expired, then we got cancelation confirmation error

...

r, err := db.QueryContext(ctx, query) <- we get same error because we are still using bad connection instead of opening new one

@codecov
Copy link

codecov bot commented Nov 9, 2022

Codecov Report

Merging #773 (2fc4276) into master (ed0f620) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #773      +/-   ##
==========================================
+ Coverage   71.35%   71.38%   +0.02%     
==========================================
  Files          24       24              
  Lines        5405     5407       +2     
==========================================
+ Hits         3857     3860       +3     
+ Misses       1303     1302       -1     
  Partials      245      245              
Impacted Files Coverage Δ
mssql.go 87.78% <100.00%> (+0.03%) ⬆️
token.go 64.35% <100.00%> (+0.14%) ⬆️
tds.go 65.85% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zcolleen zcolleen force-pushed the master branch 3 times, most recently from a0bfc23 to be6a6fc Compare November 9, 2022 13:18
Eygin Semen Leonidovich added 2 commits November 9, 2022 18:04
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 this pull request may close these issues.

1 participant