Skip to content

Commit 2d267b8

Browse files
committed
fix: improve error handling of CommandLine interfaces
Currently, errors arising from interpolating the command line of these interfaces is handled poorly at the ``Node`` level. If the command line cannot be built, the error is printed in the logfile but the exception is caught and never raised (i.e., likely leading to an infinite loop as the execution is not stopped). I have experienced that while debugging fMRIPrep. To learn which of the inputs of a faulty interface derived from ``CommandLine`` was not being formatted, I had to also add the error annotation proposed for the ``_parse_inputs`` inner loop.
1 parent eab4b2a commit 2d267b8

File tree

3 files changed

+40
-9
lines changed

3 files changed

+40
-9
lines changed

nipype/interfaces/base/core.py

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,7 @@ class must be instantiated with a command argument
607607
_cmd = None
608608
_version = None
609609
_terminal_output = "stream"
610+
_write_cmdline = False
610611

611612
@classmethod
612613
def set_default_terminal_output(cls, output_type):
@@ -623,7 +624,7 @@ def set_default_terminal_output(cls, output_type):
623624
else:
624625
raise AttributeError("Invalid terminal output_type: %s" % output_type)
625626

626-
def __init__(self, command=None, terminal_output=None, **inputs):
627+
def __init__(self, command=None, terminal_output=None, write_cmdline=False, **inputs):
627628
super(CommandLine, self).__init__(**inputs)
628629
self._environ = None
629630
# Set command. Input argument takes precedence
@@ -638,6 +639,8 @@ def __init__(self, command=None, terminal_output=None, **inputs):
638639
if terminal_output is not None:
639640
self.terminal_output = terminal_output
640641

642+
self._write_cmdline = write_cmdline
643+
641644
@property
642645
def cmd(self):
643646
"""sets base command, immutable"""
@@ -669,6 +672,14 @@ def terminal_output(self, value):
669672
)
670673
self._terminal_output = value
671674

675+
@property
676+
def write_cmdline(self):
677+
return self._write_cmdline
678+
679+
@write_cmdline.setter
680+
def write_cmdline(self, value):
681+
self._write_cmdline = value is True
682+
672683
def raise_exception(self, runtime):
673684
raise RuntimeError(
674685
(
@@ -716,9 +727,14 @@ def _run_interface(self, runtime, correct_return_codes=(0,)):
716727
adds stdout, stderr, merged, cmdline, dependencies, command_path
717728
718729
"""
719-
720730
out_environ = self._get_environ()
721731
# Initialize runtime Bunch
732+
733+
try:
734+
runtime.cmdline = self.cmdline
735+
except Exception as exc:
736+
raise RuntimeError("Error raised when interpolating the command line") from exc
737+
722738
runtime.stdout = None
723739
runtime.stderr = None
724740
runtime.cmdline = self.cmdline
@@ -742,7 +758,11 @@ def _run_interface(self, runtime, correct_return_codes=(0,)):
742758
if self._ldd
743759
else "<skipped>"
744760
)
745-
runtime = run_command(runtime, output=self.terminal_output)
761+
runtime = run_command(
762+
runtime,
763+
output=self.terminal_output,
764+
write_cmdline=self.write_cmdline,
765+
)
746766
return runtime
747767

748768
def _format_arg(self, name, trait_spec, value):
@@ -907,7 +927,14 @@ def _parse_inputs(self, skip=None):
907927

908928
if not isdefined(value):
909929
continue
910-
arg = self._format_arg(name, spec, value)
930+
931+
try:
932+
arg = self._format_arg(name, spec, value)
933+
except Exception as exc:
934+
raise ValueError(
935+
f"Error formatting command line argument '{name}' with value '{value}'"
936+
) from exc
937+
911938
if arg is None:
912939
continue
913940
pos = spec.position

nipype/pipeline/engine/nodes.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ def __init__(
209209
self.needed_outputs = needed_outputs
210210
self.config = None
211211

212+
if issubclass(self._interface.__class__, CommandLine):
213+
self._interface.write_cmdline = True
214+
212215
@property
213216
def interface(self):
214217
"""Return the underlying interface object"""
@@ -711,16 +714,13 @@ def _run_command(self, execute, copyfiles=True):
711714
f'[Node] Executing "{self.name}" <{self._interface.__module__}'
712715
f".{self._interface.__class__.__name__}>"
713716
)
717+
714718
# Invoke core run method of the interface ignoring exceptions
715719
result = self._interface.run(cwd=outdir, ignore_exception=True)
716720
logger.info(
717721
f'[Node] Finished "{self.name}", elapsed time {result.runtime.duration}s.'
718722
)
719723

720-
if issubclass(self._interface.__class__, CommandLine):
721-
# Write out command line as it happened
722-
Path.write_text(outdir / "command.txt", f"{result.runtime.cmdline}\n")
723-
724724
exc_tb = getattr(result.runtime, "traceback", None)
725725

726726
if not exc_tb:

nipype/utils/subprocess.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import select
1111
import locale
1212
import datetime
13+
from pathlib import Path
1314
from subprocess import Popen, STDOUT, PIPE
1415
from .filemanip import canonicalize_env, read_stream
1516

@@ -69,7 +70,7 @@ def _read(self, drain):
6970
self._lastidx = len(self._rows)
7071

7172

72-
def run_command(runtime, output=None, timeout=0.01):
73+
def run_command(runtime, output=None, timeout=0.01, write_cmdline=False):
7374
"""Run a command, read stdout and stderr, prefix with timestamp.
7475
7576
The returned runtime contains a merged stdout+stderr log with timestamps
@@ -100,6 +101,9 @@ def run_command(runtime, output=None, timeout=0.01):
100101
errfile = os.path.join(runtime.cwd, "stderr.nipype")
101102
stderr = open(errfile, "wb")
102103

104+
if write_cmdline:
105+
(Path(runtime.cwd) / "command.txt").write_text(cmdline)
106+
103107
proc = Popen(
104108
cmdline,
105109
stdout=stdout,

0 commit comments

Comments
 (0)