Skip to content

Return useful error for busy connections#1787

Open
ZYZ666-RGB wants to merge 2 commits into
go-sql-driver:masterfrom
ZYZ666-RGB:fix/issue-526-busy-connection-error
Open

Return useful error for busy connections#1787
ZYZ666-RGB wants to merge 2 commits into
go-sql-driver:masterfrom
ZYZ666-RGB:fix/issue-526-busy-connection-error

Conversation

@ZYZ666-RGB

@ZYZ666-RGB ZYZ666-RGB commented Jul 21, 2026

Copy link
Copy Markdown

Description

When a previous result set still owns the connection buffer, Prepare logs busy buffer and converts the error to driver.ErrBadConn. This hides the actual caller mistake.

This change:

  • returns a wrapped ErrBusyBuffer from Prepare instead of driver.ErrBadConn
  • adds cause-neutral context indicating that unread data remains on the connection
  • adds regression tests covering the error and connection reuse
  • adds the contributor to AUTHORS

Fixes #526.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change
  • All relevant tests pass
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

Validation

  • go test ./... -run 'TestPrepareBusyBuffer|TestPrepareWithOpenRows' -count=1
  • go vet ./...

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

mysqlConn.Prepare now preserves ErrBusyBuffer with an unread-data message. Tests cover buffered connections and transactions with open rows. The AUTHORS list gains one individual author entry.

Changes

Busy buffer prepare handling

Layer / File(s) Summary
ErrBusyBuffer Prepare handling
connection.go
mysqlConn.Prepare returns a wrapped ErrBusyBuffer when unread connection data prevents command writing.
Busy buffer behavior tests
connection_test.go, driver_test.go
Tests verify direct buffered preparation failure and successful preparation after transaction rows are closed.

Author list update

Layer / File(s) Summary
Individual author entry
AUTHORS
Adds ZYZ666-RGB <1991039819 at qq.com> before the Organizations section.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: shogo82148

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #526 by returning a useful busy-connection error from Prepare and adding regression coverage.
Out of Scope Changes check ✅ Passed No unrelated code changes are evident; the AUTHORS edit and tests align with the stated PR goals.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: returning a useful error for busy connections.
Description check ✅ Passed The description matches the changeset by explaining the busy-buffer error behavior, tests, and AUTHORS update.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@methane

methane commented Jul 21, 2026

Copy link
Copy Markdown
Member

How did you verify that there are no cases where ErrBusyBuffer is returned for other reasons?

@ZYZ666-RGB

Copy link
Copy Markdown
Author

You're right — I had not verified that assumption globally.

I traced all ErrBusyBuffer producers. It is returned by takeBuffer, takeSmallBuffer, and takeCompleteBuffer whenever the shared read buffer is non-empty, and those helpers are also used by authentication and other write paths. Therefore, the previous global error message was too specific.

I pushed 4cbd816 to restore the original ErrBusyBuffer message and add cause-neutral context only in Prepare:

busy buffer: unread data remains on the connection

The Prepare path is writeCommandPacketStr -> takeBuffer, so the added context reflects the verified buffer invariant without assuming that an open result set is the only possible cause. errors.Is(err, ErrBusyBuffer) is still preserved.

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

Successfully merging this pull request may close these issues.

Return useful error when attempting to reuse a blocked connection

2 participants