gh-140691: urllib.request: Close FTP control socket if data socket can't connect #140835
Conversation
Co-Authored-By: codenamenam <bluetire27@gmail.com>
| if expected_err: | ||
| msg = ("Didn't get expected error(s) %s for %s %s, got %s: %s" % | ||
| (expected_err, url, req, type(err), err)) | ||
| self.assertIsInstance(err, expected_err, msg) | ||
| else: | ||
| raise |
There was a problem hiding this comment.
(The new context (assertRaises) handles the expected error; printing information is nowadays covered by subTest)
encukou
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 3cbf28b 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
| import contextlib | ||
| import errno | ||
| import unittest | ||
| from unittest import mock |
There was a problem hiding this comment.
micro nit: from unittest alphabetically would be after from test.support (PEP 8 doesn't actually talk about alphabetical though)
There was a problem hiding this comment.
Oh, I put it next to the other unittest line.
| # gh-140691: When the data connection (but not control connection) | ||
| # cannot be made established, we shouldn't leave an open socket object. | ||
|
|
||
| class MockError(OSError): |
There was a problem hiding this comment.
Why does this need to be MockError? the except pieces don't look for MockError specifically
There was a problem hiding this comment.
I want it to be recognizable in any tracebacks.
|
The PPPC64LE RHEL8 Refleaks PR build seems to have found a distinct but similar unclosed socket in exception case: test_ftp (test.test_urllib2net.OtherNetworkTests.test_ftp) ... Warning -- Unraisable exception
Exception ignored while finalizing socket <socket.socket fd=3, family=2, type=1, proto=6, laddr=('147.229.8.193', 48118), raddr=('68.183.26.59', 21)>:
Traceback (most recent call last):
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1822, in retrfile
self.ftp.cwd(file)
~~~~~~~~~~~~^^^^^^
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 619, in cwd
return self.voidcmd(cmd)
~~~~~~~~~~~~^^^^^
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 286, in voidcmd
return self.voidresp()
~~~~~~~~~~~~~^^
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 259, in voidresp
resp = self.getresp()
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/ftplib.py", line 254, in getresp
raise error_perm(resp)
ftplib.error_perm: 550 Failed to change directory.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1547, in ftp_open
fp, retrlen = fw.retrfile(file, type)
~~~~~~~~~~~^^^^^^^^^^^^
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1824, in retrfile
raise URLError('ftp error: %r' % reason) from reason
urllib.error.URLError: <urlopen error ftp error: error_perm('550 Failed to change directory.')>
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_urllib2net.py", line 23, in _retry_thrice
return func(*args, **kwargs)
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 487, in open
response = self._open(req, data)
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 504, in _open
result = self._call_chain(self.handle_open, protocol, protocol +
'_open', req)
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 464, in _call_chain
result = func(*args)
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/urllib/request.py", line 1560, in ftp_open
raise URLError(f"ftp error: {exp}") from exp
urllib.error.URLError: <urlopen error ftp error: <urlopen error ftp error: error_perm('550 Failed to change directory.')>>
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/buildbot/buildarea/pull_request.cstratak-RHEL8-ppc64le.refleak/build/Lib/test/test_urllib2net.py", line 25, in _retry_thrice
last_exc = e
^^^^^^^^
ResourceWarning: unclosed <socket.socket fd=3, family=2, type=1, proto=6, laddr=('147.229.8.193', 48118), raddr=('68.183.26.59', 21)>
okbuild:https://buildbot.python.org/#/builders/353/builds/2319 |
|
Thank you for the review! Hmm, how did you get the traceback? The logs show |
|
!buildbot PPC64LE.Fedora.Stable |
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 57634d3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140835%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
3 participants
Uh oh!
There was an error while loading. Please reload this page.