Fixes MATLAB stop logic on startup timeout by removing redundant log check.

Fixes #38

Author: diningPhilosopher64

gui/package-lock.json

@@ -58,13 +58,13 @@ def __init__(self, settings):
         self.settings = settings
         self.processes = {"matlab": None, "xvfb": None}
 
-        # Timeout for processes launched by matlab-proxy
+        # Timeout for processes started by matlab-proxy
         self.PROCESS_TIMEOUT = get_process_startup_timeout()
 
-        # The port on which MATLAB(launched by this matlab-proxy process) starts on.
+        # The port on which MATLAB(started by this matlab-proxy process) starts on.
         self.matlab_port = None
 
-        # The directory in which the instance of MATLAB (launched by this matlab-proxy process) will write logs to.
+        # The directory in which the instance of MATLAB (started by this matlab-proxy process) will write logs to.
         self.mwi_logs_dir = None
 
         # Dictionary of all files used to manage the MATLAB session.
@@ -99,7 +99,7 @@ def __init__(self, settings):
         self.embedded_connector_start_time = None
 
         # Keep track of the state of the Embedded Connector.
-        # If there is some problem with launching the Embedded Connector(say an issue with licensing),
+        # If there is some problem with starting the Embedded Connector(say an issue with licensing),
         # the state of MATLAB process in app_state will continue to be in a 'starting' indefinitely.
         # This variable can be either "up" or "down"
         self.embedded_connector_state = "down"
@@ -222,15 +222,15 @@ def __get_cached_config_file(self):
     def __delete_cached_config_file(self):
         """Deletes the cached config file"""
         try:
-            logger.info(f"Deleting any cached config files!")
+            logger.debug(f"Deleting any cached config files!")
             os.remove(self.__get_cached_config_file())
         except FileNotFoundError:
             # The file being absent is acceptable.
             pass
 
     def __reset_and_delete_cached_config(self):
         """Reset licensing variable of the class and removes the cached config file."""
-        logger.info(f"Resetting cached config information...")
+        logger.debug(f"Resetting cached config information...")
         self.licensing = None
         self.__delete_cached_config_file()
 
@@ -259,14 +259,14 @@ async def init_licensing(self):
         # Default value
         self.licensing = None
 
-        # If MWI_USE_EXISTING_LICENSE is set in environment, try launching MATLAB directly
+        # If MWI_USE_EXISTING_LICENSE is set in environment, try starting MATLAB directly
         if self.settings["mwi_use_existing_license"]:
             self.licensing = {"type": "existing_license"}
             logger.debug(
                 f"{mwi_env.get_env_name_mwi_use_existing_license()} variable set in environment"
             )
             logger.info(
-                f"!!! Launching MATLAB without providing any additional licensing information. This requires MATLAB to have been activated on the machine from which its being launched !!!"
+                f"!!! Starting MATLAB without providing any additional licensing information. This requires MATLAB to have been activated on the machine from which its being started !!!"
             )
 
             # Delete old config info from cache to ensure its wiped out first before persisting new info.
@@ -276,7 +276,7 @@ async def init_licensing(self):
         elif self.settings.get("nlm_conn_str", None) is not None:
             nlm_licensing_str = self.settings.get("nlm_conn_str")
             logger.debug(f"Found NLM:[{nlm_licensing_str}] set in environment")
-            logger.debug(f"Using NLM string to connect ... ")
+            logger.info(f"Using NLM:{nlm_licensing_str} to connect...")
             self.licensing = {
                 "type": "nlm",
                 "conn_str": nlm_licensing_str,
@@ -307,7 +307,7 @@ async def init_licensing(self):
                             "type": "nlm",
                             "conn_str": licensing["conn_str"],
                         }
-                        logger.info("Using cached NLM licensing to launch MATLAB")
+                        logger.debug("Using cached NLM licensing to start MATLAB")
 
                     elif licensing["type"] == "mhlm":
                         self.licensing = {
@@ -335,12 +335,12 @@ async def init_licensing(self):
                             )
                             if successful_update:
                                 logger.debug(
-                                    "Using cached Online Licensing to launch MATLAB."
+                                    "Using cached Online Licensing to start MATLAB."
                                 )
                         else:
                             self.__reset_and_delete_cached_config()
                     elif licensing["type"] == "existing_license":
-                        logger.info("Using cached existing license to launch MATLAB")
+                        logger.debug("Using cached existing license to start MATLAB")
                         self.licensing = licensing
                     else:
                         # Somethings wrong, licensing is neither NLM or MHLM
@@ -920,14 +920,14 @@ def clean_up_mwi_server_session(self):
         try:
             for session_file in self.mwi_server_session_files.items():
                 if session_file[1] is not None:
-                    logger.info(f"Deleting:{session_file[1]}")
+                    logger.debug(f"Deleting:{session_file[1]}")
                     session_file[1].unlink()
         except FileNotFoundError:
             # Files may not exist if cleanup is called before they are created
             pass
 
     async def __setup_env_for_matlab(self) -> dict:
-        """Configure the environment variables required for launching MATLAB by matlab-proxy.
+        """Configure the environment variables required for starting MATLAB by matlab-proxy.
 
         Returns:
             [dict]: Containing keys as the Env variable names and values are its corresponding values.
@@ -979,17 +979,17 @@ async def __setup_env_for_matlab(self) -> dict:
         if system.is_linux():
             if self.settings.get("matlab_display", None):
                 matlab_env["DISPLAY"] = self.settings["matlab_display"]
-                logger.info(
-                    f"Using the display number supplied by Xvfb process'{matlab_env['DISPLAY']}' for launching MATLAB"
+                logger.debug(
+                    f"Using the display number supplied by Xvfb process'{matlab_env['DISPLAY']}' for starting MATLAB"
                 )
             else:
                 if "DISPLAY" in matlab_env:
-                    logger.info(
-                        f"Using the existing DISPLAY environment variable with value:{matlab_env['DISPLAY']} for launching MATLAB"
+                    logger.debug(
+                        f"Using the existing DISPLAY environment variable with value:{matlab_env['DISPLAY']} for starting MATLAB"
                     )
                 else:
-                    logger.info(
-                        "No DISPLAY environment variable found. Launching MATLAB without it."
+                    logger.debug(
+                        "No DISPLAY environment variable found. Starting MATLAB without it."
                     )
 
         # The matlab ready file is written into this location(self.mwi_logs_dir) by MATLAB
@@ -1080,11 +1080,11 @@ async def __start_xvfb_process(self):
 
             return xvfb
 
-        # If something went wrong ie. exception is raised in launching Xvfb process, capture error for logging
+        # If something went wrong ie. exception is raised in starting Xvfb process, capture error for logging
         # and for showing the error on the frontend.
 
         # FileNotFoundError: is thrown if Xvfb is not found on System Path.
-        # XvfbError: is thrown if something went wrong when launching Xvfb process.
+        # XvfbError: is thrown if something went wrong when starting Xvfb process.
         except (FileNotFoundError, XvfbError) as err:
             self.error = XvfbError(
                 """Unable to start the Xvfb process. Ensure Xvfb is installed and is available on the System Path. See https://github.com/mathworks/matlab-proxy#requirements for information on Xvfb"""
@@ -1178,7 +1178,7 @@ async def __track_embedded_connector_state(self):
                 else:
                     time_diff = time.time() - self.embedded_connector_start_time
                     if time_diff > self.PROCESS_TIMEOUT:
-                        # Since max allowed startup time has elapsed, it means that MATLAB is in a stuck state and cannot be launched.
+                        # Since max allowed startup time has elapsed, it means that MATLAB is stuck and is unable to start.
                         # Set the error and stop matlab.
                         user_visible_error = "Unable to start MATLAB.\nTry again by clicking Start MATLAB."
 
@@ -1187,32 +1187,27 @@ async def __track_embedded_connector_state(self):
                             # So, raise a generic error wherever appropriate
                             generic_error = f"MATLAB did not start in {int(self.PROCESS_TIMEOUT)} seconds. Use Windows Remote Desktop to check for any errors."
                             logger.error(f":{this_task}: {generic_error}")
-                            if len(self.logs["matlab"]) == 0:
-                                await self.__force_stop_matlab(
-                                    user_visible_error, this_task
-                                )
-                                # Breaking out of the loop to end this task as matlab-proxy was unable to launch MATLAB successfully
-                                # even after waiting for self.PROCESS_TIMEOUT
-                                break
-                            else:
-                                # Do not stop the MATLAB process or break from the loop (the error type is unknown)
-                                self.error = MatlabError(generic_error)
-                                await asyncio.sleep(5)
-                                continue
+
+                            # Stopping the MATLAB process would remove the UI window displaying the error too.
+                            # Do not stop the MATLAB or break from the loop (as the error is still unknown)
+                            self.error = MatlabError(generic_error)
+                            await asyncio.sleep(5)
+                            continue
 
                         else:
-                            # If there are no logs after the max startup time has elapsed, it means that MATLAB is in a stuck state and cannot be launched.
+                            # If there are no logs after the max startup time has elapsed, it means that MATLAB is stuck and is unable to start.
                             # Set the error and stop matlab.
                             logger.error(
                                 f":{this_task}: MATLAB did not start in {int(self.PROCESS_TIMEOUT)} seconds!"
                             )
-                            if len(self.logs["matlab"]) == 0:
-                                await self.__force_stop_matlab(
-                                    user_visible_error, this_task
-                                )
-                                # Breaking out of the loop to end this task as matlab-proxy was unable to launch MATLAB successfully
-                                # even after waiting for self.PROCESS_TIMEOUT
-                                break
+                            # MATLAB can be stopped on posix systems because the stderr pipe of the MATLAB process is
+                            # read (by __matlab_stderr_reader_posix() task) and is logged by matlab-proxy appropriately.
+                            await self.__force_stop_matlab(
+                                user_visible_error, this_task
+                            )
+                            # Breaking out of the loop to end this task as matlab-proxy was unable to start MATLAB successfully
+                            # even after waiting for self.PROCESS_TIMEOUT
+                            break
 
                     else:
                         logger.debug(
@@ -1410,7 +1405,7 @@ async def stop_matlab(self, force_quit=False):
             if session_file_path is not None:
                 self.matlab_session_files[session_file_name] = None
                 with contextlib.suppress(FileNotFoundError):
-                    logger.info(f"Deleting:{session_file_path}")
+                    logger.debug(f"Deleting:{session_file_path}")
                     session_file_path.unlink()
 
         # In posix systems, variable matlab is an instance of asyncio.subprocess.Process()
@@ -1496,7 +1491,7 @@ async def stop_matlab(self, force_quit=False):
         if system.is_posix():
             xvfb = self.processes["xvfb"]
             if xvfb is not None and xvfb.returncode is None:
-                logger.info(f"Terminating Xvfb (PID={xvfb.pid})")
+                logger.debug(f"Terminating Xvfb (PID={xvfb.pid})")
                 xvfb.terminate()
                 waiters.append(xvfb.wait())
 

matlab_proxy/app_state.py

@@ -103,7 +103,7 @@ def get_matlab_executable_and_root_path():
     # Note, error messages are formatted as multi-line strings and the front end displays them as is.
     error_message = "Unable to find MATLAB on the system PATH. Add MATLAB to the system PATH, and restart matlab-proxy."
 
-    logger.info(error_message)
+    logger.error(error_message)
     raise MatlabInstallError(error_message)
 
 

matlab_proxy/settings.py

@@ -98,7 +98,7 @@ def prepare_site(app, runner):
                 )
                 break
             except:
-                logger.info(f"Failed to launch the site on port {p}")
+                logger.error(f"Failed to launch the site on port {p}")
 
     return site
 

matlab_proxy/util/__init__.py

@@ -326,7 +326,7 @@ def validate_matlab_root_path(matlab_root: Path, is_custom_matlab_root: bool):
 
     try:
         __validate_if_paths_exist([matlab_root])
-        logger.info(
+        logger.debug(
             f"MATLAB root path: {matlab_root} exists, continuing to verify its validity..."
         )