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

chore(async): {.async: (raises).} for connmanager #1172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lchenut
Copy link
Contributor

@lchenut lchenut commented Aug 9, 2024

Add {.async: (raises).} to connmanager.nim and to semaphore.nim (which is needed by connmanager.nim)

@lchenut lchenut changed the title feat(async): {.async: (raises).} for connmanager chore(async): {.async: (raises).} for connmanager Aug 26, 2024
@@ -67,7 +67,8 @@ type
muxed: Table[PeerId, seq[Muxer]]
connEvents: array[ConnEventKind, OrderedSet[ConnEventHandler]]
peerEvents: array[PeerEventKind, OrderedSet[PeerEventHandler]]
expectedConnectionsOverLimit*: Table[(PeerId, Direction), Future[Muxer]]
expectedConnectionsOverLimit*:
Table[(PeerId, Direction), Future[Muxer].Raising([CancelledError])]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the difference between this and using a Future[Result[Muxer, ...]].

Actually, I guess the broader question would be: Is there a reason we're going for exceptions over handling errors with Result?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert, but Nim natively supports exceptions and there's also compiler support for pros signatures https://nim-lang.org/docs/tut2.html#exceptions-annotating-procs-with-raised-exceptions. Resultwas created in some status library. It might be painful to use Resultif Nim std wasn't designed for that. I can't say much as in general, this project uses more exceptions than Result.

## Acquire a resource and decrement the resource
## counter. If no more resources are available,
## the returned future will not complete until
## the resource count goes above 0.
##

let fut = newFuture[void]("AsyncSemaphore.acquire")
let fut = Future[void].Raising([CancelledError]).init("AsyncSemaphore.acquire")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can create a new template in utils/future.nim:

 template newFuture*[T](fromProc: static[string] = ""): auto =
  Future[T].Raising([CancelledError]).init(fromProc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Pipeline
Development

Successfully merging this pull request may close these issues.

3 participants