Merge pull request #4717 from woodruffw/error-pipe

Use JSON to marshal errors from children

Library/Homebrew/build.rb

@@ -11,6 +11,8 @@ require "extend/ENV"
 require "debrew"
 require "fcntl"
 require "socket"
+require "json"
+require "json/add/core"
 
 class Build
   attr_reader :formula, :deps, :reqs
@@ -190,7 +192,17 @@ begin
   build   = Build.new(formula, options)
   build.install
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_hash = JSON.parse e.to_json
+
+  # Special case: We need to toss our build state into the error hash
+  # for proper analytics reporting and sensible error messages.
+  if e.is_a?(BuildError)
+    error_hash["cmd"] = e.cmd
+    error_hash["args"] = e.args
+    error_hash["env"] = e.env
+  end
+
+  error_pipe.write error_hash.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/dev-cmd/test.rb

@@ -93,12 +93,14 @@ module Homebrew
             exec(*args)
           end
         end
-      rescue ::Test::Unit::AssertionFailedError => e
+      rescue ChildProcessError => e
         ofail "#{f.full_name}: failed"
-        puts e.message
-      rescue Exception => e # rubocop:disable Lint/RescueException
-        ofail "#{f.full_name}: failed"
-        puts e, e.backtrace
+        case e.inner["json_class"]
+        when "Test::Unit::AssertionFailedError"
+          puts e.inner["m"]
+        else
+          puts e.inner["json_class"], e.backtrace
+        end
       ensure
         ENV.replace(env)
       end

Library/Homebrew/exceptions.rb

@@ -352,14 +352,16 @@ class FormulaAmbiguousPythonError < RuntimeError
 end
 
 class BuildError < RuntimeError
-  attr_reader :formula, :env
+  attr_reader :formula, :cmd, :args, :env
   attr_accessor :options
 
   def initialize(formula, cmd, args, env)
     @formula = formula
+    @cmd = cmd
+    @args = args
     @env = env
-    args = args.map { |arg| arg.to_s.gsub " ", "\\ " }.join(" ")
-    super "Failed executing: #{cmd} #{args}"
+    pretty_args = args.map { |arg| arg.to_s.gsub " ", "\\ " }.join(" ")
+    super "Failed executing: #{cmd} #{pretty_args}"
   end
 
   def issues
@@ -596,3 +598,20 @@ class BottleFormulaUnavailableError < RuntimeError
     EOS
   end
 end
+
+# Raised when a child process sends us an exception over its error pipe.
+class ChildProcessError < RuntimeError
+  attr_reader :inner
+
+  def initialize(inner)
+    @inner = inner
+
+    super <<~EOS
+      An exception occured within a build process:
+        #{inner["json_class"]}: #{inner["m"]}
+    EOS
+
+    # Clobber our real (but irrelevant) backtrace with that of the inner exception.
+    set_backtrace inner["b"]
+  end
+end

Library/Homebrew/formula_installer.rb

@@ -763,14 +763,25 @@ class FormulaInstaller
       raise "Empty installation"
     end
   rescue Exception => e # rubocop:disable Lint/RescueException
-    e.options = display_options(formula) if e.is_a?(BuildError)
+    # If we've rescued a ChildProcessError and that ChildProcessError
+    # contains a BuildError, then we reconstruct the inner build error
+    # to make analytics happy.
+    if e.is_a?(ChildProcessError) && e.inner["json_class"] == "BuildError"
+      build_error = BuildError.new(formula, e["cmd"], e["args"], e["env"])
+      build_error.set_backtrace e.backtrace
+      build_error.options = display_options(formula)
+
+      e = build_error
+    end
+
     ignore_interrupts do
       # any exceptions must leave us with nothing installed
       formula.update_head_version
       formula.prefix.rmtree if formula.prefix.directory?
       formula.rack.rmdir_if_possible
     end
-    raise
+
+    raise e
   end
 
   def link(keg)

Library/Homebrew/postinstall.rb

@@ -4,6 +4,7 @@ require "global"
 require "debrew"
 require "fcntl"
 require "socket"
+require "json/add/core"
 
 begin
   error_pipe = UNIXSocket.open(ENV["HOMEBREW_ERROR_PIPE"], &:recv_io)
@@ -15,7 +16,7 @@ begin
   formula.extend(Debrew::Formula) if ARGV.debug?
   formula.run_post_install
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_pipe.write e.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/test.rb

@@ -7,6 +7,7 @@ require "debrew"
 require "formula_assertions"
 require "fcntl"
 require "socket"
+require "json/add/core"
 
 TEST_TIMEOUT_SECONDS = 5 * 60
 
@@ -28,7 +29,7 @@ begin
     raise "test returned false" if formula.run_test == false
   end
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_pipe.write e.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/utils/fork.rb

@@ -1,5 +1,7 @@
 require "fcntl"
 require "socket"
+require "json"
+require "json/add/core"
 
 module Utils
   def self.safe_fork(&_block)
@@ -15,7 +17,7 @@ module Utils
             write.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
             yield
           rescue Exception => e # rubocop:disable Lint/RescueException
-            Marshal.dump(e, write)
+            write.write e.to_json
             write.close
             exit!
           else
@@ -36,7 +38,7 @@ module Utils
           data = read.read
           read.close
           Process.wait(pid) unless socket.nil?
-          raise Marshal.load(data) unless data.nil? || data.empty? # rubocop:disable Security/MarshalLoad
+          raise ChildProcessError, JSON.parse(data) unless data.nil? || data.empty?
           raise Interrupt if $CHILD_STATUS.exitstatus == 130
           raise "Forked child process failed: #{$CHILD_STATUS}" unless $CHILD_STATUS.success?
         end