Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-44471: Change error type for bad objects in ExitStack.enter_context() #26820

Merged
merged 2 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Doc/library/contextlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@ Functions and classes provided:
These context managers may suppress exceptions just as they normally
would if used directly as part of a :keyword:`with` statement.

... versionchanged:: 3.11
Raises :exc:`TypeError` instead of :exc:`AttributeError` if *cm*
is not a context manager.

.. method:: push(exit)

Adds a context manager's :meth:`__exit__` method to the callback stack.
Expand Down Expand Up @@ -585,6 +589,10 @@ Functions and classes provided:
Similar to :meth:`enter_context` but expects an asynchronous context
manager.

... versionchanged:: 3.11
Raises :exc:`TypeError` instead of :exc:`AttributeError` if *cm*
is not an asynchronous context manager.

.. method:: push_async_exit(exit)

Similar to :meth:`push` but expects either an asynchronous context manager
Expand Down
6 changes: 6 additions & 0 deletions Doc/whatsnew/3.11.rst
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ New Features
Other Language Changes
======================

A :exc:`TypeError` is now raised instead of an :exc:`AttributeError` in
:meth:`contextlib.ExitStack.enter_context` and
:meth:`contextlib.AsyncExitStack.enter_async_context` for objects which do not
support the :term:`context manager` or :term:`asynchronous context manager`
protocols correspondingly.
(Contributed by Serhiy Storchaka in :issue:`44471`.)


New Modules
Expand Down
23 changes: 17 additions & 6 deletions Lib/contextlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,14 @@ def enter_context(self, cm):
"""
# We look up the special methods on the type to match the with
# statement.
_cm_type = type(cm)
_exit = _cm_type.__exit__
result = _cm_type.__enter__(cm)
cls = type(cm)
try:
_enter = cls.__enter__
_exit = cls.__exit__
except AttributeError:
raise TypeError(f"'{cls.__module__}.{cls.__qualname__}' object does "
f"not support the context manager protocol") from None
result = _enter(cm)
self._push_cm_exit(cm, _exit)
return result

Expand Down Expand Up @@ -600,9 +605,15 @@ async def enter_async_context(self, cm):
If successful, also pushes its __aexit__ method as a callback and
returns the result of the __aenter__ method.
"""
_cm_type = type(cm)
_exit = _cm_type.__aexit__
result = await _cm_type.__aenter__(cm)
cls = type(cm)
try:
_enter = cls.__aenter__
_exit = cls.__aexit__
except AttributeError:
raise TypeError(f"'{cls.__module__}.{cls.__qualname__}' object does "
f"not support the asynchronous context manager protocol"
) from None
result = await _enter(cm)
self._push_async_cm_exit(cm, _exit)
return result

Expand Down
23 changes: 22 additions & 1 deletion Lib/test/test_contextlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,25 @@ def _exit():
result.append(2)
self.assertEqual(result, [1, 2, 3, 4])

def test_enter_context_errors(self):
class LacksEnterAndExit:
pass
class LacksEnter:
def __exit__(self, *exc_info):
pass
class LacksExit:
def __enter__(self):
pass

with self.exit_stack() as stack:
with self.assertRaisesRegex(TypeError, 'the context manager'):
stack.enter_context(LacksEnterAndExit())
with self.assertRaisesRegex(TypeError, 'the context manager'):
stack.enter_context(LacksEnter())
with self.assertRaisesRegex(TypeError, 'the context manager'):
stack.enter_context(LacksExit())
self.assertFalse(stack._exit_callbacks)

def test_close(self):
result = []
with self.exit_stack() as stack:
Expand Down Expand Up @@ -886,9 +905,11 @@ def test_excessive_nesting(self):
def test_instance_bypass(self):
class Example(object): pass
cm = Example()
cm.__enter__ = object()
cm.__exit__ = object()
stack = self.exit_stack()
self.assertRaises(AttributeError, stack.enter_context, cm)
with self.assertRaisesRegex(TypeError, 'the context manager'):
stack.enter_context(cm)
stack.push(cm)
self.assertIs(stack._exit_callbacks[-1][1], cm)

Expand Down
34 changes: 33 additions & 1 deletion Lib/test/test_contextlib_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ async def __aexit__(self, *exc_details):
1/0

@_async_test
async def test_async_enter_context(self):
async def test_enter_async_context(self):
class TestCM(object):
async def __aenter__(self):
result.append(1)
Expand All @@ -504,6 +504,26 @@ async def _exit():

self.assertEqual(result, [1, 2, 3, 4])

@_async_test
async def test_enter_async_context_errors(self):
class LacksEnterAndExit:
pass
class LacksEnter:
async def __aexit__(self, *exc_info):
pass
class LacksExit:
async def __aenter__(self):
pass

async with self.exit_stack() as stack:
with self.assertRaisesRegex(TypeError, 'asynchronous context manager'):
await stack.enter_async_context(LacksEnterAndExit())
with self.assertRaisesRegex(TypeError, 'asynchronous context manager'):
await stack.enter_async_context(LacksEnter())
with self.assertRaisesRegex(TypeError, 'asynchronous context manager'):
await stack.enter_async_context(LacksExit())
self.assertFalse(stack._exit_callbacks)

@_async_test
async def test_async_exit_exception_chaining(self):
# Ensure exception chaining matches the reference behaviour
Expand Down Expand Up @@ -536,6 +556,18 @@ async def suppress_exc(*exc_details):
self.assertIsInstance(inner_exc, ValueError)
self.assertIsInstance(inner_exc.__context__, ZeroDivisionError)

@_async_test
async def test_instance_bypass_async(self):
Copy link
Member

Choose a reason for hiding this comment

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

What is this test doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a copy of similar test for synchronous context managers (test_instance_bypass), but for asynchronous context managers. It tests that special methods are looked up in a class, not in an instance. So merely setting __aenter__ and __aexit__ as instance attributes does not make a context manager. enter_async_context() fails with relevant message, and push_async_exit() treats argument as a callable, not context manager.

class Example(object): pass
cm = Example()
cm.__aenter__ = object()
cm.__aexit__ = object()
stack = self.exit_stack()
with self.assertRaisesRegex(TypeError, 'asynchronous context manager'):
await stack.enter_async_context(cm)
stack.push_async_exit(cm)
self.assertIs(stack._exit_callbacks[-1][1], cm)


class TestAsyncNullcontext(unittest.TestCase):
@_async_test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
A :exc:`TypeError` is now raised instead of an :exc:`AttributeError` in
:meth:`contextlib.ExitStack.enter_context` and
:meth:`contextlib.AsyncExitStack.enter_async_context` for objects which do
not support the :term:`context manager` or :term:`asynchronous context
manager` protocols correspondingly.