Merge pull request #2280 from AppDaemon/ast-parsing

Failing app fixes

Author: acockburn

appdaemon/app_management.py

@@ -615,7 +615,7 @@ async def check_app_config_files(self, update_actions: UpdateActions):
                     self._compare_sequences(update_actions, cfg, files_to_read)
                     continue
 
-                if name in self.non_apps:
+                if name in self.non_apps or cfg.disable:
                     continue
 
                 # New config found
@@ -685,8 +685,14 @@ def import_module(self, module_name: str) -> int:
                 if not module_name.startswith("appdaemon"):
                     self.logger.debug("Importing '%s'", module_name)
                     importlib.import_module(module_name)
-        except SyntaxError as exc:
-            path = Path(exc.filename)
+        except Exception as exc:
+            match exc:
+                case SyntaxError():
+                    path = Path(exc.filename)
+                case NameError():
+                    path = Path(traceback.extract_tb(exc.__traceback__)[-1].filename)
+                case _:
+                    raise exc
             mtime = self.dependency_manager.python_deps.files.mtimes.get(path)
             self.dependency_manager.python_deps.bad_files.add((path, mtime))
             raise exc
@@ -989,13 +995,16 @@ async def _stop_apps(self, update_actions: UpdateActions):
             update_actions.apps.reload -= failed_to_stop
 
     async def _start_apps(self, update_actions: UpdateActions):
+        if failed := update_actions.apps.failed:
+            self.logger.warning('Failed to start apps: %s', failed)
+
         start_order = update_actions.apps.start_sort(self.dependency_manager)
         if start_order:
             self.logger.info("Starting apps: %s", update_actions.apps.init_set)
             self.logger.debug("App start order: %s", start_order)
 
             for app_name in start_order:
-                if isinstance((cfg := self.app_config.root[app_name]), AppConfig):
+                if isinstance((cfg := self.app_config.root[app_name]), AppConfig) and not cfg.disable:
                     @ade.wrap_async(self.error, self.AD.app_dir, f"'{app_name}' instantiation")
                     async def safe_create(self: "AppManagement"):
                         try:

appdaemon/dependency.py

@@ -92,6 +92,7 @@ def gen_modules() -> Generator[str, None, None]:
             mod: ast.Module = ast.parse(file_content, filename=file_path)
         except Exception as e:
             logger.warning(f"Error parsing python module with AST: {e}")
+            raise e
         else:
             for node in get_imports(mod):
                 match node:
@@ -107,18 +108,32 @@ def gen_modules() -> Generator[str, None, None]:
     return set(gen_modules())
 
 
-def get_dependency_graph(files: Iterable[Path], exclude: set[Path] | None = None):
-    graph = {
-        get_full_module_name(f): get_file_deps(f)
-        for f in files
-        if exclude is None or f not in exclude
-    }
+def get_dependency_graph(
+    files: Iterable[Path],
+    exclude: set[Path] | None = None
+) -> tuple[dict[str, set[str]], set[Path]]:
+    """Gets the dependency graph for some Python files.
+
+    Returns:
+        A tuple containing:
+        - A dictionary where keys are module names and values are sets of module names that the key module depends on.
+        - A set of paths that failed to parse or resolve dependencies.
+    """
+    graph = {}
+    failed = set()
+    for f in files:
+        if exclude is None or f not in exclude:
+            try:
+                graph[get_full_module_name(f)] = get_file_deps(f)
+            except Exception:
+                failed.add(f)
+                continue
 
     for mod, deps in graph.items():
         if mod in deps:
             deps.remove(mod)
 
-    return graph
+    return graph, failed
 
 
 def get_all_nodes(deps: dict[str, set[str]]) -> set[str]:

appdaemon/dependency_manager.py

@@ -1,4 +1,5 @@
 from abc import ABC
+from copy import deepcopy
 from dataclasses import InitVar, dataclass, field
 from pathlib import Path
 from typing import Iterable
@@ -16,14 +17,14 @@ class Dependencies(ABC):
     ext: str = field(init=False)  # this has to be defined by the children classes
     dep_graph: dict[str, set[str]] = field(init=False)
     rev_graph: dict[str, set[str]] = field(init=False)
-    bad_files: set[Path] = field(default_factory=set, init=False)
+    bad_files: set[tuple[Path, float]] = field(default_factory=set, init=False)
 
     def __post_init__(self):
         self.refresh_dep_graph()
 
     def update(self, new_files: Iterable[Path]):
         self.files.update(new_files)
-        for bf, mtime in self.bad_files:
+        for bf, mtime in deepcopy(self.bad_files):
             new_mtime = self.files.mtimes.get(bf)
             if new_mtime != mtime:
                 assert new_mtime > mtime, f"File {bf} was modified in the future"
@@ -65,7 +66,12 @@ def refresh_dep_graph(self):
         if self.bad_files:
             bad_files, _ = zip(*self.bad_files)
             bad_files = set(bad_files)
-        self.dep_graph = get_dependency_graph(self.files, exclude=bad_files)
+
+        self.dep_graph, failed = get_dependency_graph(self.files, exclude=bad_files)
+
+        for file in failed:
+            self.bad_files.add((file, self.files.mtimes.get(file, 0)))
+
         self.rev_graph = reverse_graph(self.dep_graph)
 
     def modules_to_import(self) -> set[str]:

appdaemon/exceptions.py

@@ -54,9 +54,10 @@ def user_exception_block(logger: Logger, exception: AppDaemonException, app_dir:
     """Function to generate a user-friendly block of text for an exception. Gets the whole chain of exception causes to decide what to do.
     """
     width = 75
+    spacing = 4
     inset = 5
     if header is not None:
-        header = f'{"=" * inset}  {header}  {"=" * (width - inset - len(header))}'
+        header = f'{"=" * inset}  {header}  {"=" * (width - spacing - inset - len(header))}'
     else:
         header = '=' * width
     logger.error(header)
@@ -86,10 +87,12 @@ def user_exception_block(logger: Logger, exception: AppDaemonException, app_dir:
                         logger.error(f'{indent}{filename} line {line} in {func_name}')
             case OSError() if str(exc).endswith('address already in use'):
                 logger.error(f'{indent}{exc.__class__.__name__}: {exc}')
-            case NameError():
+            case NameError() | ImportError():
                 logger.error(f'{indent}{exc.__class__.__name__}: {exc}')
                 if tb := traceback.extract_tb(exc.__traceback__):
                     frame = tb[-1]
+                    file = Path(frame.filename).relative_to(app_dir.parent)
+                    logger.error(f'{indent}  line {frame.lineno} in {file.name}')
                     logger.error(f'{indent}  {frame._line.rstrip()}')
                     error_len = frame.end_colno - frame.colno
                     logger.error(f'{indent}  {" " * (frame.colno - 1)}{"^" * error_len}')
@@ -112,7 +115,7 @@ def user_exception_block(logger: Logger, exception: AppDaemonException, app_dir:
                     for line in lines:
                         logger.error(f'{indent}{line}')
 
-    logger.error('=' * 75)
+    logger.error('=' * width)
 
 
 def unexpected_block(logger: Logger, exception: Exception):

appdaemon/models/internal/app_management.py

@@ -71,7 +71,7 @@ def start_sort(self, dm: DependencyManager, logger: Logger = None) -> list[str]:
         Uses a dependency graph to sort the internal ``init`` and ``reload`` sets together
         """
         items = copy(self.init_set)
-        items |= find_all_dependents(items, dm.app_deps.rev_graph)
+        items |= find_all_dependents(items, dm.app_deps.dep_graph)
         priorities = {
             app_name: dm.app_deps.app_config.root[app_name].priority
             for app_name in items

docs/HISTORY.md

@@ -9,6 +9,13 @@ None
 **Fixes**
 
 - Revert unintentional sync vs async changes
+- Respecting the `disable` key in app configurations when
+    - processing module import order
+    - processing app start order
+- Fixed a bug where a failed app could get re-introduced to the start order only to fail again
+- Fixed a bug where changing files at exactly right time would throw an error
+- Files with syntax errors get added to `bad_files` list, even if they aren't used for apps
+- Improved error text for `ImportErrors` and `SyntaxErrors` in user apps
 - Add a fix to the http component to handle URLs with no port, and URLs with port 80
 
 **Breaking Changes**