Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic try-with torguard fixed #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

parhamsaremi
Copy link

Remove generic try-with and handle errors one-by-one.

Previously, this block had a generic try-with which is
dangerous since it can cause unwanted behaviours and
cause confusion for future developers.
NOnion/Network/TorGuard.fs Outdated Show resolved Hide resolved
@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from 0f60878 to ac4e3b8 Compare November 21, 2022 11:44
NOnion/Network/TorGuard.fs Outdated Show resolved Hide resolved
@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from ac4e3b8 to d23285f Compare November 21, 2022 12:30
@aarani
Copy link
Collaborator

aarani commented Nov 22, 2022

LGTM!

@knocte
Copy link
Member

knocte commented Nov 22, 2022

2nd commiit's message is quite scarce in wording

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from d23285f to 1557451 Compare November 23, 2022 10:30
@parhamsaremi
Copy link
Author

2nd commiit's message is quite scarce in wording

I updated the message, do you think that this new version is good?

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch 2 times, most recently from 658cfa9 to 0572f20 Compare November 23, 2022 12:20
@parhamsaremi
Copy link
Author

@knocte updated the commit message.

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from 0572f20 to 5f2ba14 Compare November 23, 2022 12:26
@parhamsaremi
Copy link
Author

it seems like I have some problems with commit lint 🤔 but I used ``` and it still gives me error 🤔

@knocte
Copy link
Member

knocte commented Nov 23, 2022

Mmm looks like it's a commitlint bug, we should look into it.

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from 5f2ba14 to a2b265e Compare November 23, 2022 12:31
@parhamsaremi
Copy link
Author

Mmm looks like it's a commitlint bug, we should look into it.

should I create an issue for this?

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from a2b265e to 186e075 Compare November 23, 2022 12:45
@knocte
Copy link
Member

knocte commented Nov 23, 2022

should I create an issue for this?

No, we're gonna investigate it first to make sure it's a commitlint bug.

@parhamsaremi
Copy link
Author

should I create an issue for this?

No, we're gonna investigate it first to make sure it's a commitlint bug.

Good idea. indeed in my second commit, which I just made, I had a problem pasting the error, so I pasted it line by line. let's see if that'll fix the problem.

@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from f38390e to 186e075 Compare November 23, 2022 13:18
Previously, TorHandshakes used failwith to raise handshake
failed error. Since we need to catch it, as a result of
replacing generic try-with which lead to a red CI in the
PR, we had to define its special excpetion and catch that:

```
The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.Exception: Key handshake failed!

   at NOnion.TorHandshakes.NTorHandshake.NOnion-TorHandshakes-IHandshake-GenerateKdfResult(ICreatedCell serverSideData) in /home/runner/work/NOnion/NOnion/NOnion/TorHandshakes/NTorHandshake.fs:line 112
   at <StartupCode$NOnion>.$TorCircuit.handleIncomingCell@348-1.Invoke(Unit unitVar) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 352
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvoke[T,TResult](AsyncActivation`1 ctxt, TResult result1, FSharpFunc`2 part2) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 398
   at NOnion.Utility.MailboxResultUtil.TryExecuteAsyncAndReplyAsResult@25-3.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Utility/MailboxUtil.fs:line 25
   at NOnion.Utility.MailboxResultUtil.TryExecuteAsyncAndReplyAsResult@24-6.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Utility/MailboxUtil.fs:line 24
   at <StartupCode$NOnion>.$TorCircuit.clo@965-15.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 965
   at <StartupCode$NOnion>.$TorCircuit.clo@950-39.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 950

   at <StartupCode$FSharp-Core>.$Mailbox.processFirstArrival@303-8.Invoke(AsyncActivation`1 ctxt) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\mailbox.fs:line 303

   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 109

--- End of stack trace from previous location where exception was thrown ---

   at Microsoft.FSharp.Control.AsyncPrimitives.Start@907-1.Invoke(ExceptionDispatchInfo edi) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 907

   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 109

   at <StartupCode$FSharp-Core>.$Async.-ctor@163-1.Invoke(Object o) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 165

   at System.Threading.QueueUserWorkItemCallback.Execute()

   at System.Threading.ThreadPoolWorkQueue.Dispatch()
```
@parhamsaremi parhamsaremi force-pushed the generic-try-catch-torguard-fixed branch from fd68e72 to a1534d2 Compare November 24, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants