classification
Title: FTP.makeport() loses socket error details
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: giampaolo.rodola, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-12-08 18:51 by serhiy.storchaka, last changed 2012-12-17 19:47 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
ftplib_makeport_raise_err.patch serhiy.storchaka, 2012-12-08 18:57 Patch for 3.3+ review
ftplib.patch giampaolo.rodola, 2012-12-17 12:13
Messages (9)
msg177171 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-08 18:51
In FTP.makeport() a socket error catched and saved but then raised a new error with saved error as argument.

Here is a patch which reraises an original error.
msg177643 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2012-12-17 12:13
Why did you replace socket.error with OSError?
I think we should use socket.create_connection() as a guide line:
http://hg.python.org/cpython/file/45dfb657b430/Lib/socket.py#l401
A patch is in attachment.
msg177644 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-17 12:30
socket.error is alias to OSError since 3.3 (PEP 3151). For versions < 3.3 socket.error should be preserved.
msg177645 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-17 12:38
I agree that copying a code from socket.create_connection() is a good idea. Modernizing socket.error to OSError can be done in a separated issue.
msg177648 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-17 13:24
New changeset b7419f88c628 by Giampaolo Rodola' in branch '3.2':
Fix issue #16646: ftplib.FTP.makeport() might lose socket error details.  (patch by Serhiy Storchaka)
http://hg.python.org/cpython/rev/b7419f88c628

New changeset b8289a08d720 by Giampaolo Rodola' in branch '3.3':
Fix issue #16646: ftplib.FTP.makeport() might lose socket error details.  (patch by Serhiy Storchaka)
http://hg.python.org/cpython/rev/b8289a08d720

New changeset a0b1942600a2 by Giampaolo Rodola' in branch 'default':
Fix issue #16646: ftplib.FTP.makeport() might lose socket error details.  (patch by Serhiy Storchaka)
http://hg.python.org/cpython/rev/a0b1942600a2
msg177649 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-17 13:30
New changeset 6e07be3dfb6b by Giampaolo Rodola' in branch '2.7':
Fix issue #16646: ftplib.FTP.makeport() might lose socket error details.  (patch by Serhiy Storchaka)
http://hg.python.org/cpython/rev/6e07be3dfb6b
msg177656 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-17 19:34
I doubt about the committed variant of patch. Note that both my original patch and socket.create_connection() use intermediate variable for catching an exception. This is done deliberately.

Example:

>>> err = None
>>> try: raise ValueError
... except ValueError as err: pass
... 
>>> err
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'err' is not defined
msg177659 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-17 19:47
New changeset dcefa2c8386b by Giampaolo Rodola' in branch '3.2':
Issue 16646 (ftplib): deliberately use intermediate variable after catching exception
http://hg.python.org/cpython/rev/dcefa2c8386b

New changeset da161499d0c0 by Giampaolo Rodola' in branch '3.3':
Issue 16646 (ftplib): deliberately use intermediate variable after catching exception
http://hg.python.org/cpython/rev/da161499d0c0

New changeset 0845a3dbee38 by Giampaolo Rodola' in branch 'default':
Issue 16646 (ftplib): deliberately use intermediate variable after catching exception
http://hg.python.org/cpython/rev/0845a3dbee38
msg177661 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2012-12-17 19:47
Ouch! My bad. Thanks.
History
Date User Action Args
2012-12-17 19:47:57giampaolo.rodolasetmessages: + msg177661
2012-12-17 19:47:37python-devsetmessages: + msg177659
2012-12-17 19:34:27serhiy.storchakasetmessages: + msg177656
2012-12-17 13:35:49giampaolo.rodolasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2012-12-17 13:30:57python-devsetmessages: + msg177649
2012-12-17 13:24:56python-devsetnosy: + python-dev
messages: + msg177648
2012-12-17 12:38:20serhiy.storchakasetmessages: + msg177645
stage: patch review -> commit review
2012-12-17 12:30:37serhiy.storchakasetmessages: + msg177644
2012-12-17 12:23:47giampaolo.rodolasetassignee: giampaolo.rodola
2012-12-17 12:13:27giampaolo.rodolasetfiles: + ftplib.patch

messages: + msg177643
2012-12-15 22:01:23serhiy.storchakalinkissue16648 dependencies
2012-12-15 19:07:13pitrousetnosy: + giampaolo.rodola
2012-12-08 18:57:34serhiy.storchakasetfiles: + ftplib_makeport_raise_err.patch
2012-12-08 18:56:46serhiy.storchakasetfiles: - ftplib_makeport_raise_err.patch
2012-12-08 18:51:28serhiy.storchakacreate