http.query should handle ConnectionRefusedError exception more nicely

[flassman]

Description of Issue

If you call the http.query against a web server which is down, you get the following stacktrace instead of a simple error message:

      ID: http://localhost/selftest
Function: http.query
  Result: False
 Comment: An exception occurred in this state: Traceback (most recent call last):
            File "c:\salt\bin\lib\site-packages\salt\state.py", line 1919, in call
              **cdata['kwargs'])
            File "c:\salt\bin\lib\site-packages\salt\loader.py", line 1918, in wrapper
              return f(*args, **kwargs)
            File "c:\salt\bin\lib\site-packages\salt\states\http.py", line 92, in query
              data = __salt__['http.query'](name, **kwargs)
            File "c:\salt\bin\lib\site-packages\salt\modules\http.py", line 38, in query
              return salt.utils.http.query(url=url, opts=opts, **kwargs)
            File "c:\salt\bin\lib\site-packages\salt\utils\http.py", line 557, in query
              result = download_client.fetch(url_full, **req_kwargs)
            File "c:\salt\bin\lib\site-packages\tornado\httpclient.py", line 102, in fetch
              self._async_client.fetch, request, **kwargs))
            File "c:\salt\bin\lib\site-packages\tornado\ioloop.py", line 458, in run_sync
              return future_cell[0].result()
            File "c:\salt\bin\lib\site-packages\tornado\concurrent.py", line 238, in result
              raise_exc_info(self._exc_info)
            File "<string>", line 4, in raise_exc_info
          ConnectionRefusedError: [Errno 10061] Unknown error

I would expect that the exception is handled and an error is returned, like in other places, for example if you run file.managed with a source which does not exist:

      ID: test_file_managed
Function: file.managed
    Name: /tmp/something
  Result: False
 Comment: Source file salt://something not found

Setup

I ran the following state, but the URL was not reachable because the server was down:

http://localhost/selftest:
http.query:
- status: 200

Steps to Reproduce Issue

put the above state in some sls file then call it with state.apply

Versions Report

Salt server:

Salt Version:
Salt: 2018.3.4

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 1.5
docker-py: Not Installed
gitdb: 2.0.3
gitpython: 2.1.8
ioflo: Not Installed
Jinja2: 2.7.2
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: 0.31.0
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.5 (default, Jun 11 2019, 12:19:05)
python-gnupg: Not Installed
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: 2.0.3
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4

System Versions:
dist: redhat 7.6 Maipo
locale: UTF-8
machine: x86_64
release: 3.10.0-957.21.3.el7.x86_64
system: Linux
version: Red Hat Enterprise Linux Server 7.6 Maipo

Salt minion:

Salt Version:
Salt: 2018.3.4

Dependency Versions:
cffi: 1.10.0
cherrypy: 10.2.1
dateutil: 2.6.1
docker-py: Not Installed
gitdb: 2.0.5
gitpython: 2.1.3
ioflo: Not Installed
Jinja2: 2.9.6
libgit2: Not Installed
libnacl: 1.6.1
M2Crypto: Not Installed
Mako: 1.0.6
msgpack-pure: Not Installed
msgpack-python: 0.4.8
mysql-python: Not Installed
pycparser: 2.17
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.5.3 (v3.5.3:1880cb95a742, Jan 16 2017, 16:02:32) [MSC v.1900 64 bit (AMD64)]
python-gnupg: 0.4.1
PyYAML: 3.12
PyZMQ: 16.0.3
RAET: Not Installed
smmap: 2.0.5
timelib: 0.2.4
Tornado: 4.5.1
ZMQ: 4.1.6

System Versions:
dist:
locale: cp1252
machine: AMD64
release: 2016Server
system: Windows
version: 2016Server 10.0.14393 SP0 Multiprocessor Free

[cmcmarrow]

Okay @flassman thanks for bring this up.

[doesitblend]

@cmcmarrow I believe this should be labeled as a bug. This may be debatable though depending on expected behavior.

In some scenarios, the user may be restarting a service and then using this to check for the service to be back online as a final step. While a service is restarting the client may receive a "connection refused" error causing the state to terminate execution early and fail.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[flassman]

I assume the issue has been fixed in the 2018.3 branch with #53394
Would be nice if this could be merged into newer versions as well

[stale]

Thank you for updating this issue. It is no longer marked as stale.