thoughtbot / terrapin

Run shell commands safely, even with user-supplied values
Other
250 stars 18 forks source link

Addition of timeout feature? #9

Open jayjlawrence opened 6 years ago

jayjlawrence commented 6 years ago

Has any thought been given to a timeout feature? The thinking here is that if the spawned command is spinning we'll want to abort it after a period of time. This leads to increased issue resiliency in the gem user's code.

Posix::Spawn has implemented their own approach to timeout in the gem which (I expect) avoids the safety issues in MRI Ruby's timeout. I did not see how the timeout parameter is passed through to Posix::Spawn in Terrapin. Additionally the other mechanisms for running the command do not appear to have any timeout handling code. Jruby would be addressed as well.

Would this be a welcome addition to the gem?

zenzei commented 4 years ago

@jayjlawrence i think you need to user the runner_options param to pass timeout to runners.

Terrapin::CommandLine.new("your_command", "--param1", runner_options: {timeout: 30}).run

By the way I have on my code the call method on ProcessRunner overriden to get that timeout behaviour on my scripts. Not my best code but is working so far.

require 'timeout'

module Terrapin
  class TimeoutError < CommandLineError; end

  class CommandLine
    class ProcessRunner
      def call(command, env = {}, options = {})
        pipe = MultiPipe.new
        #puts 'starting process'
        timeout = options.delete(:timeout)
        pid = spawn(env, command, options.merge(pipe.pipe_options))
        begin
          #A value of 0 or nil will execute the block without any timeout.
          Timeout.timeout(timeout) do
            #puts 'waiting for the process #{pid} to end'
            pipe.read_and_then do
              waitpid(pid)
            end
            #puts 'process #{pid} finished in time'
          end
          pipe.output
        rescue Timeout::Error
          #puts 'Process #{pid} not finished in time, killing it'
          #https://blog.simplificator.com/2016/01/18/how-to-kill-processes-on-windows-using-ruby/
          Terrapin::OS.windows? ? system("taskkill /f /pid #{pid}") : Process.kill('TERM', pid)
          raise Terrapin::TimeoutError, "Process #{pid} killed, not finished in time"
        end

      end

    end
  end
end