Skip to content

Conversation

jfrconley
Copy link

@jfrconley jfrconley commented Jun 13, 2022

Currently, errors in transactions do not get propagated to the pool. This means failed connections will get returned to the pool, effectively poisoning the well. This change simply passes exceptions back to the pool when they occur, allowing the connection to be dropped.

@jawj

@john-trashlab
Copy link

@jfrconley I'm curious what the impact of this bug/fix is. Can you elaborate?

@jawj
Copy link
Owner

jawj commented Jan 23, 2023

I'm sorry to see I never responded to this. @jfrconley Is there any chance you could put together a simple test case that demonstrates the problem and how your patch solves it?

@olepbr
Copy link

olepbr commented May 21, 2025

AFAICT, there are no functional changes in this PR; all it does is add an catch to the outer try, which does nothing except replicate the logic already present in the finally block, and also explicitly rethrows the error. This is pointless, as a) the finally block will always run, hence releasing the client, and b) omitting the catch block (as the code already does) causes the error to propagate to the caller of transaction(), hence there's no need to rethrow. As such, I believe this PR should be closed without merging.

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.

4 participants