-
Notifications
You must be signed in to change notification settings - Fork 456
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
Fix: edgeAgent creates rogue ModuleClients in case of error #5332
Conversation
…multiple clients trying to connect with same identity
// swallowing intentionally | ||
} | ||
|
||
throw; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would this get caught/handled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the main idea of the rethrow was to not change the existing error handling, but
- this is called from an OnClosed() event handler, and it catches and swallows the exception
- it is called by 'GetTwinFromIotHub' which catches it and either retries or just swallows the exception (retries once)
- called from the ModuleClient ctor - there the idea I suppose that the module client subscribes to events (method handler and desired properties) plus when the connection gets closed, it reopens. I think this latter logic might be broken as I see, because started offline, the method will throw an exception and I don't see how it would automatically create a new connection anytime soon (it will when some periodic operation, like twin pull requests a new connection) - but again, with this change the logic stays the same.
added a ticket for this last question: (10555951)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good. Can we add a test for it?
causing multiple ModuleClients trying to connect with same identity. When edgeAgent creates a new instance of module client, it sets up subscriptions. If that failed (e.g in case of network problem), then the newly created ModuleClient was left there without referencing it and nothing closed it afterwards. Later edgeAgent needed to create a new module client again (as the previous attempt failed). Thus at least two active ModuleClient was created with the same connection string, both trying to connect iothub. When this happened, they started to fight over the connection - iothub always disconnected the previously connected ModuleClient. Because edgeAgent pulls a twin at every connection, the connection fight mentioned above caused pulling several twins in every second.
causing multiple ModuleClients trying to connect with same identity.
When edgeAgent creates a new instance of module client, it sets up subscriptions. If that failed (e.g in case of network problem), then the newly created ModuleClient was left there without referencing it and nothing closed it afterwards. Later edgeAgent needed to create a new module client again (as the previous attempt failed). Thus at least two active ModuleClient was created with the same connection string, both trying to connect iothub. When this happened, they started to fight over the connection - iothub always disconnected the previously connected ModuleClient.
Because edgeAgent pulls a twin at every connection, the connection fight mentioned above caused pulling several twins in every second.