Skip to content

Commit

Permalink
THRIFT-5715 Python Exceptions mutable with slots
Browse files Browse the repository at this point in the history
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](#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.
  • Loading branch information
KTAtkinson committed Jul 7, 2023
1 parent 284e6b3 commit d3d8fd7
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 6 deletions.
42 changes: 36 additions & 6 deletions compiler/cpp/src/thrift/generate/t_py_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -893,12 +893,42 @@ void t_py_generator::generate_py_struct_definition(ostream& out,

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.
Expand Down
29 changes: 29 additions & 0 deletions test/py/TestClient.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,35 @@ def testMultiException(self):
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()
Expand Down

0 comments on commit d3d8fd7

Please sign in to comment.