If pyroe encounters an error, simpleaf prevents debugging by swallowing stderr

[robsyme-seqera]

Description

In the event that pyroe does not return cleanly, simpleaf prints an error notifying the source of the problem, but little else to help debug the error:

found `salmon` in the PATH at /usr/local/bin/salmon
found `alevin-fry` in the PATH at /usr/local/bin/alevin-fry
found `pyroe` in the PATH at /usr/local/bin/pyroe
Error: pyroe failed to return succesfully ExitStatus(unix_wait_status(256))

Recommendations/Requests

When the pyroe error is caught, perhaps it would be helpful if simpleaf printed two extra pieces of information:

1. It would be helpful to have the pyroe command that was run
2. It would be helpful to have the stderr and stdout of the pyroe command printed before or after the error to help debugging.

I think that the Debug implementation of std::process::Command will print the full command, which makes request #1 a bit easier. The stderr/out can be pulled from cres.stderr

Relevant lines:

https://github.com/COMBINE-lab/simpleaf/blob/50db027cd84b07711e26cddb6cf3940e2e620656/src/main.rs#L519-L524

[rob-p]

@DongzeHE ☝🏻 we could work this into the upcoming release.

[DongzeHE]

Sure! I will work on this today.

[robsyme]

You beat me to it! I was looking forward to writing some more Rust :D

I'm running into the same coy behaviour from simpleaf when generate-permit-list fails, so it might be worth printing the gpl_proc_out stderr here as well:

https://github.com/COMBINE-lab/simpleaf/blob/50db027cd84b07711e26cddb6cf3940e2e620656/src/main.rs#L901-L906

[rob-p]

Ok, just pushed 44fb9cb that works toward addressing this. The idea is to create a more verbose general function that does the actual command execution and has the appropriately verbose behavior in the event of a failure. The commit message describes the current behavior, which should satisfy the initial desired output. Moving forward, we can expand / enhance logging through this function (e.g. if we want to log failed stdout/stderr to a file log as well, etc.).

[robsyme]

Thanks Rob! This will surely be helpful to other users, as I'm sure I won't be the only person with odd-looking input data. The utility function seems absolutely the correct idea.

As a small measure of thanks (and a shameless excuse to right some rust), I've submitted https://github.com/COMBINE-lab/simpleaf/pull/44 that contains some humbly offered suggestions, but feel free to reject it if don't like the syntax changes. The spelling fix is probably worth keeping though, but that can be quickly fixed yourself if you reject the PR.

[rob-p]

These changes have been incorporated and improved propagation of errors upstream in pyroe has also been incorporated as of v0.8.0 — which is pending release in bioconda now as well. So, I'm going to close this issue, but please feel free to open another if you notice anything else related to this.

Currently, the plan is to cut the new simpleaf release this afternoon, which will incorporate all of these changes.