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

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

Open
wants to merge 1 commit into
base: v6.x.x
Choose a base branch
from

Conversation

matthias-wende-sociomantic
Copy link
Contributor

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).

@matthias-wende-sociomantic matthias-wende-sociomantic changed the base branch from v5.x.x to v6.x.x April 11, 2019 16:15
@gavin-norman-sociomantic

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

@matthias-wende-sociomantic matthias-wende-sociomantic changed the base branch from v6.x.x to v5.x.x April 24, 2019 15:08
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).
@matthias-wende-sociomantic
Copy link
Contributor Author

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

@liza-churkyn-sociomantic

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...

@Geod24 Geod24 changed the base branch from v5.x.x to v6.x.x August 13, 2020 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants