Make after & else clauses optional

[nathanalderson]

This PR makes the after and else clauses optional. Essentially, this:

retry with: ... do
  ...
end

Is now equivalent to:

retry with: ... do
  ...
after
  result -> result
else
  error -> raise error
end

I could be convinced to change the default else behavior to simply returning the exception, but re-raising it seems more natural to me.

I also improved the error messaging slightly by calling out the specific option or clause the user messed up when possible. So you'll get a message like you must provide a "do" clause or option "foo" is unsupported instead of a more generic message.

So far I have only done the implementation for the retry macro, but if you like the concept, I will update wait similarly. I think retry_while is already fine in this respect since it only has a do block.

[safwank]

Thanks for the PR @nathanalderson! I've been wanting to do this for a while. I'll take a look at the PR this weekend and get back to you.

[nathanalderson]

Went ahead and did wait. This:

wait ... do
  ...
end

Is now equivalent to this:

wait ... do
  ...
after
  result -> {:ok, result}
else
  error -> {:error, error}
end

A few other notes:

• The order of the clauses no longer matters.
• We don't detect and error on duplicate clauses. The last one wins (like a standard keyword list). Could add a check for this if desired.

[nathanalderson]

After some thought, I realized that the appropriate thing to do in the default retry ... else clause is to reraise if the last failure was an exception and return the failed value otherwise. That's now the behavior. In other words:

retry with: ... do
  ...
end

Is now equivalent to:

retry with: ... do
  ...
after
  result -> result
else
  e when is_exception(e) -> raise e
  e -> e
end

[nathanalderson]

Sorry, meant to mark this as ready. Let me know if you have feedback.

I have a follow-up PR that addresses some more needs I have including some discussed in #54.

[nathanalderson]

Just bumping this a bit. Not super urgent but didn't want it to fall off your radar or something. Let me know if there's anything I can do to assist.

[safwank]

@nathanalderson Rest assured I haven't forgotten about this. I'm currently busy with life, so please bear with me.

[nathanalderson]

Periodic bump. Would love to get this in and introduce a follow-up. 🙂

[safwank]

@nathanalderson Thanks for the fixes. A couple of comments

• The test coverage has dropped a bit. Mind plugging the gap?
• Can you please revert this commit and create a separate PR for it so we can keep this PR small and focused?

[nathanalderson]

Sorry, the merge commit was accidental. That's the follow-up PR I've mentioned. Meant to merge upstream/master instead of origin/master. I've reverted. I also covered the two missed syntax errors in unit test.

I also added a comment to disable the complexity warning from credo on the main retry macro. I tried moving the else/after logic into a function and that did fix the warning, but in my opinion it made things less readable. I'm open to doing that if you prefer.