using `finally` ends connections prematurely in `daemonapi` about nim-libp2p HOT 7 CLOSED

You are right, its my fault, but we need to stick with CatchableError not Exception.

from nim-libp2p.

Since the exception handling here is used for something similar to destructor clean-up, catching Exception and re-raising seems appropriate.

from nim-libp2p.

well it's not - it's undefined behaviour when a Defect is raised - by definition you're in an invalid state, so you can't unwind sanely.. you don't necessarily want to run destructors (ie perform a clean database shutdown like a destructor would) in that case because they'll be behaving in an undefined way.

from nim-libp2p.

The whole point of unwinding is to achieve graceful shutdown. Otherwise, it would be perfectly adequate to quit immediately. Raising a Defect exception still comes with various guarantees about memory safety, so executing clean up code is quite reasonable. In general, such clean up code can take care of releasing distributed resources such as database transactions in flight, distributed locks and so on. Without doing this clean up, such resources will have to time out which is a less optimal solution.

Even locally, the OS takes care of releasing some resources automatically, but it may still leave files of the application lying around and other unwanted side effects that can be cleaned up.

from nim-libp2p.

but a Defect by definition is a state that you did not anticipate - therefore you also cannot handle it gracefully, because the graceful shutdown code can no longer be expected to perform its task correctly - in fact, this is when it might cause further damage and simply letting go and not doing anything is as good as any other response.

the state of the objects you're trying to unwind is wrong and you don't know how. it's simply not consistent with the definition of Defect to try to make sense of the situation - memory might be corrupt as well, in case of an indexoutofbounds for example.

what you're describing is appropriate behaviour for an exception - ie an error that is so unusual that it is an exception that it happens, but still within the realm of consistent and anticipated states, and graceful shutdown can happen: the transaction can be rolled back etc.

from nim-libp2p.

the root cause of this inconsistency is that the definition of Defect is problematic: recoverable or not is generally contextual for the caller of the api, not its designer, and two Defect:s are generally not the same: missing key during lookup in a map is a defect or not depending on how the map is used, not how it was designed - the map is not the right granularity to be making that call. same thing for arithmetic and overflows, same thing for array accesses etc etc

from nim-libp2p.

One of Nim's design goals is to ensure that the memory cannot be corrupted. The language tries to detect situations where this is about to happen and a Defect exception is raised to abort the program in a controlled way.

You cannot anticipate all consequences of a Defect, but it's quite possible to design stack objects with local invariants that provide enough guarantees to safety execute clean up code during unwinding.

Your problem with the map API is easy to resolve. A Defect is something that violates the contract imposed by the creator of the map type. If the creator of the type requires that all key lookups are performed with existing keys, then the correct usage of the map is to first check for the key existence. Failing to do so is a bug in your usage code that must be fixed.

It's always possible to define such a contract for every API and a defect is just a violation of that contract. If you are not happy with the contract, you can create a different API offering a different contract.

from nim-libp2p.