jayvdb created this task. jayvdb claimed this task. jayvdb added subscribers: jayvdb, valhallasw. jayvdb added a project: pywikibot-core. jayvdb changed Security from none to none.
TASK DESCRIPTION ```
http.fetch(uri='http://getstatuscode.com/301')
Exception in thread Thread-1: Traceback (most recent call last): File "/usr/lib64/python2.7/threading.py", line 811, in __bootstrap_inner self.run() File "pywikibot/comms/threadedhttp.py", line 486, in run item.data = self.http.request(*item.args, **item.kwargs) File "pywikibot/comms/threadedhttp.py", line 254, in request uri, method, body, headers, response, content, max_redirects) File "pywikibot/comms/threadedhttp.py", line 273, in _follow_redirect response, content) RedirectMissingLocation: Redirected but the response is missing a Location: header. ```
And the main thread doesnt return.
This exception is thrown by threadedhttp, whereas threadedhttp normally returns the exception to be passed back to the main thread.
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
To: jayvdb Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
jayvdb added a comment.
Presumably the same happens with 'raise httplib2.RedirectLimit' in the same method.
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
jayvdb changed the title from "HTTP redirect without Location locks up the http layer" to "exceptions in threadedhttp.Http._follow_redirect cause a lock up". jayvdb triaged this task as "High" priority.
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
valhallasw added a comment.
I think that instead of the current
try: (response, content) = httplib2.Http.request( self, uri, method, body, headers, max_redirects, connection_type ) except Exception as e: # what types? # return exception instance to be retrieved by the calling thread return e
wrapper, we should actually just wrap all of `def request` with a try-catch block which returns e on error. Does that sound like the right solution to you?
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb, valhallasw Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
jayvdb added a comment.
I was thinking it should be in HttpProcessor.run , which has a try..finally , .. but not catch. depending on how bad the exception is (and it is best to assume the worst, like some state has been irreparably altered: https://gerrit.wikimedia.org/r/#/c/178789/ ), it would be sensible to replace the dead http worker with a new one.
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
valhallasw added a comment.
Yes, that sounds reasonable. This does have some implications for when we can and cannot re-use a connection, though -- we should prevent spinning up a new connection for typical issues (e.g. 503s).
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb, valhallasw Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
jayvdb added a comment.
ya, we have a try.. catch inside .request , and it returns the exception to be processed by the client. I think that should not be changed, which is where 503's normally happen. It would only be unexpected exceptions which would be handled by HttpProcessor.run and result in a new worker being created.
I am also interested in why we need the special follow code. Im pretty sure it is being used because of %20 -> _ on MediaWiki sites, but that should only be needed for full urls to wiki pages, which is very rare occurrence in pywikibot core these days.
TASK DETAIL https://phabricator.wikimedia.org/T78123
REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign <username>.
EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: jayvdb Cc: Aklapper, jayvdb, valhallasw, pywikipedia-bugs
pywikipedia-bugs@lists.wikimedia.org