Microsoft: handle sporadic underlying connection was closed, cache webhook_notifications_enabled#458
Conversation
| # First check if we already have cached value since this function is called | ||
| # repeatedly and there is no need to do extra HTTP request every time. | ||
| if self._webhook_notifications_enabled is not None: | ||
| return self._webhook_notifications_enabled |
There was a problem hiding this comment.
Is it possible that this method is called one time for Account A and another time for Account B? If so, this will have issues. If not, should this method not accept the account argument?
There was a problem hiding this comment.
No. It's always the same account object, I know it's weird given you receive account as parameter but that's how it was done in the past for some reason.
It's:
- single greenlet that is responsible for account sync
- that has single events provider for this specific account
- that has single client for this specific account
There was a problem hiding this comment.
I guess this was done like that with Google because they did not want to make repetitive ORM calls in this component, see Google one.
There was a problem hiding this comment.
But I guess the whole thing could be refactored to accept Account instance in the constructor, but it would be probably a little bit problematic given how session management is handled in sync-engine.
There was a problem hiding this comment.
Yea, that would make for sense, I think. Should probably note it as an issue or maybe at least leave a TODO comment.
This takes care of two things:
When I changed
webhook_notifications_enabledto create a dummy subscription and then delete it I did not realize this is called periodically in a loop. We need to do it only once to know if we are on unlucky account, then we can just cache it.We sporadically get
The underlying connection was closed: A connection that was expected to be kept alive was closed by the server.. We get this when subscribing to notifications over webhooks Microsoft makes an initial contact with webhook url. This is safe to retry.