Skip to content

FIX: exception pickle/unpickle round-trip#616

Merged
jahnvi480 merged 3 commits into
mainfrom
jahnvi/fix-exception-pickle-587
Jun 5, 2026
Merged

FIX: exception pickle/unpickle round-trip#616
jahnvi480 merged 3 commits into
mainfrom
jahnvi/fix-exception-pickle-587

Conversation

@jahnvi480

@jahnvi480 jahnvi480 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#45394

GitHub Issue: #587


Summary

This pull request improves the serialization support for custom exception classes in the codebase, specifically ensuring that all DB-API exception subclasses can be pickled and unpickled (serialized and deserialized) correctly. It does so by implementing the __reduce__ method for relevant exception classes and by adding comprehensive tests to verify this behavior.

Serialization support for exceptions:

  • Added a __reduce__ method to ConnectionStringParseError and all DB-API exception subclasses to enable correct pickling and unpickling, preserving all relevant attributes

Testing improvements:

  • Added new tests in tests/test_006_exceptions.py to verify that all DB-API exception subclasses and ConnectionStringParseError survive a pickle/unpickle round-trip, including cases with empty attributes and deep copies.

Add __reduce__ to the base Exception class and ConnectionStringParseError
so that pickle.loads(pickle.dumps(exc)) reconstructs the exception with
the correct constructor arguments.

Previously, super().__init__(self.message) stored only one combined string
in self.args, but __init__ requires two positional arguments (driver_error,
ddbc_error). This caused TypeError on unpickle, breaking multiprocessing,
concurrent.futures, celery, and copy.deepcopy.

Fixes #587
Copilot AI review requested due to automatic review settings June 2, 2026 06:04
@github-actions github-actions Bot added the pr-size: small Minimal code update label Jun 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves exception serialization in mssql_python so driver-defined DB-API exceptions (and ConnectionStringParseError) can be pickled/unpickled reliably—important for multiprocessing and cross-process error propagation.

Changes:

  • Added __reduce__ implementations for ConnectionStringParseError and the DB-API exception base class to support pickle/unpickle round-trips.
  • Added tests verifying pickle round-trip behavior (and copy.deepcopy) for DB-API exception subclasses and ConnectionStringParseError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
mssql_python/exceptions.py Adds __reduce__ hooks to make custom exceptions pickle/unpickle friendly.
tests/test_006_exceptions.py Adds coverage ensuring exceptions survive pickle/unpickle and deepcopy round-trips.

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

Comment thread mssql_python/exceptions.py
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

80%


📈 Total Lines Covered: 6667 out of 8268
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/exceptions.py (100%)

Summary

  • Total: 12 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 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.7%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.1%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.pybind.connection.connection.cpp: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@bewithgaurav bewithgaurav left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jahnvi480 jahnvi480 merged commit 4f12cee into main Jun 5, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: small Minimal code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants