safwank / ElixirRetry

Simple Elixir macros for linear retry, exponential backoff and wait with composable delays
Other
441 stars 32 forks source link

Consolidated `retry/2` by moving `exceptions` argument into `rescue_only` option #12

Closed fredwu closed 7 years ago

fredwu commented 7 years ago

Hi,

11 introduced retry/3 to specifically cater for a list of exceptions to allow rescuing. It's great but it means having both retry/2 and retry/3. I propose to have only retry/2 and have exceptions be passed in as an option. This would also allow more flexible expansion of options in the future.

What do you guys thinks? @safwank @neerfri

fredwu commented 7 years ago

On a somewhat unrelated note, the test suite takes over 20 seconds to run due to the timing checks. In cd39ed8 I reduced those timing numbers so now it runs under 3 seconds.

safwank commented 7 years ago

@fredwu Ah, that's actually pretty nice. Why didn't I think of that earlier?

I'll sleep on it tonight.

halostatue commented 7 years ago

You probably want to consider a slight change to this because it is possible to call retry without a with: parameter now. The simplest change would be to use Keyword.fetch!(opts, :with) instead of opts[:with], but by using a function and two heads on the macro, you can get the compile-time failure that originally existed:

    stream_builder = opts[:with]
    exceptions     = opts[:rescue_only] || [RuntimeError]

    quote do
      fun = unquote(block_runner(block, exceptions))

      unquote(delays_from(stream_builder))
      |> Enum.reduce_while(nil, fn(delay, _last_result) ->
        :timer.sleep(delay)
        fun.()
      end)
      |> case do
        {:exception, e} -> raise e
        result          -> result
      end
    end
  end
safwank commented 7 years ago

@halostatue Very good point. Something along these lines?

  defmacro retry(opts = [with: _stream_builder], block) do
    do_retry(opts, block)
  end
  defmacro retry(opts = [with: _stream_builder, rescue_only: _exceptions], block) do
    do_retry(opts, block)
  end
safwank commented 7 years ago

Actually there's a more generic way:

  defmacro retry(opts = [{:with, _stream_builder} | _], block) do
    do_retry(opts, block)
  end

That way we not only get compile-time checking for with but also allow other options to be specified without having to add additional function heads. The fact that with has to be the first option is a plus.

safwank commented 7 years ago

Released as v0.8.1.