sociomantic-tsunami / swarm

Asynchronous client/node framework library
Boost Software License 1.0
14 stars 26 forks source link

Add new connection error notifier and error field to union #392

Open matthias-wende-sociomantic opened 5 years ago

matthias-wende-sociomantic commented 5 years ago

The user provided connection notifier was called on startup only in case of an successfully established connection or if an error happened while connecting. This patch adds a new (internal) client connection error notifier that is called from the ISelectClient error method, in that case the client Connection class. That happens when the ISelectClient is unregistered from epoll due to a exception thrown in the RequestOnConn fiber.

The exception is passed to the user by utilizing a new error field in the ConnNotificationUnion (defined in the ConnectionSet).

gavin-norman-sociomantic commented 5 years ago

Blocked on testing to confirm that the theory works in practice.

matthias-wende-sociomantic commented 5 years ago

Changed base branch for testing. The actual patch should be merged into v6.x.x (i.e. the next major) though.

liza-churkyn-sociomantic commented 5 years ago

Tested with my POC UpdateRequest app, specifically calling a blocking request from within the notifier of another blocking request. Doing that is a bug (as it leads to a wrong cast in the Task in the 2nd blocking request) and causes an error, and the thought behind this PR was to notify the client that an error has occurred by calling the Connection notifier with an error. But, the notifier is not called, and we don't reach this line: https://github.com/sociomantic-tsunami/swarm/pull/392/files#diff-3fab881125f1bae0cbb5e9bd40b76b05R308.

Could it be that its because this is not an exception, but an error, and is not caught by the base code? Just speculating here...