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

Error not returned even though connection is closed #1109

Closed
aflynn93 opened this issue Nov 11, 2021 · 2 comments
Closed

Error not returned even though connection is closed #1109

aflynn93 opened this issue Nov 11, 2021 · 2 comments

Comments

@aflynn93
Copy link

pgx/conn.go

Line 571 in 6cd6c43

func (c *Conn) Query(ctx context.Context, sql string, args ...interface{}) (Rows, error) {

I've noticed if my DB connection gets closed, maybe after sitting idle for a day, no error is getting returned. Granted, I probably shouldn't leave the connection open for that long, but it'd be good to get an error returned that the connection is closed. What I'm seeing is that rows.err is getting returned, and this error is nil, but rows.resultReader.err is not getting returned, and this error says that the connection is closed. Should Query() return rows, rows.resultReader.err in the event the error is not nil?

@aflynn93
Copy link
Author

aflynn93 commented Nov 11, 2021

Also, I just realized that the ResultReader err is not getting exported - maybe rows.err should be getting set to the resultReader.err somewhere along the way?

@jackc
Copy link
Owner

jackc commented Nov 13, 2021

Not all errors can be detected when Query returns. You must read the results of the query and then check rows.Err() after that.

I suppose this particular case could be reported earlier, but given that correct usage of Query requires reading the query result and then checking rows.Err() (due to some errors only being detectable if the entire result is read such as a division by 0 on the last result row) I never worried much whether an error was exposed earlier from Query. Honestly, I wish Query didn't return an error at all to avoid this ambiguity but I have it that way to mimic the database/sql interface.

I don't have anything against reporting this error earlier though -- just never been something that personally concerned me.

I also updated the docs for Query to make more clear that rows must be read to detect all errors. (851091f)

@bfontaine bfontaine closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2022
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

No branches or pull requests

3 participants