From 2ef9f168d34d5f0573dd486629e8cf0218b189a9 Mon Sep 17 00:00:00 2001
From: Jake Zimmerman <zimmerman.jake@gmail.com>
Date: Sat, 28 May 2022 21:18:20 -0700
Subject: [PATCH] Add retcode to CalledProcessError (#81)

It's useful to be able to know the exit code of the process when it
fails with a non-zero exit code.

To get this to work, I had to fix a bug in the implementation of
check_output. Previously, check_output would call both `p.communicate()`
and `p.poll()`. The former has the effect of waiting for EOF on the
input and then waiting for the child to exit, reaping it with
`waitpid(2)`. Unfortunately, `p.poll()` was hoping to be able to also
use `waitpid(2)` to retrieve the exit code of the process. But since the
child had already been reaped, the given pid no longer existed, and thus
`waitpid(2)` would return `ECHILD`.

Luckily the call to `p.poll()` is unnecessary, as the process already
provides `p.retcode()` for retrieving the exit code.
---
 subprocess.hpp        | 13 +++++++------
 test/test_ret_code.cc | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/subprocess.hpp b/subprocess.hpp
index e6ea910..8ebac01 100755
--- a/subprocess.hpp
+++ b/subprocess.hpp
@@ -123,8 +123,9 @@ static const size_t DEFAULT_BUF_CAP_BYTES = 8192;
 class CalledProcessError: public std::runtime_error
 {
 public:
-  explicit CalledProcessError(const std::string& error_msg):
-    std::runtime_error(error_msg)
+  int retcode;
+  CalledProcessError(const std::string& error_msg, int retcode):
+    std::runtime_error(error_msg), retcode(retcode)
   {}
 };
 
@@ -1630,10 +1631,10 @@ inline void Popen::execute_process() noexcept(false)
         // Call waitpid to reap the child process
         // waitpid suspends the calling process until the
         // child terminates.
-        wait();
+        int retcode = wait();
 
         // Throw whatever information we have about child failure
-        throw CalledProcessError(err_buf);
+        throw CalledProcessError(err_buf, retcode);
       }
     } catch (std::exception& exp) {
       stream_.cleanup_fds();
@@ -1984,9 +1985,9 @@ namespace detail
     static_assert(!detail::has_type<output, detail::param_pack<Args...>>::value, "output not allowed in args");
     auto p = Popen(std::forward<F>(farg), std::forward<Args>(args)..., output{PIPE});
     auto res = p.communicate();
-    auto retcode = p.poll();
+    auto retcode = p.retcode();
     if (retcode > 0) {
-      throw CalledProcessError("Command failed : Non zero retcode");
+      throw CalledProcessError("Command failed : Non zero retcode", retcode);
     }
     return std::move(res.first);
   }
diff --git a/test/test_ret_code.cc b/test/test_ret_code.cc
index 1db9e87..f2c0c3a 100644
--- a/test/test_ret_code.cc
+++ b/test/test_ret_code.cc
@@ -27,9 +27,25 @@ void test_ret_code_comm()
   std::cout << "retcode: " << cut.retcode() << std::endl;
 }
 
+void test_ret_code_check_output()
+{
+  using namespace sp;
+  bool caught = false;
+  try {
+      auto obuf = check_output({"/bin/false"}, shell{false});
+      assert(false); // Expected to throw
+  } catch (CalledProcessError &e) {
+      std::cout << "retcode: " << e.retcode << std::endl;
+      assert (e.retcode == 1);
+      caught = true;
+  }
+  assert(caught);
+}
+
 int main() {
   // test_ret_code();
   test_ret_code_comm();
+  test_ret_code_check_output();
 
   return 0;
 }