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.