Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3f8f67c. Configure here.
| return ch.CheckNamedValue(nv) | ||
| } | ||
| return driver.ErrSkip | ||
| } |
There was a problem hiding this comment.
Stmt wrapper bypasses conn-level NamedValueChecker
Medium Severity
sentryStmt unconditionally implements driver.NamedValueChecker. The Go stdlib's driverArgsConnLocked checks the stmt for this interface first, then falls back to the conn. Because sentryStmt always claims to implement it, the conn-level NamedValueChecker is never consulted for prepared statements. When the underlying stmt lacks NamedValueChecker but the underlying conn has one (common in drivers like lib/pq for custom types like pq.Array), the wrapper returns ErrSkip and the stdlib falls through to DefaultParameterConverter instead of using the conn's checker — breaking custom type handling.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3f8f67c. Configure here.
| case <-ctx.Done(): | ||
| return nil, errors.Join(ctx.Err(), tx.Rollback()) | ||
| } | ||
| return tx, nil |
There was a problem hiding this comment.
BeginTx legacy fallback checks context after Begin
Medium Severity
In the legacy BeginTx fallback path, c.Begin() is called before checking ctx.Done(). The Go stdlib's ctxDriverBegin checks context cancellation before calling ci.Begin(). This means the wrapper starts a transaction on the database even when the context is already cancelled, then immediately rolls it back — causing unnecessary database round-trips and a different error shape (errors.Join of context error and potential rollback error).
Reviewed by Cursor Bugbot for commit 3f8f67c. Configure here.
| select { | ||
| default: | ||
| case <-ctx.Done(): | ||
| return nil, ctx.Err() | ||
| } |
There was a problem hiding this comment.
Honestly this is useless, it just guards whenever the cancel is called before we call the ex.Exec(). If it's already passed, then it won't be canceled at all.
2 participants
Description
This PR adds the base passthrough wrappers for
database/sql/driverand the public API . Instrumentation will be added in a follow-up PR.The core design is for
sentryConnto always expose context-aware interfaces. Then each method delegates to the underlying driver implementation. For legacy drivers the wrapper falls back to the non-context methods. This way we can correctly instrument the legacy drivers as well with the correct context.Issues
Changelog Entry Instructions
To add a custom changelog entry, uncomment the section above. Supports:
For more details: custom changelog entries
Reminders
feat:,fix:,ref:,meta:)