kotlin-result
kotlin-result copied to clipboard
Make BindException public
As BindException can be thrown its possible for a another try block to catch it so technically it's part of the API. I actually discovered this because of bug where I was accidentally catching it and then I was unable to write a condition to specifically rethrow it because its marked internal 😬
Could you paste the example code in which you managed to catch it? It definitely shouldn't leak from the call to binding{}
/.bind()
.
Generally I would not consider the BindException
part of the public API. It is only thrown as part of the ResultBindingImpl
which itself is marked as internal
. with the expectation that the binding
function will catch it.
): Result<Unit, InvocationError> = binding {
...
val result = runCatching { // runCatching will catch the inner bind as it catches throwable
instructions.forEach { instruction ->
instructionExecutor(instruction, store, stack).bind()
}
}
So this wouldn't be a problem if I could do this
result.onFailure { err ->
// todo rethrow bind exception
if(err is BindException) throw err
if (err is BreakException) {
...
}
if (err is ReturnException) {
...
}
I'm mixing exceptions and errors here intentionally, I understand this isn't how you typically program with result but its essential in my use case
So fundamentally the problem resides in the fact that all calls to .bind()
in a binding
expect to throw exactly one level up the chain, such that it breaks out of the immediate parent binding
statement, but if a consumer wraps a call to .bind()
with a try { } catch (t: Throwable)
(or runCatching
) they'll catch it but be unable to narrow down the type of Throwable
to just the BindException
.
If my explanation above is correct then I think I am generally happy with the way things are currently and wouldn't be looking to make the BindException
public. This forces you to not try/catch
or runCatching
any calls to .bind()
- and instead expects them to never be able to fail, which is intentional in design.
The fact that .bind()
itself returns a Result
should be an indicator to the caller that it cannot throw, and therefore a try/catch
/runCatching
around the call to .bind()
is inappropriate.
I understand this isn't how you typically program with result but its essential in my use case
If you'd like to provide a more full-fledged example I'm sure I can help you rewrite it to avoid this case. As it stands the inclusion of a call to runCatching
inside of a binding
is concerning, and something I definitely would advise against.
I appreciate I have a very unique scenario, I'm building an interpreter and I'm forced to use Exceptions when modelling branching (breaking and returning), so I wanted to mix and match.
If you're happy with your api design thats no problem, what I can do is just not throw bind inside the runCatching lambda and manually wire the result
Hi @CharlieTap. I did some thinking about the use case that you've hit, and I have an idea (to make runCatching
explicitly handle BindingException
for you). I was trying this, but I actually can't seem to come up with a test case that fails with the behaviour you've outlined.
Please can you have a look this test case and tell me what you are doing differently?
For me the runCatching
is not swallowing the inner .bind()
, and thus I do not need to rethrow it. Please can you explain what you're doing that makes you hit the edge case so I can come up with a test for it and then see if my idea above will solve it for you? Thanks.