uri: check unexpected failure doesn't occur when file cannot be saved (#45824)

* uri: fix TypeError when file can't be saved

Fix the following exception (and others):

    Traceback (most recent call last):
      File "/home/lilou/debug_dir/__main__.py", line 604, in <module>
        main()
      File "/home/lilou/debug_dir/__main__.py", line 554, in main
        write_file(module, url, dest, content, resp)
      File "/home/lilou/debug_dir/__main__.py", line 320, in write_file
        module.fail_json(msg="Destination dir '%s' not writable" % os.path.dirname(dest), **resp)
    TypeError: fail_json() got multiple values for keyword argument 'msg'

I would rather remove **resp from returned values but this module is
flagged as stableinterface.

* Static imports are more straight forward and preferred unless dynamic inclusion is required.

changelogs/fragments/45824-uri-fix-TypeError.yaml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - "uri: fix TypeError when file can't be saved"

lib/ansible/modules/net_tools/basics/uri.py

@@ -298,6 +298,11 @@ from ansible.module_utils.urls import fetch_url, url_argument_spec
 JSON_CANDIDATES = ('text', 'json', 'javascript')
 
 
+def format_message(err, resp):
+    msg = resp.pop('msg')
+    return err + (' %s' % msg if msg else '')
+
+
 def write_file(module, url, dest, content, resp):
     # create a tempfile with some test content
     fd, tmpsrc = tempfile.mkstemp(dir=module.tmpdir)
@@ -306,8 +311,8 @@ def write_file(module, url, dest, content, resp):
         f.write(content)
     except Exception as e:
         os.remove(tmpsrc)
-        module.fail_json(msg="failed to create temporary content file: %s" % to_native(e),
-                         exception=traceback.format_exc(), **resp)
+        msg = format_message("Failed to create temporary content file: %s" % to_native(e), resp)
+        module.fail_json(msg=msg, exception=traceback.format_exc(), **resp)
     f.close()
 
     checksum_src = None
@@ -316,10 +321,12 @@ def write_file(module, url, dest, content, resp):
     # raise an error if there is no tmpsrc file
     if not os.path.exists(tmpsrc):
         os.remove(tmpsrc)
-        module.fail_json(msg="Source '%s' does not exist" % tmpsrc, **resp)
+        msg = format_message("Source '%s' does not exist" % tmpsrc, resp)
+        module.fail_json(msg=msg, **resp)
     if not os.access(tmpsrc, os.R_OK):
         os.remove(tmpsrc)
-        module.fail_json(msg="Source '%s' not readable" % tmpsrc, **resp)
+        msg = format_message("Source '%s' not readable" % tmpsrc, resp)
+        module.fail_json(msg=msg, **resp)
     checksum_src = module.sha1(tmpsrc)
 
     # check if there is no dest file
@@ -327,23 +334,26 @@ def write_file(module, url, dest, content, resp):
         # raise an error if copy has no permission on dest
         if not os.access(dest, os.W_OK):
             os.remove(tmpsrc)
-            module.fail_json(msg="Destination '%s' not writable" % dest, **resp)
+            msg = format_message("Destination '%s' not writable" % dest, resp)
+            module.fail_json(msg=msg, **resp)
         if not os.access(dest, os.R_OK):
             os.remove(tmpsrc)
-            module.fail_json(msg="Destination '%s' not readable" % dest, **resp)
+            msg = format_message("Destination '%s' not readable" % dest, resp)
+            module.fail_json(msg=msg, **resp)
         checksum_dest = module.sha1(dest)
     else:
         if not os.access(os.path.dirname(dest), os.W_OK):
             os.remove(tmpsrc)
-            module.fail_json(msg="Destination dir '%s' not writable" % os.path.dirname(dest), **resp)
+            msg = format_message("Destination dir '%s' not writable" % os.path.dirname(dest), resp)
+            module.fail_json(msg=msg, **resp)
 
     if checksum_src != checksum_dest:
         try:
             shutil.copyfile(tmpsrc, dest)
         except Exception as e:
             os.remove(tmpsrc)
-            module.fail_json(msg="failed to copy %s to %s: %s" % (tmpsrc, dest, to_native(e)),
-                             exception=traceback.format_exc(), **resp)
+            msg = format_message("failed to copy %s to %s: %s" % (tmpsrc, dest, to_native(e)), resp)
+            module.fail_json(msg=msg, exception=traceback.format_exc(), **resp)
 
     os.remove(tmpsrc)
 

test/integration/targets/uri/tasks/main.yml

@@ -488,13 +488,16 @@
       - result.json.json[0] == 'JSON Test Pattern pass1'
 
 - name: Test follow_redirects=none
-  include_tasks: redirect-none.yml
+  import_tasks: redirect-none.yml
 
 - name: Test follow_redirects=safe
-  include_tasks: redirect-safe.yml
+  import_tasks: redirect-safe.yml
 
 - name: Test follow_redirects=urllib2
-  include_tasks: redirect-urllib2.yml
+  import_tasks: redirect-urllib2.yml
 
 - name: Test follow_redirects=all
-  include_tasks: redirect-all.yml
+  import_tasks: redirect-all.yml
+
+- name: Check unexpected failures
+  import_tasks: unexpected-failures.yml

test/integration/targets/uri/tasks/unexpected-failures.yml

@@ -0,0 +1,27 @@
+---
+# same as expanduser & expandvars called on managed host
+- command: 'echo {{ output_dir }}'
+  register: echo
+
+- set_fact:
+    remote_dir_expanded: '{{ echo.stdout }}'
+
+- name: ensure test directory doesn't exist
+  file:
+    path: '{{ output_dir }}/non/existent/path'
+    state: absent
+
+- name: destination doesn't exist
+  uri:
+    url: 'https://{{ httpbin_host }}/get'
+    dest: '{{ output_dir }}/non/existent/path'
+  ignore_errors: true
+  register: ret
+
+- name: check that unexpected failure didn't happen
+  assert:
+    that:
+      - ret is failed
+      - "not ret.msg.startswith('MODULE FAILURE')"
+      - '"Destination dir ''" ~ remote_dir_expanded ~ "/non/existent'' not writable" in ret.msg'
+      - ret.status == 200