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

rows.Err must be checked using rowserrcheck #945

Closed
1 of 3 tasks
yabetsu93 opened this issue Jan 29, 2020 · 3 comments
Closed
1 of 3 tasks

rows.Err must be checked using rowserrcheck #945

yabetsu93 opened this issue Jan 29, 2020 · 3 comments

Comments

@yabetsu93
Copy link

Thank you for creating the issue!

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

I upgrade my golangci-lint to 1.23.1 to check unclosed sql connection
for example i have this query:

      if err != nil {
          // must passed the linter check
      }

but with this current version it keeps on display the error message
rows.Err must be checked (rowserrcheck) despite of existing condition
on the code can someone validate this?
i found this one golang/go#24329 but it doesnt satisfy
me hoping to hear your reply

Version of golangci-lint
$ golangci-lint --version
# paste output here
Config file
$ cat .golangci.yml
# paste output here
Go environment
$ go version && go env
# paste output here
Verbose output of running
$ golangci-lint run -v
# paste output here
@okhowang
Copy link

I found a style for pass rowserrcheck

rows, err := db.Query("some sql")
if err != nil {
// error handling
}
defer func() {
    _ = rows.Close()
    _ = rows.Err() // or modify return value
}
for rows.Next() {
  // normal handling
}

@kashifkhan0771
Copy link

kashifkhan0771 commented Jan 21, 2021

I found a style for pass rowserrcheck

rows, err := db.Query("some sql")
if err != nil {
// error handling
}
defer func() {
    _ = rows.Close()
    _ = rows.Err() // or modify return value
}
for rows.Next() {
  // normal handling
}
defer func() {
    _ = rows.Close()
    _ = rows.Err() // or modify return value
}()

should be like this

avtalabirchuk pushed a commit to avtalabirchuk/otus-golang that referenced this issue Oct 9, 2021
@panter-dsd
Copy link

Guys, you fixed the linter error but didn't figure out the real problem in your code.
In this code

rows, err := db.Query(....)
if err != nil {
  return nil, err
}

for rows.Next() {
  // scan row
}

return result, nil

you have a big problem - rows.Next() returns false either when you've reached the end of data or you've gotten an read error. The second case you do not check and this leads you to the fact that only part of the data is returned.
You have to change the code to check Err

rows, err := db.Query(....)
if err != nil {
  return nil, err
}

for rows.Next() {
  // scan row
}

if err := rows.Err(); err != nil {
  return nil, err
}

return result, nil

serprex added a commit to PeerDB-io/peerdb that referenced this issue Jan 26, 2024
It turns out `rows.Next()` can return false on error,
avoiding error detection with `rows.Scan`

See golangci/golangci-lint#945

Fix that. In particular, `QueryRowContext` with checking for `sql.ErrNowRows`
is much simpler when expecting 0 or 1 results

Mark two false positives when only wanting schema of `LIMIT 0` query
serprex added a commit to PeerDB-io/peerdb that referenced this issue Jan 26, 2024
It turns out `rows.Next()` can return false on error,
avoiding error detection with `rows.Scan`

See golangci/golangci-lint#945

Fix that. In particular, `QueryRowContext` with checking for `sql.ErrNowRows`
is much simpler when expecting 0 or 1 results

Mark two false positives when only wanting schema of `LIMIT 0` query
serprex added a commit to PeerDB-io/peerdb that referenced this issue Jan 26, 2024
It turns out `rows.Next()` can return false on error,
avoiding error detection with `rows.Scan`

See golangci/golangci-lint#945

Fix that. In particular, `QueryRowContext` with checking for
`sql.ErrNowRows` is much simpler when expecting 0 or 1 results

Mark two false positives when only wanting schema of `LIMIT 0` query
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

4 participants