chore(protocol): list raised exceptions in handlers #1250
Conversation
| proc handler(conn: Connection, proto: string) {.async: (raises: [CancelledError]).} = | ||
| ## main protocol handler that gets triggered on every | ||
| ## connection for a protocol string | ||
| ## e.g. ``/floodsub/1.0.0``, etc... |
There was a problem hiding this comment.
here CancelledError has to be raised because it is handler, but this handler is catching this exception.
There was a problem hiding this comment.
it might be a good idea to review whether the handler should catch the cancellation or let it propagate - much of this code was written as cancellation was being developed and it might not always be following best practice .. catching the cancellation and translating it to some other outcome is sometimes the right thing, but often not (it "hides" the fact that a cancellation is ongoing from the calling code which deprives it of the means to make the distinction).
There was a problem hiding this comment.
hey @arnetheduck, yes, it makes much more sense to catch cancellation. this effort is in #1260
|
Looks good so far. CI seems to be complaining tho. |
vladopajic
changed the title
|
closing this PR in favor of #1260 |
part of #962 effort.