Fixing up error handling for fetch_file ops in connection plugins

* enable batch mode (configurable with a config option, on by default)
  for sftp transfers, so we can catch errors more easily
* general cleanup in the local connection plugin and fetch action plugin

Fixes #11612

examples/ansible.cfg

@@ -220,6 +220,11 @@ fact_caching = memory
 # (default is sftp)
 #scp_if_ssh = True
 
+# if False, sftp will not use batch mode to transfer files. This may cause some
+# types of file transfer failures impossible to catch however, and should
+# only be disabled if your sftp version has problems with batch mode
+#sftp_batch_mode = False
+
 [accelerate]
 accelerate_port = 5099
 accelerate_timeout = 30

lib/ansible/constants.py

@@ -130,6 +130,7 @@ DEFAULT_ASK_VAULT_PASS    = get_config(p, DEFAULTS, 'ask_vault_pass',    'ANSIBL
 DEFAULT_VAULT_PASSWORD_FILE = shell_expand_path(get_config(p, DEFAULTS, 'vault_password_file', 'ANSIBLE_VAULT_PASSWORD_FILE', None))
 DEFAULT_TRANSPORT         = get_config(p, DEFAULTS, 'transport',        'ANSIBLE_TRANSPORT',        'smart')
 DEFAULT_SCP_IF_SSH        = get_config(p, 'ssh_connection', 'scp_if_ssh',       'ANSIBLE_SCP_IF_SSH',       False, boolean=True)
+DEFAULT_SFTP_BATCH_MODE   = get_config(p, 'ssh_connection', 'sftp_batch_mode', 'ANSIBLE_SFTP_BATCH_MODE', True, boolean=True)
 DEFAULT_MANAGED_STR       = get_config(p, DEFAULTS, 'ansible_managed',  None,           'Ansible managed: {file} modified on %Y-%m-%d %H:%M:%S by {uid} on {host}')
 DEFAULT_SYSLOG_FACILITY   = get_config(p, DEFAULTS, 'syslog_facility',  'ANSIBLE_SYSLOG_FACILITY', 'LOG_USER')
 DEFAULT_KEEP_REMOTE_FILES = get_config(p, DEFAULTS, 'keep_remote_files', 'ANSIBLE_KEEP_REMOTE_FILES', False, boolean=True)

lib/ansible/plugins/action/fetch.py

@@ -131,9 +131,12 @@ class ActionModule(ActionBase):
             if remote_data is None:
                 self._connection.fetch_file(source, dest)
             else:
+                try:
                     f = open(dest, 'w')
                     f.write(remote_data)
                     f.close()
+                except (IOError, OSError) as e:
+                    raise AnsibleError("Failed to fetch the file: %s" % e)
             new_checksum = secure_hash(dest)
             # For backwards compatibility.  We'll return None on FIPS enabled
             # systems

lib/ansible/plugins/connections/local.py

@@ -113,11 +113,9 @@ class Connection(ConnectionBase):
         try:
             shutil.copyfile(in_path, out_path)
         except shutil.Error:
-            traceback.print_exc()
             raise AnsibleError("failed to copy: {0} and {1} are the same".format(in_path, out_path))
-        except IOError:
+        except IOError as e:
-            traceback.print_exc()
+            raise AnsibleError("failed to transfer file to {0}: {1}".format(out_path, e))
-            raise AnsibleError("failed to transfer file to {0}".format(out_path))
 
     def fetch_file(self, in_path, out_path):
         ''' fetch a file from local to local -- for copatibility '''

lib/ansible/plugins/connections/ssh.py

@@ -472,6 +472,12 @@ class Connection(ConnectionBase):
             indata = None
         else:
             cmd.append('sftp')
+            # sftp batch mode allows us to correctly catch failed transfers,
+            # but can be disabled if for some reason the client side doesn't
+            # support the option
+            if C.DEFAULT_SFTP_BATCH_MODE:
+                cmd.append('-b')
+                cmd.append('-')
             cmd.extend(self._common_args)
             cmd.append(self.host)
             indata = "get {0} {1}\n".format(in_path, out_path)