From 4d44f5a40aa70994fe7781cdd6f6ff0f9ce08d6a Mon Sep 17 00:00:00 2001 From: Reid Sunderland Date: Tue, 16 Jun 2026 16:00:56 +0000 Subject: [PATCH 1/2] Fix #1714 putAccelerated errors not being caught, replace repeated Popen code with a wrapper --- sarracenia/transfer/__init__.py | 16 ++++++++++++++++ sarracenia/transfer/file.py | 9 +++++---- sarracenia/transfer/ftp.py | 14 ++++++-------- sarracenia/transfer/https.py | 9 ++++----- sarracenia/transfer/sftp.py | 14 ++++++-------- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/sarracenia/transfer/__init__.py b/sarracenia/transfer/__init__.py index dbdfe0764..d22a0ea03 100644 --- a/sarracenia/transfer/__init__.py +++ b/sarracenia/transfer/__init__.py @@ -28,6 +28,7 @@ import os import random import signal +import subprocess import stat import sys import time @@ -475,6 +476,21 @@ def write_chunk_init(self, proto): def gethttpsUrl(self, path): return None + def runAccelCommand(self, cmd, exc_prefix=''): + """ Run a command, capture stderr. exc_prefix is a string that is added to the beginning of the + exception message. + Raises Exception if the command returns non-zero. + """ + exc_prefix += ' ' + p = subprocess.Popen(cmd, stderr=subprocess.PIPE) + _, stderr = p.communicate() + if p.returncode != 0: + try: + stderr = stderr.decode().strip() + except Exception: + pass + raise Exception(f"{exc_prefix}failed: {stderr} (cmd used: {' '.join(cmd)})") + # batteries included. import sarracenia.transfer.file import sarracenia.transfer.ftp diff --git a/sarracenia/transfer/file.py b/sarracenia/transfer/file.py index 2cda50a0a..4f5187b88 100644 --- a/sarracenia/transfer/file.py +++ b/sarracenia/transfer/file.py @@ -25,7 +25,7 @@ import sarracenia -import os, stat, subprocess, sys, time +import os, stat, sys, time import logging @@ -152,9 +152,10 @@ def getAccelerated(self, msg, remote_file, local_file, length=0, remote_offset=0 cmd = cmd.replace('%d', arg2).split() logger.info(f"accel_cp: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: + try: + self.runAccelCommand(cmd) + except Exception as e: + logger.error(e) return -1 sz = os.stat(arg2).st_size return sz diff --git a/sarracenia/transfer/ftp.py b/sarracenia/transfer/ftp.py index 7fce82a35..c436b51e2 100644 --- a/sarracenia/transfer/ftp.py +++ b/sarracenia/transfer/ftp.py @@ -20,7 +20,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA # -import ftplib, os, subprocess, sys, time, ssl +import ftplib, os, sys, time, ssl import logging from sarracenia.transfer import Transfer from sarracenia.transfer import alarm_cancel, alarm_set, alarm_raise @@ -364,9 +364,10 @@ def getAccelerated(self, msg, remote_file, local_file, length=0, remote_offset=0 cmd = cmd.replace('%d', arg2).split() logger.info(f"accel_ftp: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: + try: + self.runAccelCommand(cmd) + except Exception as e: + logger.error(e) return -1 sz = os.stat(arg2).st_size return sz @@ -487,10 +488,7 @@ def putAccelerated(self, msg, local_file, remote_file, length=0): cmd = cmd.replace('%d', arg2).split() logger.info(f"accel_ftp: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: - return -1 + self.runAccelCommand(cmd, 'putAccelerated') # FIXME: faking success... not sure how to check really. sz = int(msg['size']) return sz diff --git a/sarracenia/transfer/https.py b/sarracenia/transfer/https.py index a42f919a3..edb97735d 100644 --- a/sarracenia/transfer/https.py +++ b/sarracenia/transfer/https.py @@ -26,7 +26,6 @@ import os import sarracenia import ssl -import subprocess import sys from sarracenia.transfer import Transfer @@ -237,10 +236,10 @@ def getAccelerated(self, msg, remote_file, local_file, length, remote_offset=0, cmd = [cmd[0]] + cmd[1:] logger.info(f"accel_wget: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: - logger.warning( f"binary accelerator {cmd} returned: {p.returncode}" ) + try: + self.runAccelCommand(cmd) + except Exception as e: + logger.error(e) return -1 # FIXME: length is not validated. return length diff --git a/sarracenia/transfer/sftp.py b/sarracenia/transfer/sftp.py index 119dfa3fd..69893076a 100644 --- a/sarracenia/transfer/sftp.py +++ b/sarracenia/transfer/sftp.py @@ -21,7 +21,7 @@ # # -import logging, paramiko, os, subprocess, sys, time +import logging, paramiko, os, sys, time from paramiko import * from stat import * @@ -399,9 +399,10 @@ def getAccelerated(self, msg, remote_file, local_file, length=0, remote_offset=0 cmd = self.o.accelScpCommand.replace('%s', arg1) cmd = cmd.replace('%d', arg2).split() logger.info(f"accel_sftp: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: + try: + self.runAccelCommand(cmd) + except Exception as e: + logger.error(e) return -1 sz = os.stat(arg2).st_size return sz @@ -548,10 +549,7 @@ def putAccelerated(self, msg, local_file, remote_file, length=0): cmd = cmd.replace('%d', arg2).split() logger.info(f"accel_sftp: {' '.join(cmd)}") - p = subprocess.Popen(cmd) - p.wait() - if p.returncode != 0: - return -1 + self.runAccelCommand(cmd, 'putAccelerated') # FIXME: faking success... not sure how to check really. sz = int(msg['size']) return sz From b7f3340e4f25b7782a68471caae5eb9c9acd85a2 Mon Sep 17 00:00:00 2001 From: Reid Sunderland Date: Tue, 16 Jun 2026 16:35:25 +0000 Subject: [PATCH 2/2] that extra line was pointless --- sarracenia/transfer/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sarracenia/transfer/__init__.py b/sarracenia/transfer/__init__.py index d22a0ea03..b13b12581 100644 --- a/sarracenia/transfer/__init__.py +++ b/sarracenia/transfer/__init__.py @@ -481,7 +481,6 @@ def runAccelCommand(self, cmd, exc_prefix=''): exception message. Raises Exception if the command returns non-zero. """ - exc_prefix += ' ' p = subprocess.Popen(cmd, stderr=subprocess.PIPE) _, stderr = p.communicate() if p.returncode != 0: @@ -489,7 +488,7 @@ def runAccelCommand(self, cmd, exc_prefix=''): stderr = stderr.decode().strip() except Exception: pass - raise Exception(f"{exc_prefix}failed: {stderr} (cmd used: {' '.join(cmd)})") + raise Exception(f"{exc_prefix} failed: {stderr} (cmd used: {' '.join(cmd)})") # batteries included. import sarracenia.transfer.file