Use rescue to avoid dependency inversion

[carrascoacd]

Problem

When using Finch is expected to receive a RuntimeError if the connection pool is exhausted, see https://github.com/sneako/finch/blob/main/test/finch_test.exs#L561

This is a problem because in a service that uses a lower tier API than the service itself, a dependency inversion of tiers can happen. For example, a tier 2 service calling a tier 3 service.

The tier inversion happens because we are not rescuing the error and returning it into a tuple, so the operation can work without problems under this situation.

Proposed solution

Encapsulate the error in the error tuple. I'm adding this solution here, but I want to know if you consider that it would be best suited in the Finch code itself. Changing reraise basically.

cc/ @yordis

[yordis]

I am concerned with this one; it would change the public API, I am not saying it is bad, but I must be careful here.

[carrascoacd]

What do you mean with changing the public API? As far as I know, the adapter is still returning the error tuple as usual but with a new reason.

Could you provide me with an example?

[yordis]

{:error, {:runtime_error, e}}

That is a new pattern matching that must be taken into consideration. I just do not know what assumptions people made, which leads me to:

e in RuntimeError ->

What would happen if somebody was expecting such RuntimeError exception btw?

[carrascoacd]

Regarding the pattern matching, we could change it to a raw string.

What would happen if somebody was expecting such RuntimeError exception btw?

Good question. In my opinion, returning a RuntimeError is breaking the contract the Adapter defines, that us basically a ok or error tuple. This is already breaking the contract in fact. If you use Mint adapter you should be able to replace it with Finch adapter without changing your codebase. This is basically the typical substitution principle.

But I get your point, that's what I'm no sure if this should be added in the Finch library instead. The fact is that, this adapter is not valid for production usage as it is (without catching the error).

Let me know if you prefer to close this and move this PR yo Finch instead.

Thanks!