feat: add with_timeout() function

[jay7x]

No description provided.

[jay7x]

unit tests are missing as those doesn't seem to be executed at the moment at all..

[jay7x]

Not sure if this should be ctrl::with_timeout() actually 🤔

[nmburgan]

We perhaps shouldn't use Timeout.timeout. It's considered dangerous. It works by raising Timeout::Error via Thread#raise, which can interrupt execution at any point, including inside ensure blocks, mutex operations, etc. If Timeout::Error fires inside a run_task call's internal ensure block (like while it's closing an SSH connection), the cleanup code gets interrupted and the reraised Bolt::Error masks the real problem.

In order to make this safe, you'd either need to guard a bunch of stuff in the internals of OpenBolt, or perhaps you could do this:

def with_timeout(timeout, &block)
    unless Puppet[:tasks]
      raise Puppet::ParseErrorWithIssue
        .from_issue_and_stack(Bolt::PAL::Issues::PLAN_OPERATION_NOT_SUPPORTED_WHEN_COMPILING,
                              action: 'with_timeout')
    end

    executor = Puppet.lookup(:bolt_executor)
    executor.report_function_call(self.class.name)

    # Each invocation gets a unique exception class so that:
    # 1. Transport code's `rescue => e` (StandardError) won't catch it
    # 2. Nested with_timeout calls won't catch each other's exceptions
    exc_class = Class.new(Exception)

    Thread.handle_interrupt(exc_class => :never) do
      Timeout.timeout(timeout, exc_class) do
        Thread.handle_interrupt(exc_class => :on_blocking) do
          block.call
        end
      end
    end
  rescue exc_class
    raise Bolt::Error.new(
      "Execution in with_timeout() block expired after #{timeout} seconds",
      'bolt/execution-expired',
      details: { timeout: timeout }
    )
  end
end

But I'm not 100% confident this doesn't have edge cases and problems of its own.

[donoghuc]

perhaps the name should imply the behavior. Maybe "abandon_after" or something. Ideally on timeout there would be some action on the target to stop the hung thing, this just abandon it. I think a better (though more complex) solution would be to figure out a way to send a "stop" to a target. That may be tricky for some transports, but there is a pattern in pxp for PE.

[jay7x]

@nmburgan would you prefer me to change the implementation to the suggested one? Or you'd prefer to not have such a function at all until we have proper support in (at least some) transports?