Skip to content

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Mar 18, 2019

The imap.IMAP4.logout() method no longer ignores silently arbitrary
exceptions.

Changes:

  • The IMAP4.logout() method now expects a "BYE" untagged response,
    rather than relying on _check_bye() which raises a self.abort()
    exception.
  • IMAP4.exit() now does nothing if the client already logged out.
  • Add more debug info if test_logout() tests fail.

https://bugs.python.org/issue36348

@@ -625,11 +625,8 @@ def logout(self):
Returns server 'BYE' response.
"""
self.state = 'LOGOUT'
try: typ, dat = self._simple_command('LOGOUT')
except: typ, dat = 'NO', ['%s: %s' % sys.exc_info()[:2]]
typ, dat = self._simple_command('LOGOUT')
Copy link
Member

Choose a reason for hiding this comment

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

👍

@matrixise
Copy link
Member

title: "does not ignore"?

The imap.IMAP4.logout() method no longer ignores silently arbitrary
exceptions.

Changes:

* The IMAP4.logout() method now expects a "BYE" untagged response,
  rather than relying on _check_bye() which raises a self.abort()
  exception.
* IMAP4.__exit__() now does nothing if the client already logged out.
* Add more debug info if test_logout() tests fail.
@vstinner vstinner changed the title bpo-36348: IMAP4.logout() don't ignore bugs bpo-36348: IMAP4.logout() doesn't ignore exc Mar 18, 2019
@vstinner
Copy link
Member Author

I'm not sure if this change will fix the random failures on buildbot, since _command() wraps socket exceptions into self.abort(). Example if you revert my __exit__() change:

======================================================================
ERROR: test_with_statement_logout (test.test_imaplib.NewIMAPSSLTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 976, in _command
    self.send(data + CRLF)
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 318, in send
    self.sock.sendall(data)
  File "/home/vstinner/prog/python/master/Lib/ssl.py", line 1018, in sendall
    return super().sendall(data, flags)
OSError: [Errno 9] Bad file descriptor

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/test/test_imaplib.py", line 461, in test_with_statement_logout
    self.assertIsNone(server.logged)
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 276, in __exit__
    self.logout()
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 628, in logout
    typ, dat = self._simple_command('LOGOUT')
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 1200, in _simple_command
    return self._command_complete(name, self._command(name, *args))
  File "/home/vstinner/prog/python/master/Lib/imaplib.py", line 978, in _command
    raise self.abort('socket error: %s' % val)
imaplib.IMAP4.abort: socket error: [Errno 9] Bad file descriptor

@vstinner
Copy link
Member Author

I tried but failed to reproduce the buildbot failure: https://bugs.python.org/issue36348#msg338233 I ran "./python -u -m test -u all -m test_logout -v test_imaplib" 4 times in parallel and I modified the tests to remove transient_internet().

@vstinner
Copy link
Member Author

I plan to merge this PR at the end of the week.

@bitdancer, @warsaw: Would you mind to have a look?

@vstinner vstinner merged commit 74125a6 into python:master Apr 15, 2019
@vstinner vstinner deleted the imaplib_logout branch April 15, 2019 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants