changeset 3244:b10d207f95f9

core (xmpp): properly clean profile data in case of startConnection failure: - profile is now always removed from profiles_connecting - an exception is raised if client already exists only if client is connected. Otherwise, it's deleted (this allow to keep session open to e.g. modify settings in case of failure during connection)
author Goffi <goffi@goffi.org>
date Wed, 01 Apr 2020 16:17:09 +0200
parents f2e30aa031e9
children 2a0a16b906ac
files sat/core/xmpp.py
diffstat 1 files changed, 89 insertions(+), 83 deletions(-) [+]
line wrap: on
line diff
--- a/sat/core/xmpp.py	Wed Apr 01 15:40:29 2020 +0200
+++ b/sat/core/xmpp.py	Wed Apr 01 16:17:09 2020 +0200
@@ -163,103 +163,109 @@
             raise exceptions.CancelError(f"{profile} is already being connected")
         cls.profiles_connecting.add(profile)
         try:
-            port = int(
-                host.memory.getParamA(
-                    C.FORCE_PORT_PARAM, "Connection", profile_key=profile
+            try:
+                port = int(
+                    host.memory.getParamA(
+                        C.FORCE_PORT_PARAM, "Connection", profile_key=profile
+                    )
                 )
-            )
-        except ValueError:
-            log.debug(_("Can't parse port value, using default value"))
-            port = (
-                None
-            )  # will use default value 5222 or be retrieved from a DNS SRV record
-
-        password = await host.memory.asyncGetParamA(
-            "Password", "Connection", profile_key=profile
-        )
-
-        entity_jid_s = await host.memory.asyncGetParamA(
-            "JabberID", "Connection", profile_key=profile)
-        entity_jid = jid.JID(entity_jid_s)
+            except ValueError:
+                log.debug(_("Can't parse port value, using default value"))
+                port = (
+                    None
+                )  # will use default value 5222 or be retrieved from a DNS SRV record
 
-        if not entity_jid.resource and not cls.is_component and entity_jid.user:
-            # if no resource is specified, we create our own instead of using
-            # server returned one, as it will then stay stable in case of reconnection.
-            # we only do that for client and if there is a user part, to let server
-            # decide for anonymous login
-            resource_dict = await host.memory.storage.getPrivates(
-                "core:xmpp", ["resource"] , profile=profile)
-            try:
-                resource = resource_dict["resource"]
-            except KeyError:
-                resource = f"{C.APP_NAME_FILE}.{shortuuid.uuid()}"
-                await host.memory.storage.setPrivateValue(
-                    "core:xmpp", "resource", resource, profile=profile)
-
-            log.info(_("We'll use the stable resource {resource}").format(
-                resource=resource))
-            entity_jid.resource = resource
-
-        if profile in host.profiles:
-            raise exceptions.InternalError(
-                f"There is already a profile of name {profile} in host")
-        entity = host.profiles[profile] = cls(
-            host, profile, entity_jid, password,
-            host.memory.getParamA(C.FORCE_SERVER_PARAM, "Connection",
-                                  profile_key=profile) or None,
-            port, max_retries,
+            password = await host.memory.asyncGetParamA(
+                "Password", "Connection", profile_key=profile
             )
 
-        await entity.encryption.loadSessions()
-
-        entity._createSubProtocols()
+            entity_jid_s = await host.memory.asyncGetParamA(
+                "JabberID", "Connection", profile_key=profile)
+            entity_jid = jid.JID(entity_jid_s)
 
-        entity.fallBack = SatFallbackHandler(host)
-        entity.fallBack.setHandlerParent(entity)
+            if not entity_jid.resource and not cls.is_component and entity_jid.user:
+                # if no resource is specified, we create our own instead of using
+                # server returned one, as it will then stay stable in case of
+                # reconnection. we only do that for client and if there is a user part, to
+                # let server decide for anonymous login
+                resource_dict = await host.memory.storage.getPrivates(
+                    "core:xmpp", ["resource"] , profile=profile)
+                try:
+                    resource = resource_dict["resource"]
+                except KeyError:
+                    resource = f"{C.APP_NAME_FILE}.{shortuuid.uuid()}"
+                    await host.memory.storage.setPrivateValue(
+                        "core:xmpp", "resource", resource, profile=profile)
 
-        entity.versionHandler = SatVersionHandler(C.APP_NAME_FULL, host.full_version)
-        entity.versionHandler.setHandlerParent(entity)
+                log.info(_("We'll use the stable resource {resource}").format(
+                    resource=resource))
+                entity_jid.resource = resource
 
-        entity.identityHandler = SatIdentityHandler()
-        entity.identityHandler.setHandlerParent(entity)
-
-        log.debug(_("setting plugins parents"))
+            if profile in host.profiles:
+                if host.profiles[profile].isConnected():
+                    raise exceptions.InternalError(
+                        f"There is already a connected profile of name {profile!r} in "
+                        f"host")
+                log.debug(
+                    "removing unconnected profile {profile!r}")
+                del host.profiles[profile]
+            entity = host.profiles[profile] = cls(
+                host, profile, entity_jid, password,
+                host.memory.getParamA(C.FORCE_SERVER_PARAM, "Connection",
+                                      profile_key=profile) or None,
+                port, max_retries,
+                )
 
-        plugin_conn_cb = await entity._callConnectionTriggers()
-
-        entity.startService()
+            await entity.encryption.loadSessions()
 
-        await entity.conn_deferred
+            entity._createSubProtocols()
 
-        await defer.maybeDeferred(entity.entityConnected)
+            entity.fallBack = SatFallbackHandler(host)
+            entity.fallBack.setHandlerParent(entity)
 
-        # Call profileConnected callback for all plugins,
-        # and print error message if any of them fails
-        conn_cb_list = []
-        for __, callback in plugin_conn_cb:
-            conn_cb_list.append(utils.asDeferred(callback, entity))
-        list_d = defer.DeferredList(conn_cb_list)
+            entity.versionHandler = SatVersionHandler(C.APP_NAME_FULL, host.full_version)
+            entity.versionHandler.setHandlerParent(entity)
+
+            entity.identityHandler = SatIdentityHandler()
+            entity.identityHandler.setHandlerParent(entity)
+
+            log.debug(_("setting plugins parents"))
+
+            plugin_conn_cb = await entity._callConnectionTriggers()
+
+            entity.startService()
+
+            await entity.conn_deferred
+
+            await defer.maybeDeferred(entity.entityConnected)
 
-        def logPluginResults(results):
-            all_succeed = all([success for success, result in results])
-            if not all_succeed:
-                log.error(_("Plugins initialisation error"))
-                for idx, (success, result) in enumerate(results):
-                    if not success:
-                        log.error(
-                            "error (plugin %(name)s): %(failure)s"
-                            % {
-                                "name": plugin_conn_cb[idx][0]._info["import_name"],
-                                "failure": result,
-                            }
-                        )
+            # Call profileConnected callback for all plugins,
+            # and print error message if any of them fails
+            conn_cb_list = []
+            for __, callback in plugin_conn_cb:
+                conn_cb_list.append(utils.asDeferred(callback, entity))
+            list_d = defer.DeferredList(conn_cb_list)
 
-        await list_d.addCallback(
-            logPluginResults
-        )  # FIXME: we should have a timeout here, and a way to know if a plugin freeze
-        # TODO: mesure launch time of each plugin
+            def logPluginResults(results):
+                all_succeed = all([success for success, result in results])
+                if not all_succeed:
+                    log.error(_("Plugins initialisation error"))
+                    for idx, (success, result) in enumerate(results):
+                        if not success:
+                            log.error(
+                                "error (plugin %(name)s): %(failure)s"
+                                % {
+                                    "name": plugin_conn_cb[idx][0]._info["import_name"],
+                                    "failure": result,
+                                }
+                            )
 
-        cls.profiles_connecting.remove(profile)
+            await list_d.addCallback(
+                logPluginResults
+            )  # FIXME: we should have a timeout here, and a way to know if a plugin freeze
+            # TODO: mesure launch time of each plugin
+        finally:
+            cls.profiles_connecting.remove(profile)
 
     def _disconnectionCb(self, __):
         self._connected_d = None