process.stderr.read() fails with NoneType

[tjgalvin]

Expected Behavior

I think that this might be related to #209

I am running a container that contains a program that prints exclusively to the stderr. I added #209 to let a user tweak which stream to capture and print. Ideally this will simply print the stderr output as it becomes available.

Actual Behavior

  File "/scratch3/gal16b/packages/flint/flint/prefect/flows/continuum_pipeline.py", line 77, in task_run_bane_and_aegean
    aegean_outputs = run_bane_and_aegean(image=image_path, aegean_container=aegean_container)
  File "/scratch3/gal16b/packages/flint/flint/source_finding/aegean.py", line 60, in run_bane_and_aegean
    run_singularity_command(image=aegean_container, command=aegean_command, bind_dirs=bind_dir)
  File "/scratch3/gal16b/packages/flint/flint/sclient.py", line 61, in run_singularity_command
    for line in output:
  File "/scratch3/gal16b/packages/singularity-cli/spython/utils/terminal.py", line 149, in stream_command
    print(process.stderr.read(), file=sys.stderr)
AttributeError: 'NoneType' object has no attribute 'read'

I am not sure what the interplay between subprocess.Popen(..., stderr=subprocess.STDOUT) and process.stderr.read() is. My reading of the docs (https://docs.python.org/3/library/subprocess.html) suggest that if preocess.stderr is None then there was nothing captured.

Maybe it is worth having a check to ensure that stderr is not None before attempting to print? I am not familiar with the finer details fo subprocess here to know.

Steps to Reproduce

I am trying to come up with a reproducible example.

Context

[provide more detailed introduction to the issue itself . This is for make a reproducible issue.]

• Operating System: Flavor of redhead (this is on a Cray HPC)
• singularity version: 3.7.3
• spython version: 0.3.1
• python version: 3.8.17

Failure Logs

Encountered exception during execution:
Traceback (most recent call last):
  File "/scratch3/gal16b/mambaforge/envs/flint/lib/python3.8/site-packages/prefect/engine.py", line 1719, in orchestrate_task_run
    result = await call.aresult()
  File "/scratch3/gal16b/mambaforge/envs/flint/lib/python3.8/site-packages/prefect/_internal/concurrency/calls.py", line 292, in aresult
    return await asyncio.wrap_future(self.future)
  File "/scratch3/gal16b/mambaforge/envs/flint/lib/python3.8/site-packages/prefect/_internal/concurrency/calls.py", line 316, in _run_sync
    result = self.fn(*self.args, **self.kwargs)
  File "/scratch3/gal16b/packages/flint/flint/prefect/flows/continuum_pipeline.py", line 77, in task_run_bane_and_aegean
    aegean_outputs = run_bane_and_aegean(image=image_path, aegean_container=aegean_container)
  File "/scratch3/gal16b/packages/flint/flint/source_finding/aegean.py", line 60, in run_bane_and_aegean
    run_singularity_command(image=aegean_container, command=aegean_command, bind_dirs=bind_dir)
  File "/scratch3/gal16b/packages/flint/flint/sclient.py", line 61, in run_singularity_command
    for line in output:
  File "/scratch3/gal16b/packages/singularity-cli/spython/utils/terminal.py", line 149, in stream_command
    print(process.stderr.read(), file=sys.stderr)
AttributeError: 'NoneType' object has no attribute 'read'

Possible Fix

Perhaps something like

    # If there is an error, raise.
    process.stdout.close()
    return_code = process.wait()
    if return_code:
       err_lines = process.stderr.read() if process.stderr else process.stdout.read() 
       print(err_lines file=sys.stderr)
       
        raise subprocess.CalledProcessError(return_code, cmd)

[tjgalvin]

So, when I rerun the command that caused the initial error it looks like my OS kills it as it gobbles up too much memory. So I could see why this was not an error properly captured. I am not sure what the behaviour is here - is it the singularity process that is killed, or the parent python process that ultimately runs the subprocess? It must be just the singularity command otherwise I wouldn't get this stack trace.

For some more context this particular error was raised with a prefect pipeline. So on my side are a few layers to unwrap.

This might be a user error issue unrelated to spython. But I think perhaps the check for a non-None stderr could still be useful.

[vsoch]

I think likely what is happening is that the container (which is just akin to any running process) is killed one level up. Would it make sense to wrap this call in something to catch the error? So if I understand your fix, when this happens you see output to the stdout but not stderr? Isn't that the opposite of what we'd expect?

[tjgalvin]

So, as far as I can tell it is not related to #209 like I thought initially.

It seems to be a case that in this particular failure more there is simply a case that process.stderr is None. I could capture the error upstream in my own code. My preference would be keeping the try/except catch as close to the source as possible though.

[vsoch]

I would be okay with some check for None (and a different outcome given this case). I don't think we want to take a preference on how to handle a specific try/catch, however. Let me know if that works for you. Your possible fix section above I think would be OK.

[tjgalvin]

Sorry for my delay in getting back to this. I have raised #212 as a fix for this. Any thoughts or comments?