diff --git a/changelogs/fragments/45824-uri-fix-TypeError.yaml b/changelogs/fragments/45824-uri-fix-TypeError.yaml new file mode 100644 index 0000000000..bb284f0be8 --- /dev/null +++ b/changelogs/fragments/45824-uri-fix-TypeError.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - "uri: fix TypeError when file can't be saved" diff --git a/lib/ansible/modules/net_tools/basics/uri.py b/lib/ansible/modules/net_tools/basics/uri.py index 35e0a916ae..e45e7fb179 100644 --- a/lib/ansible/modules/net_tools/basics/uri.py +++ b/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) diff --git a/test/integration/targets/uri/tasks/main.yml b/test/integration/targets/uri/tasks/main.yml index 7571ab8abf..b76c772256 100644 --- a/test/integration/targets/uri/tasks/main.yml +++ b/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 \ No newline at end of file + import_tasks: redirect-all.yml + +- name: Check unexpected failures + import_tasks: unexpected-failures.yml diff --git a/test/integration/targets/uri/tasks/unexpected-failures.yml b/test/integration/targets/uri/tasks/unexpected-failures.yml new file mode 100644 index 0000000000..ac38871c33 --- /dev/null +++ b/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