Generic try-with torguard fixed#52
Open
parhamsaremi wants to merge 2 commits intonblockchain:masterfrom
Open
Conversation
Previously, this block had a generic try-with which is dangerous since it can cause unwanted behaviours and cause confusion for future developers.
aarani
reviewed
Nov 21, 2022
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
0f60878 to
ac4e3b8
Compare
aarani
reviewed
Nov 21, 2022
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
ac4e3b8 to
d23285f
Compare
Collaborator
|
LGTM! |
Member
|
2nd commiit's message is quite scarce in wording |
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
d23285f to
1557451
Compare
Author
I updated the message, do you think that this new version is good? |
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
2 times, most recently
from
658cfa9 to
0572f20
Compare
Author
|
@knocte updated the commit message. |
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
0572f20 to
5f2ba14
Compare
Author
|
it seems like I have some problems with commit lint 🤔 but I used ``` and it still gives me error 🤔 |
Member
|
Mmm looks like it's a commitlint bug, we should look into it. |
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
5f2ba14 to
a2b265e
Compare
Author
should I create an issue for this? |
parhamsaremi
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
a2b265e to
186e075
Compare
Member
No, we're gonna investigate it first to make sure it's a commitlint bug. |
Author
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
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
f38390e to
186e075
Compare
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
force-pushed
the
generic-try-catch-torguard-fixed
branch
from
fd68e72 to
a1534d2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove generic try-with and handle errors one-by-one.