-
-
Notifications
You must be signed in to change notification settings - Fork 415
Description
Query hook receives non nil Result with nil value. This leads to a panic:
runtime error: invalid memory address or nil pointer dereference
Expected Behavior
Code like this don't trigger a nil pointer panic as it have a e.Result != nil
check.
func (h hook) AfterQuery(ctx context.Context, e *pg.QueryEvent) error {
if e.Err != nil {
h.log("error", e.Err)
}
if e.Result != nil {
h.log("returned", e.Result.RowsReturned())
h.log("affected", e.Result.RowsAffected())
}
return nil
}
Current Behavior
Part of panic stack trace
/usr/local/go/src/runtime/panic.go:1038 +0x215
github.com/go-pg/pg/v10.(*result).RowsReturned(0xdc4c80)
/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/result.go:52
<REDACTED>.AfterQuery({{0xdae1c0, 0xc00011e110}}, {0xdc4c80, 0xc000b5b7d0}, 0xc0004a1900)
<REDACTED>.go:110 +0xf2
github.com/go-pg/pg/v10.(*baseDB).afterQueryFromIndex(0xc0000af0e0, {0xdc4c80, 0xc000b5b7d0}, 0x0, 0xdd8890)
/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/hook.go:130 +0x75
github.com/go-pg/pg/v10.(*baseDB).afterQuery(0xc000146d00, {0xdc4c80, 0xc000b5b7d0}, 0xc0009c8005, {0xdbce20, 0x0}, {0xdae1c0, 0xc00011e110})
/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/hook.go:125 +0xaa
Possible Solution
Explicitly set nil for Result
when *result
is nil.
Related article https://codefibershq.com/blog/golang-why-nil-is-not-always-nil
Steps to Reproduce
- Add a query hook as provided above
- Execute SQL which fails
- nil pointer panic is triggered
Here is an example of how nil becomes not nil https://goplay.tools/snippet/DVV1XNz5icn
Context (Environment)
Detailed Description
On first line res
is a nil. At the last line it assigned a value from db.simpleQuery
Lines 236 to 246 in 51b8018
var res Result | |
var lastErr error | |
for attempt := 0; attempt <= db.opt.MaxRetries; attempt++ { | |
if attempt > 0 { | |
if err := internal.Sleep(ctx, db.retryBackoff(attempt-1)); err != nil { | |
return nil, err | |
} | |
} | |
lastErr = db.withConn(ctx, func(ctx context.Context, cn *pool.Conn) error { | |
res, err = db.simpleQuery(ctx, cn, wb) |
In db.simpleQuery
result might get returned with nil value of type *result
, but in db.simpleQuery
it will be wrapped into Result
and will pass != nil
check
Lines 544 to 553 in 51b8018
var res *result | |
if err := cn.WithReader(c, db.opt.ReadTimeout, func(rd *pool.ReaderContext) error { | |
var err error | |
res, err = readSimpleQuery(rd) | |
return err | |
}); err != nil { | |
return nil, err | |
} | |
return res, nil |
Note: Same issue exists in other parts of a code.
Possible Implementation
Workaround of this issue is to change a hook to look like this:
func (h hook) AfterQuery(ctx context.Context, e *pg.QueryEvent) error {
if e.Err != nil {
h.log("error", e.Err)
} else if e.Result != nil {
h.log("returned", e.Result.RowsReturned())
h.log("affected", e.Result.RowsAffected())
}
return nil
}