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.

[bewithgaurav]

why *args → exc_type, exc_val, exc_tb:

the old __exit__ didn't inspect its arguments at all (just called close()), so *args was fine. now we branch on exc_type to decide commit vs rollback, so we use the canonical __exit__(self, exc_type, exc_val, exc_tb) signature from the Python data model.

this is what sqlite3, psycopg2, and every other DB-API driver uses. no behavioral change from the rename itself - just makes the branching logic readable.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 35%

🎯 Overall Coverage 80%

📈 Total Lines Covered: 6749 out of 8352 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/connection.py (35.0%): Missing lines 1723,1729-1737,1740-1742

Summary

• Total: 20 lines
• Missing: 13 lines
• Coverage: 35%

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-1746

  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                 pass
! 1735             if exc_type is None:
! 1736                 raise
! 1737             return
  1738         try:
  1739             self.close()
! 1740         except Exception:
! 1741             if exc_type is None:
! 1742                 raise
  1743 
  1744     def __del__(self) -> None:
  1745         """
  1746         Destructor to ensure the connection is closed when the connection object

---

---

📋 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.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.4%
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.