FIX: Context manager commits on clean exit, rolls back on exception

[bewithgaurav]

Work Item / Issue Reference

AB#45829

GitHub Issue: #635

---

Summary

Fixes the Connection context manager (__exit__) to implement commit-on-success / rollback-on-exception semantics, matching the documented behavior in the wiki, the DevBlogs post, and the docstrings.

Previously __exit__ only called close(), which always rolled back uncommitted changes regardless of whether the block exited cleanly or via exception. Now:

• Clean exit + autocommit=False: commits the transaction
• Exception + autocommit=False: rolls back the transaction
• autocommit=True: no explicit commit/rollback (same as before)
• Connection always closed on exit

close() is unchanged. Its internal rollback becomes a harmless no-op on an already-resolved transaction.

24 subprocess-isolated tests cover: commit/rollback semantics, autocommit toggling, manual commit/rollback inside block, DDL+DML, pending cursor results, doomed transactions (XACT_ABORT), KeyboardInterrupt, generator abandonment, killed SPID, 10k-row transactions, double-exit idempotency, and GC safety. Resource-intensive tests (rapid connection cycling, 10k inserts) are marked @pytest.mark.stress.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 36%

🎯 Overall Coverage 80%

📈 Total Lines Covered: 6743 out of 8343 📁 Project: mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/connection.py (36.8%): Missing lines 1723,1729-1733,1739-1741,1744-1746

Summary

• Total: 19 lines
• Missing: 12 lines
• Coverage: 36%

mssql_python/connection.py

Lines 1719-1727

  1719         (rollback or close) are suppressed so the original user exception
  1720         propagates unchanged.
  1721         """
  1722         if self._closed:
! 1723             return
  1724         try:
  1725             if not self.autocommit:
  1726                 if exc_type is None:
  1727                     self.commit()

---

Lines 1725-1737

  1725             if not self.autocommit:
  1726                 if exc_type is None:
  1727                     self.commit()
  1728                 else:
! 1729                     self.rollback()
! 1730         except Exception:
! 1731             try:
! 1732                 self.close()
! 1733             except Exception:
  1734                 logger.warning(
  1735                     "Failed to close connection after failed "
  1736                     "commit/rollback in context manager.",
  1737                     exc_info=True,

---

Lines 1735-1750

  1735                     "Failed to close connection after failed "
  1736                     "commit/rollback in context manager.",
  1737                     exc_info=True,
  1738                 )
! 1739             if exc_type is None:
! 1740                 raise
! 1741             return
  1742         try:
  1743             self.close()
! 1744         except Exception:
! 1745             if exc_type is None:
! 1746                 raise
  1747             logger.warning(
  1748                 "Failed to close connection in context manager.",
  1749                 exc_info=True,
  1750             )

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.6%
mssql_python.logging.py: 85.5%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Copilot]

Pull request overview

This pull request updates the Connection context manager behavior to match documented transaction semantics: commit on clean exit and rollback on exception when autocommit=False, while always closing the connection on exit.

Changes:

• Implemented commit-on-success / rollback-on-exception logic in Connection.__exit__.
• Added a new subprocess-isolated test suite to validate context manager transaction behavior and crash-safety edge cases.
• Marked resource-intensive context-manager tests as @pytest.mark.stress (excluded by default via pytest.ini).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
mssql_python/connection.py	Updates Connection.__exit__ to commit/rollback based on exit condition and autocommit, then close the connection.
tests/test_024_context_manager_transaction.py	Adds subprocess-based tests covering commit/rollback semantics, exception propagation, and various hardening scenarios.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sumitmsft]

Approved with minor comments