Network: clean TorGuard exception handling (Not Tested)

[parhamsaremi]

Move TorGuard exception handling to a new module so that every time we want to handle a new type of exception, we
won't be bothered by adding a lot of try/except expressions.
This code is necessary for fixing [1].

Fixes #45
[1] nblockchain/geewallet#184

[aarani]

Is this all the places we do try-with in TorGuard?

[parhamsaremi]

Is this all the places we do try-with in TorGuard?

Nope, There was another try but it wasn't with the same behavior https://github.com/parhamsaremi/NOnion/blob/geewallet-issue-184/NOnion/Network/TorGuard.fs#L332

[knocte]

https://github.com/parhamsaremi/NOnion/blob/geewallet-issue-184/NOnion/Network/TorGuard.fs#L332

WTF that's a generic catch, let's remove it in a 2nd commit (after removing it, we might get some crashes; but thanks to getting them, we'll be able to bring a more specific try-block later.

[parhamsaremi]

https://github.com/parhamsaremi/NOnion/blob/geewallet-issue-184/NOnion/Network/TorGuard.fs#L332

WTF that's a generic catch, let's remove it in a 2nd commit (after removing it, we might get some crashes; but thanks to getting them, we'll be able to bring a more specific try-block later.

Sounds good. I'll work on this after addressing the previous typos (which won't take long).

[aarani]

https://github.com/parhamsaremi/NOnion/blob/geewallet-issue-184/NOnion/Network/TorGuard.fs#L332

WTF that's a generic catch, let's remove it in a 2nd commit (after removing it, we might get some crashes; but thanks to getting them, we'll be able to bring a more specific try-block later.

Technically, I had a reason for putting a generic catch there.
At the time, any crash there would kill the listening thread without any notice to the end user which was not ideal,
I preferred swallowing exceptions.
Currently, since I added self.KillChildCircuits() the guard announces death to all circuits if a single exception happen so it doesn't matter if the listening thread dies.

we might get some crashes

you won't. exceptions there won't be raised. only exception you might get is CircuitDestroyed-like exceptions when guard dies.

[aarani]

https://github.com/parhamsaremi/NOnion/blob/geewallet-issue-184/NOnion/Network/TorGuard.fs#L332

WTF that's a generic catch, let's remove it in a 2nd commit (after removing it, we might get some crashes; but thanks to getting them, we'll be able to bring a more specific try-block later.

Technically, I had a reason for putting a generic catch there. At the time, any crash there would kill the listening thread without any notice to the end user which was not ideal, I preferred swallowing exceptions. Currently, since I added self.KillChildCircuits() the guard announces death to all circuits if a single exception happen so it doesn't matter if the listening thread dies.

we might get some crashes

you won't. exceptions there won't be raised. only exception you might get is CircuitDestroyed-like exceptions when guard dies.

What I'm most afraid is that I remember getting ObjectDisposedExceptions (I'm not sure if it was here) which I do not like being raised because you can just swallow them (as far as the listening thread cares).

[aarani]

Huh! I have a comment about this:

    // Some circuit handlers send data, which by itself might try to use the stream

    // after it's already disposed, so we need to be able to handle cancellation during cell handling as well

[aarani]

TL/DR: Personally, I prefer keeping that generic catch there.