Use asyncraises in p2p #675
Conversation
| @@ -155,7 +155,7 @@ type | |||
| MessageHandlerDecorator* = proc(msgId: int, n: NimNode): NimNode | |||
|
|
|||
| ThunkProc* = proc(x: Peer, msgId: int, data: Rlp): Future[void] | |||
| {.gcsafe, raises: [RlpError].} | |||
| {.gcsafe, async: (raises: [RlpError, CatchableError]).} | |||
There was a problem hiding this comment.
there's no point adding CatchableError to raises - that's already the default at which point it's generally better to use just plain async
There was a problem hiding this comment.
okay, thanks. But unfortunately it doesn't works. The compiler/chronos still complaints.
There was a problem hiding this comment.
this would be because you need to match usages to the declaration - ie either all use {.async.} or all use {.async: [errors].} - this is related to the guarantees that you want to uphold, ie either the thunk proc is responsible for handling errors or the thing calling it is and one needs to decide.
broadly though, this declaration is non-sensical: RlpError is a subclass of CatchableError so if this declaration is needed, something went wrong "somewhere else" and it would be interesting perhaps to investigate where that was - ie likely, there is some deeper change that will either remove RlpError or remove CatcahbleError from this declaration.
There was a problem hiding this comment.
Yup, the rlpx needs overhaul.
eth/p2p/rlpx.nim
Outdated
| @@ -176,7 +176,7 @@ proc messagePrinter[MsgType](msg: pointer): string {.gcsafe.} = | |||
| # result = $(cast[ptr MsgType](msg)[]) | |||
|
|
|||
| proc disconnect*(peer: Peer, reason: DisconnectionReason, | |||
| notifyOtherPeer = false) {.gcsafe, async.} | |||
| notifyOtherPeer = false) {.gcsafe, async: (raises:[CatchableError]).} | |||
There was a problem hiding this comment.
no need for gcsafe when async is specified
jangko
force-pushed
the
p2p-asyncraises
branch
from
72dafb2 to
f4e1745
Compare
No description provided.