uri: Add redirect tests for none, safe, urllib2 and all

[dagwieers]

SUMMARY

This is required if we want to ensure that #36809 doesn't cause any important behavioral changes.

This PR includes:

• All possible tests for testing HTTP forwarding behaviour in fetch_url and open_url
• Exposes 'urllib2' method to users (for testing) but undocumented
• Adds support for BadStatusLine exceptions to provide a more detailed error message (instead of An unknown error occured)

This requires httpbin v0.6.0+ we are using v0.5.0.
Latest httpbin is v0.6.2.

See #37076 #37222

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

uri

ANSIBLE VERSION

v2.5

[ansibot]

cc @romeotheriault
click here for bot help

[kbreit]

Checking in. Is there something in this PR we're waiting on?

[sivel]

We are waiting on updated testing environment to support these tests. The PRs are referenced above.

[dagwieers]

Now that we have an updated httpbin, the integration tests are failing, but surprisingly it's not any tests of this PR that are failing...

The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_O84sPO/ansible_modlib.zip/ansible/module_utils/urls.py", line 1017, in fetch_url
    client_key=client_key, cookies=cookies)
  File "/tmp/ansible_O84sPO/ansible_modlib.zip/ansible/module_utils/urls.py", line 920, in open_url
    r = urllib_request.urlopen(*urlopen_args)
  File "/usr/lib64/python2.7/urllib2.py", line 154, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib64/python2.7/urllib2.py", line 431, in open
    response = self._open(req, data)
  File "/usr/lib64/python2.7/urllib2.py", line 449, in _open
    '_open', req)
  File "/usr/lib64/python2.7/urllib2.py", line 409, in _call_chain
    result = func(*args)
  File "/usr/lib64/python2.7/urllib2.py", line 1244, in http_open
    return self.do_open(httplib.HTTPConnection, req)
  File "/usr/lib64/python2.7/urllib2.py", line 1217, in do_open
    r = h.getresponse(buffering=True)
  File "/usr/lib64/python2.7/httplib.py", line 1089, in getresponse
    response.begin()
  File "/usr/lib64/python2.7/httplib.py", line 444, in begin
    version, status, reason = self._read_status()
  File "/usr/lib64/python2.7/httplib.py", line 408, in _read_status
    raise BadStatusLine(line)
BadStatusLine: ''

PS And it is not failing locally. Let's restart Shippable.

[sivel]

The httptester sha update hasn't been merged yet: #37746

[dagwieers]

@sivel In any case we need to catch BadStatusLine better in order to debug this... Thanks for the heads up.

[sivel]

Those failures are generally due to the simple http server we start during the tests having trouble, or terminating before the tests are complete.

[dagwieers]

@sivel I'd like to return a proper error to the user, how about:

except BadStatusLine as e:
    info.update(dict(msg="Connection failure: connection was closed before a valid response was received: %s" % to_native(e.line), status=-1))

[dagwieers]

@sivel This PR is ready for re-evaluation. It only adds tests to pinpoint the existing forwarding behavior of the different methods, before we add #36809

[dagwieers]

This is ready to be re-evaluated.

[sivel]

Is the intention to get this PR merged and then rebase #36809 to resolve the FIXME issues?

I don't necessarily have a preference, I just want to make sure I understand. The other option would be to remove the failing tests and add them in #36809

[dagwieers]

Yes, first we need this one merged, so we have a baseline for the original behavior.
I will then resolve the HTTP 307/308 fixes, so you can easily tell from the integration tests diff if those changes are to be expected. If this gets merged now, I will do the other PR asap.

[sivel]

rebuild_merge