Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of timeout feature? #9

Open
jayjlawrence opened this issue Jul 31, 2018 · 1 comment
Open

Addition of timeout feature? #9

jayjlawrence opened this issue Jul 31, 2018 · 1 comment

Comments

@jayjlawrence
Copy link

jayjlawrence commented Jul 31, 2018

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?

@jayjlawrence jayjlawrence changed the title addition of tiimeout feature? Addition of timeout feature? Jul 31, 2018
@zenzei
Copy link

zenzei commented Jan 26, 2020

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants