Let FastlanePty detect when externally invoked programs crash, harden it when using popen, and expose process statuses.

fastlane_core/lib/fastlane_core/fastlane_pty.rb

@@ -10,48 +10,78 @@ def exit_status
 
 module FastlaneCore
   class FastlanePtyError < StandardError
-    attr_reader :exit_status
-    def initialize(e, exit_status)
+    attr_reader :exit_status, :process_status
+    def initialize(e, exit_status, process_status)
       super(e)
       set_backtrace(e.backtrace) if e
       @exit_status = exit_status
+      @process_status = process_status
     end
   end
 
   class FastlanePty
-    def self.spawn(command)
-      require 'pty'
-      PTY.spawn(command) do |command_stdout, command_stdin, pid|
-        begin
-          yield(command_stdout, command_stdin, pid)
-        rescue Errno::EIO
-          # Exception ignored intentionally.
-          # https://stackoverflow.com/questions/10238298/ruby-on-linux-pty-goes-away-without-eof-raises-errnoeio
-          # This is expected on some linux systems, that indicates that the subcommand finished
-          # and we kept trying to read, ignore it
-        ensure
+    def self.spawn(command, &block)
+      begin
+        spawn_with_pty(command, &block)
+      rescue LoadError
+        spawn_with_popen(command, &block)
+      end
+    end
+
+    def self.spawn_with_pty(command, &block)
+      begin
+        require 'pty'
+        PTY.spawn(command) do |command_stdout, command_stdin, pid|
           begin
-            Process.wait(pid)
-          rescue Errno::ECHILD, PTY::ChildExited
-            # The process might have exited.
+            yield(command_stdout, command_stdin, pid)
+          rescue Errno::EIO
+            # Exception ignored intentionally.
+            # https://stackoverflow.com/questions/10238298/ruby-on-linux-pty-goes-away-without-eof-raises-errnoeio
+            # This is expected on some linux systems, that indicates that the subcommand finished
+            # and we kept trying to read, ignore it
+          ensure
+            begin
+              Process.wait(pid)
+            rescue Errno::ECHILD, PTY::ChildExited
+              # The process might have exited.
+            end
           end
         end
+        status = self.process_status
+        raise StandardError, "Process crashed" if status.signaled?
+        status.exitstatus
+      rescue StandardError => e
+        # Wrapping any error in FastlanePtyError to allow
+        # callers to see and use $?.exitstatus that
+        # would usually get returned
+        status = self.process_status
+        raise FastlanePtyError.new(e, status.exitstatus || e.exit_status, status)
       end
-      $?.exitstatus
-    rescue LoadError
-      require 'open3'
-      Open3.popen2e(command) do |command_stdin, command_stdout, p| # note the inversion
-        yield(command_stdout, command_stdin, p.value.pid)
+    end
 
-        command_stdin.close
-        command_stdout.close
-        p.value.exitstatus
+    def self.spawn_with_popen(command, &block)
+      status = nil
+      begin
+        require 'open3'
+        Open3.popen2e(command) do |command_stdin, command_stdout, p| # note the inversion
+          status = p.value
+          yield(command_stdout, command_stdin, status.pid)
+          command_stdin.close
+          command_stdout.close
+          raise StandardError, "Process crashed" if status.signaled?
+          status.exitstatus
+        end
+      rescue StandardError => e
+        # Wrapping any error in FastlanePtyError to allow
+        # callers to see and use $?.exitstatus that
+        # would usually get returned
+        raise FastlanePtyError.new(e, status.exitstatus || e.exit_status, status)
       end
-    rescue StandardError => e
-      # Wrapping any error in FastlanePtyError to allow
-      # callers to see and use $?.exitstatus that
-      # would usually get returned
-      raise FastlanePtyError.new(e, $?.exitstatus)
+    end
+
+    # to ease mocking
+    def self.process_status
+      $?
     end
   end
 end

fastlane_core/spec/crasher/README.md

@@ -0,0 +1,3 @@
+A program that crashes, used to test corner cases in FastlanePty
+
+gcc -o crasher main.c

fastlane_core/spec/crasher/main.c

@@ -0,0 +1,12 @@
+#include<stdio.h>
+#include<signal.h>
+#include<unistd.h> 
+#include<stdlib.h>
+int main()
+{
+    sigset_t act;
+    sigemptyset(&act);
+    sigfillset(&act);
+    sigprocmask(SIG_UNBLOCK,&act,NULL);
+    abort();
+}

fastlane_core/spec/fastlane_pty_spec.rb

@@ -0,0 +1,89 @@
+describe FastlaneCore do
+  describe FastlaneCore::FastlanePty do
+    describe "spawn" do
+      it 'executes a simple command successfully' do
+        @all_lines = []
+
+        exit_status = FastlaneCore::FastlanePty.spawn('echo foo') do |command_stdout, command_stdin, pid|
+          command_stdout.each do |line|
+            @all_lines << line.chomp
+          end
+        end
+        expect(exit_status).to eq(0)
+        expect(@all_lines).to eq(["foo"])
+      end
+
+      it 'doesn t return -1 if an exception was raised in the block in PTY.spawn' do
+        exception = StandardError.new
+        expect {
+          exit_status = FastlaneCore::FastlanePty.spawn('echo foo') do |command_stdout, command_stdin, pid|
+            raise exception
+          end
+        }.to raise_error(FastlaneCore::FastlanePtyError) { |error|
+          expect(error.exit_status).to eq(0) # command was success but output handling failed
+        }
+      end
+
+      it 'doesn t return -1 if an exception was raised in the block in Open3.popen2e' do
+        expect(FastlaneCore::FastlanePty).to receive(:require).with("pty").and_raise(LoadError)
+        allow(FastlaneCore::FastlanePty).to receive(:require).with("open3").and_call_original
+        allow(FastlaneCore::FastlanePty).to receive(:open3)
+
+        exception = StandardError.new
+        expect {
+          exit_status = FastlaneCore::FastlanePty.spawn('echo foo') do |command_stdout, command_stdin, pid|
+            raise exception
+          end
+        }.to raise_error(FastlaneCore::FastlanePtyError) { |error|
+          expect(error.exit_status).to eq(0) # command was success but output handling failed
+        }
+      end
+
+      # could be used to test
+      # let(:crasher_path) { File.expand_path("./fastlane_core/spec/crasher/crasher") }
+
+      it 'raises an error if the program crashes through PTY.spawn' do
+        status = double("ProcessStatus")
+        allow(status).to receive(:exitstatus) { nil }
+        allow(status).to receive(:signaled?) { true }
+
+        expect(FastlaneCore::FastlanePty).to receive(:require).with("pty").and_return(nil)
+        allow(FastlaneCore::FastlanePty).to receive(:process_status).and_return(status)
+
+        expect {
+          exit_status = FastlaneCore::FastlanePty.spawn("a path of a crasher exec") do |command_stdout, command_stdin, pid|
+          end
+          UI.message(exit_status)
+        }.to raise_error(FastlaneCore::FastlanePtyError) { |error|
+          expect(error.exit_status).to eq(-1) # command was forced to -1
+        }
+      end
+
+      it 'raises an error if the program crashes through PTY.popen' do
+        stdin = double("stdin")
+        allow(stdin).to receive(:close)
+        stdout = double("stdout")
+        allow(stdout).to receive(:close)
+
+        status = double("ProcessStatus")
+        allow(status).to receive(:exitstatus) { nil }
+        allow(status).to receive(:signaled?) { true }
+        allow(status).to receive(:pid) { 12_345 }
+
+        process = double("process")
+        allow(process).to receive(:value) { status }
+
+        expect(FastlaneCore::FastlanePty).to receive(:require).with("pty").and_raise(LoadError)
+        allow(FastlaneCore::FastlanePty).to receive(:require).with("open3").and_return(nil)
+        allow(Open3).to receive(:popen2e).and_yield(stdin, stdout, process)
+
+        expect {
+          exit_status = FastlaneCore::FastlanePty.spawn("a path of a crasher exec") do |command_stdout, command_stdin, pid|
+          end
+        }.to raise_error(FastlaneCore::FastlanePtyError) { |error|
+          expect(error.exit_status).to eq(-1) # command was forced to -1
+        }
+      end
+    end
+  end
+end