THRIFT-5715 Python Exceptions mutable with slots
In Python 3.11 exceptions generated by the compiler can't be used with a
context manager because they are immutable. As of Python 3.11
`contextlib.contextmanager` sets `exc.__traceback__` in the event that
the code in the context manager errors.
As of Thrift v0.18.1 exceptions are generated as immutable by default.
See [PR#1835](https://github.com/apache/thrift/pull/1835) for more
information about why exceptions were made immutable by default.
This change makes all non-Thrift fields mutable when slots is used
without dynamic. This will allow exceptions to be re-raised properly by
the contextmanager in Python 3.11.
diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc b/compiler/cpp/src/thrift/generate/t_py_generator.cc
index 8dd915e..ffabd56 100644
--- a/compiler/cpp/src/thrift/generate/t_py_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc
@@ -893,12 +893,42 @@
if (is_immutable(tstruct)) {
out << endl;
- out << indent() << "def __setattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify immutable instance\")" << endl
- << endl;
- out << indent() << "def __delattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify immutable instance\")" << endl
- << endl;
+ out << indent() << "def __setattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard Library can edit
+ // internal fields of std library base classes. For example, in Python 3.11 ContextManager
+ // edits the `__traceback__` field on Exceptions. Allowing this to work with `__slots__` is
+ // trivial because we know which fields are user-provided, without slots we need to build a
+ // way to know which fields are user-provided.
+ if (gen_slots_ && !gen_dynamic_) {
+ out << indent() << "if args[0] not in self.__slots__:" << endl;
+ indent_up();
+ out << indent() << "super().__setattr__(*args)" << endl
+ << indent() << "return" << endl;
+ indent_down();
+ }
+ out << indent() << "raise TypeError(\"can't modify immutable instance\")" << endl;
+ indent_down();
+ out << endl;
+ out << indent() << "def __delattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard Library can edit
+ // internal fields of std library base classes. For example, in Python 3.11 ContextManager
+ // edits the `__traceback__` field on Exceptions. Allowing this to work with `__slots__` is
+ // trivial because we know which fields are user-provided, without slots we need to build a
+ // way to know which fields are user-provided.
+ if (gen_slots_ && !gen_dynamic_) {
+ out << indent() << "if args[0] not in self.__slots__:" << endl;
+ indent_up();
+ out << indent() << "super().__delattr__(*args)" << endl
+ << indent() << "return" << endl;
+ indent_down();
+ }
+ out << indent() << "raise TypeError(\"can't modify immutable instance\")" << endl;
+ indent_down();
+ out << endl;
// Hash all of the members in order, and also hash in the class
// to avoid collisions for stuff like single-field structures.
diff --git a/test/py/TestClient.py b/test/py/TestClient.py
index 8a30c3a..61a9c60 100755
--- a/test/py/TestClient.py
+++ b/test/py/TestClient.py
@@ -256,6 +256,35 @@
y = self.client.testMultiException('success', 'foobar')
self.assertEqual(y.string_thing, 'foobar')
+ def testException__traceback__(self):
+ print('testException__traceback__')
+ self.client.testException('Safe')
+ expect_slots = uses_slots = False
+ expect_dynamic = uses_dynamic = False
+ try:
+ self.client.testException('Xception')
+ self.fail("should have gotten exception")
+ except Xception as x:
+ uses_slots = hasattr(x, '__slots__')
+ uses_dynamic = (not isinstance(x, TException))
+ # We set expected values here so that we get clean tracebackes when
+ # the assertions fail.
+ try:
+ x.__traceback__ = x.__traceback__
+ # If `__traceback__` was set without errors than we expect that
+ # the slots option was used and that dynamic classes were not.
+ expect_slots = True
+ expect_dynamic = False
+ except Exception as e:
+ self.assertTrue(isinstance(e, TypeError))
+ # There are no other meaningful tests we can preform because we
+ # are unable to determine the desired state of either `__slots__`
+ # or `dynamic`.
+ return
+
+ self.assertEqual(expect_slots, uses_slots)
+ self.assertEqual(expect_dynamic, uses_dynamic)
+
def testOneway(self):
print('testOneway')
start = time.time()