Merge pull request #269 from MikaelSmith/LTH-149

(LTH-149) Handle errors if child has exited before all writing finished

CHANGELOG.md

@@ -2,6 +2,11 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.2.2]
+
+### Fixed
+- Allow Leatherman.execute calls to opt into allowing tasks to finish without reading stdin (i.e. don't fail when the pipe is closed) via the `allow_stdin_unread` option. This specifically supports pxp-agent's task execution where input may or may not be used. (LTH-149)
+
 ## [1.2.1]
 
 ### Fixed

CMakeLists.txt

@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.2.2)
-project(leatherman VERSION 1.2.1)
+project(leatherman VERSION 1.2.2)
 
 if (WIN32)
     link_libraries("-Wl,--nxcompat -Wl,--dynamicbase")

curl/tests/mock_curl.cc

@@ -99,11 +99,15 @@ CURL *curl_easy_init()
  * option is set, we'll return CURLE_COULDNT_CONNECT for the current
  * option.
  */
+#pragma clang diagnostic push
+// This function signature is required to mock curl, so disable a warning generated from it.
+#pragma clang diagnostic ignored "-Wvarargs"
 CURLcode curl_easy_setopt(CURL *handle, CURLoption option, ...)
 {
     auto h = reinterpret_cast<curl_impl*>(handle);
     va_list vl;
     va_start(vl, option);
+#pragma clang diagnostic pop
 
     switch (option) {
         case CURLOPT_HEADERFUNCTION:

execution/inc/leatherman/execution/execution.hpp

@@ -82,6 +82,10 @@ namespace leatherman { namespace execution {
          * if called from multi threaded processes.
          */
         thread_safe = (1 << 11),
+        /**
+         * Allow standard input to be unread, rather than failing if it is ignored.
+         */
+        allow_stdin_unread = (1 << 12),
         /**
          * A combination of all throw options.
          */

execution/src/posix/execution.cc

@@ -184,7 +184,7 @@ namespace leatherman { namespace execution {
         bool read;
     };
 
-    static void rw_from_child(pid_t child, array<pipe, 3>& pipes, uint32_t timeout)
+    static void rw_from_child(pid_t child, array<pipe, 3>& pipes, uint32_t timeout, bool allow_stdin_unread)
     {
         // Each pipe is a tuple of descriptor, buffer to use to read data, and a callback to call when data is read
         // The input pair is a descriptor and text to write to it
@@ -242,13 +242,17 @@ namespace leatherman { namespace execution {
                     read(pipe.descriptor, &pipe.buffer[0], pipe.buffer.size()) :
                     write(pipe.descriptor, pipe.buffer.c_str(), pipe.buffer.size());
                 if (count < 0) {
-                    if (errno != EINTR) {
-                        LOG_ERROR("{1} pipe i/o failed: {2}.", pipe.name, format_error());
-                        throw execution_exception(_("child i/o failed."));
+                    if (allow_stdin_unread && !pipe.read && errno == EPIPE) {
+                        // Input pipe was closed prematurely due to process exit, log and let it go.
+                        LOG_DEBUG("{1} pipe i/o was closed early, process may have ignored input.", pipe.name);
+                        pipe.descriptor = {};
+                        continue;
+                    } else if (errno == EINTR) {
+                        // Interrupted by signal
+                        LOG_DEBUG("{1} pipe i/o was interrupted and will be retried.", pipe.name);
+                        continue;
                     }
-                    // Interrupted by signal
-                    LOG_DEBUG("{1} pipe i/o was interrupted and will be retried.", pipe.name);
-                    continue;
+                    throw execution_exception(_("{1} pipe i/o failed: {2}", pipe.name, format_error()));
                 } else if (count == 0) {
                     // Pipe has closed
                     pipe.descriptor = {};
@@ -538,7 +542,7 @@ namespace leatherman { namespace execution {
                 input ? pipe("stdin", move(stdin_write), *input) : pipe("", {}, "")
             }};
 
-            rw_from_child(child, pipes, timeout);
+            rw_from_child(child, pipes, timeout, options[execution_options::allow_stdin_unread]);
         });
 
         // Close the read pipes

locales/leatherman.pot

@@ -6,7 +6,7 @@
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: leatherman 1.2.1\n"
+"Project-Id-Version: leatherman 1.2.2\n"
 "Report-Msgid-Bugs-To: docs@puppet.com\n"
 "POT-Creation-Date: \n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
@@ -216,17 +216,20 @@ msgstr ""
 msgid "select call was interrupted and will be retried."
 msgstr ""
 
-#. error
+#. debug
 #: execution/src/posix/execution.cc
-#: execution/src/windows/execution.cc
-msgid "{1} pipe i/o failed: {2}."
+msgid "{1} pipe i/o was closed early, process may have ignored input."
 msgstr ""
 
 #. debug
 #: execution/src/posix/execution.cc
 msgid "{1} pipe i/o was interrupted and will be retried."
 msgstr ""
 
+#: execution/src/posix/execution.cc
+msgid "{1} pipe i/o failed: {2}"
+msgstr ""
+
 #: execution/src/posix/execution.cc
 #: execution/src/windows/execution.cc
 msgid "command timed out after {1} seconds."
@@ -345,6 +348,11 @@ msgstr ""
 msgid "failed to create read event."
 msgstr ""
 
+#. error
+#: execution/src/windows/execution.cc
+msgid "{1} pipe i/o failed: {2}."
+msgstr ""
+
 #. error
 #: execution/src/windows/execution.cc
 msgid "failed to wait for child process i/o: {1}."