rawlinp commented on a change in pull request #4700:
URL: https://github.com/apache/trafficcontrol/pull/4700#discussion_r434778334
##########
File path: traffic_ops/traffic_ops_golang/server/servers.go
##########
@@ -1202,8 +1202,15 @@ func Delete(w http.ResponseWriter, r *http.Request) {
return
}
- if err := tx.QueryRow(deleteServerQuery, id).Scan(); err != nil {
- userErr, sysErr, errCode = api.ParseDBError(err)
+ if _, err := tx.Exec(deleteServerQuery, id); err != nil {
+ if err == sql.ErrNoRows {
Review comment:
```
// ErrNoRows is returned by Scan when QueryRow doesn't return a
// row. In such a case, QueryRow returns a placeholder *Row value that
// defers this error until a Scan.
var ErrNoRows = errors.New("sql: no rows in result set")
```
Digging into this a bit more, it seems like `Exec` won't actually return
that error if no rows were affected, so it seems like the real fix is to check
the result similar this example (though not exactly since it should probably
differentiate between "too many rows affected" and "no rows affected"):
```
result, err := origin.ReqInfo.Tx.NamedExec(deleteQuery(), origin)
if err != nil {
return nil, errors.New("origin delete: query: " + err.Error()),
http.StatusInternalServerError
}
rowsAffected, err := result.RowsAffected()
if err != nil {
return nil, errors.New("origin delete: getting rows affected: "
+ err.Error()), http.StatusInternalServerError
}
if rowsAffected != 1 {
return nil, errors.New("origin delete: multiple rows
affected"), http.StatusInternalServerError
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
