Summary
RequeueNoAckRows can panic on a nil transaction and swallows all errors, so a failed requeue leaves rows stuck in processing with no signal. It is called from the newQueue constructor.
Root cause
queue.go:69-85:
func (q *Queue) RequeueNoAckRows() {
tx, err := q.client.Begin() // error unchecked
defer func() {
if err != nil {
tx.Rollback() // tx is nil if Begin failed -> panic
}
}()
_, err = tx.Exec(...) // err set on failure
err = tx.Commit() // overwrites Exec's err
}
Problems:
Begin() error is not checked; on failure tx == nil and the deferred tx.Rollback() nil-derefs (panic).
err from Exec is overwritten by Commit(), so an Exec failure is masked; Commit on a failed tx behavior is undefined here.
- The function returns nothing, so
newQueue (queue.go:39) cannot know requeue failed — orphaned processing rows are never recovered and callers get no error.
Impact
Medium. On startup, previously-crashed in-flight items are supposed to be requeued to pending. A failure here is silent (and can panic), leaving items permanently stuck.
Possible fix
Check every error, guard the rollback, and propagate:
func (q *Queue) RequeueNoAckRows() error {
tx, err := q.client.Begin()
if err != nil {
return err
}
if _, err = tx.Exec(
fmt.Sprintf("UPDATE %s SET status='pending', updated_at=? WHERE status='processing' AND ack=0",
quoteIdent(q.tableName)), time.Now().UTC(),
); err != nil {
tx.Rollback()
return err
}
return tx.Commit()
}
Then have newQueue propagate the error (or at least log it). A single-statement UPDATE arguably needs no explicit transaction at all — q.client.Exec(...) would be simpler and auto-committed.
Summary
RequeueNoAckRowscan panic on a nil transaction and swallows all errors, so a failed requeue leaves rows stuck inprocessingwith no signal. It is called from thenewQueueconstructor.Root cause
queue.go:69-85:Problems:
Begin()error is not checked; on failuretx == niland the deferredtx.Rollback()nil-derefs (panic).errfromExecis overwritten byCommit(), so an Exec failure is masked;Commiton a failed tx behavior is undefined here.newQueue(queue.go:39) cannot know requeue failed — orphanedprocessingrows are never recovered and callers get no error.Impact
Medium. On startup, previously-crashed in-flight items are supposed to be requeued to
pending. A failure here is silent (and can panic), leaving items permanently stuck.Possible fix
Check every error, guard the rollback, and propagate:
Then have
newQueuepropagate the error (or at least log it). A single-statementUPDATEarguably needs no explicit transaction at all —q.client.Exec(...)would be simpler and auto-committed.