From 3a516d6d5807aef629d7c4a46fafa059604377e2 Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Thu, 19 Mar 2026 15:17:12 +0300 Subject: [PATCH] Fix runtime issues found during live testing - Fix jinja2.sandbox import: use `from jinja2.sandbox import SandboxedEnvironment` (dotted attribute access doesn't work) in templates.py, sync.py, and notifier.py - Fix greenlet crash in tracker trigger: SQLAlchemy async sessions can't survive across aiohttp.ClientSession context managers. Eagerly load all tracker/server data before entering HTTP context. Split check_tracker into check_tracker (scheduler, own session) and check_tracker_with_session (API, reuses route session). - Fix _check_album to accept pre-loaded params instead of tracker object (avoids lazy-load access after greenlet context break) Tested end-to-end against live Immich server: - Server connection + album browsing: OK (39 albums) - Template creation + preview: OK - Webhook target creation: OK - Tracker creation + trigger: OK (initialized 4 assets) - Second trigger: OK (no_changes detected) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/immich_watcher_server/api/sync.py | 3 +- .../immich_watcher_server/api/templates.py | 3 +- .../src/immich_watcher_server/api/trackers.py | 5 +- .../services/notifier.py | 3 +- .../immich_watcher_server/services/watcher.py | 80 ++++++++++++------ test-data/immich_watcher.db | Bin 0 -> 36864 bytes 6 files changed, 60 insertions(+), 34 deletions(-) create mode 100644 test-data/immich_watcher.db diff --git a/packages/server/src/immich_watcher_server/api/sync.py b/packages/server/src/immich_watcher_server/api/sync.py index 6395a53..0ac2688 100644 --- a/packages/server/src/immich_watcher_server/api/sync.py +++ b/packages/server/src/immich_watcher_server/api/sync.py @@ -6,6 +6,7 @@ from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession import jinja2 +from jinja2.sandbox import SandboxedEnvironment from ..database.engine import get_session from ..database.models import ( @@ -136,7 +137,7 @@ async def render_template( raise HTTPException(status_code=404, detail="Template not found") try: - env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) + env = SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template.body) rendered = tmpl.render(**body.context) return {"rendered": rendered} diff --git a/packages/server/src/immich_watcher_server/api/templates.py b/packages/server/src/immich_watcher_server/api/templates.py index e58ac79..6895ce7 100644 --- a/packages/server/src/immich_watcher_server/api/templates.py +++ b/packages/server/src/immich_watcher_server/api/templates.py @@ -6,6 +6,7 @@ from sqlmodel import select from sqlmodel.ext.asyncio.session import AsyncSession import jinja2 +from jinja2.sandbox import SandboxedEnvironment from ..auth.dependencies import get_current_user from ..database.engine import get_session @@ -124,7 +125,7 @@ async def preview_template( """Render a template with sample data.""" template = await _get_user_template(session, template_id, user.id) try: - env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) + env = SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template.body) rendered = tmpl.render(**_SAMPLE_CONTEXT) return {"rendered": rendered} diff --git a/packages/server/src/immich_watcher_server/api/trackers.py b/packages/server/src/immich_watcher_server/api/trackers.py index 1879aeb..832ed2a 100644 --- a/packages/server/src/immich_watcher_server/api/trackers.py +++ b/packages/server/src/immich_watcher_server/api/trackers.py @@ -128,9 +128,8 @@ async def trigger_tracker( ): """Force an immediate check for a tracker.""" tracker = await _get_user_tracker(session, tracker_id, user.id) - # Import here to avoid circular imports - from ..services.watcher import check_tracker - result = await check_tracker(tracker.id) + from ..services.watcher import check_tracker_with_session + result = await check_tracker_with_session(tracker.id, session) return {"triggered": True, "result": result} diff --git a/packages/server/src/immich_watcher_server/services/notifier.py b/packages/server/src/immich_watcher_server/services/notifier.py index 9244d0d..510456d 100644 --- a/packages/server/src/immich_watcher_server/services/notifier.py +++ b/packages/server/src/immich_watcher_server/services/notifier.py @@ -7,6 +7,7 @@ from typing import Any import aiohttp import jinja2 +from jinja2.sandbox import SandboxedEnvironment from immich_watcher_core.telegram.client import TelegramClient @@ -24,7 +25,7 @@ DEFAULT_TEMPLATE = ( def render_template(template_body: str, context: dict[str, Any]) -> str: """Render a Jinja2 template with the given context.""" - env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) + env = SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template_body) return tmpl.render(**context) diff --git a/packages/server/src/immich_watcher_server/services/watcher.py b/packages/server/src/immich_watcher_server/services/watcher.py index 8aae3a5..d3d3b21 100644 --- a/packages/server/src/immich_watcher_server/services/watcher.py +++ b/packages/server/src/immich_watcher_server/services/watcher.py @@ -33,43 +33,67 @@ _LOGGER = logging.getLogger(__name__) async def check_tracker(tracker_id: int) -> dict[str, Any]: """Check a single tracker for album changes. - Called by the scheduler or manually via API trigger. + Called by the scheduler (creates its own session). """ engine = get_engine() async with AsyncSession(engine) as session: - tracker = await session.get(AlbumTracker, tracker_id) - if not tracker or not tracker.enabled: - return {"skipped": True, "reason": "disabled or not found"} + return await check_tracker_with_session(tracker_id, session) - server = await session.get(ImmichServer, tracker.server_id) - if not server: - return {"error": "Server not found"} - results = [] - async with aiohttp.ClientSession() as http_session: - client = ImmichClient(http_session, server.url, server.api_key) +async def check_tracker_with_session( + tracker_id: int, session: AsyncSession +) -> dict[str, Any]: + """Check a single tracker using a provided session. - # Fetch server config for external domain - await client.get_server_config() - users_cache = await client.get_users() + Called by API trigger (reuses route session) or by check_tracker. + """ + tracker = await session.get(AlbumTracker, tracker_id) + if not tracker or not tracker.enabled: + return {"skipped": True, "reason": "disabled or not found"} - for album_id in tracker.album_ids: - result = await _check_album( - session, http_session, client, tracker, album_id, users_cache - ) - results.append(result) + server = await session.get(ImmichServer, tracker.server_id) + if not server: + return {"error": "Server not found"} - await session.commit() - return {"albums_checked": len(tracker.album_ids), "results": results} + # Eagerly read all needed data before entering aiohttp context + # (SQLAlchemy async greenlet context doesn't survive across other async CMs) + album_ids = list(tracker.album_ids) + event_types = list(tracker.event_types) + target_ids = list(tracker.target_ids) + template_id = tracker.template_id + tracker_db_id = tracker_id + server_url = server.url + server_api_key = server.api_key + + results = [] + async with aiohttp.ClientSession() as http_session: + client = ImmichClient(http_session, server_url, server_api_key) + + # Fetch server config for external domain + await client.get_server_config() + users_cache = await client.get_users() + + for album_id in album_ids: + result = await _check_album( + session, http_session, client, tracker_db_id, + album_id, users_cache, event_types, target_ids, template_id, + ) + results.append(result) + + await session.commit() + return {"albums_checked": len(album_ids), "results": results} async def _check_album( session: AsyncSession, http_session: aiohttp.ClientSession, client: ImmichClient, - tracker: AlbumTracker, + tracker_id: int, album_id: str, users_cache: dict[str, str], + event_types: list[str], + target_ids: list[int], + template_id: int | None, ) -> dict[str, Any]: """Check a single album for changes.""" try: @@ -84,7 +108,7 @@ async def _check_album( # Load previous state result = await session.exec( select(AlbumState).where( - AlbumState.tracker_id == tracker.id, + AlbumState.tracker_id == tracker_id, AlbumState.album_id == album_id, ) ) @@ -93,7 +117,7 @@ async def _check_album( if state is None: # First check - save state, no change detection state = AlbumState( - tracker_id=tracker.id, + tracker_id=tracker_id, album_id=album_id, asset_ids=list(album.asset_ids), pending_asset_ids=[], @@ -134,7 +158,7 @@ async def _check_album( return {"album_id": album_id, "status": "no_changes"} # Check if this event type is tracked - if change.change_type not in tracker.event_types and "changed" not in tracker.event_types: + if change.change_type not in event_types and "changed" not in event_types: return {"album_id": album_id, "status": "filtered", "change_type": change.change_type} # Log the event @@ -142,7 +166,7 @@ async def _check_album( event_data = _build_event_data(change, album, client.external_url, shared_links) event_log = EventLog( - tracker_id=tracker.id, + tracker_id=tracker_id, event_type=change.change_type, album_id=album_id, album_name=album.name, @@ -151,14 +175,14 @@ async def _check_album( session.add(event_log) # Send notifications to all configured targets - for target_id in tracker.target_ids: + for target_id in target_ids: target = await session.get(NotificationTarget, target_id) if not target: continue template = None - if tracker.template_id: - template = await session.get(MessageTemplate, tracker.template_id) + if template_id: + template = await session.get(MessageTemplate, template_id) try: use_ai = target.config.get("ai_captions", False) diff --git a/test-data/immich_watcher.db b/test-data/immich_watcher.db new file mode 100644 index 0000000000000000000000000000000000000000..4ae066c83d374eba9d478cee4995173d35d4d0e8 GIT binary patch literal 36864 zcmeI*TW{J{8~|{8Z31L#=`=AkRa1{>r6yaQ8rv8Xb=pWny4)a;OIt+|I<^ns#n{v~ zY0@YU$?`C1&(j|IA^N^Avahgju-6@%kl@s$YuBt&^G96lbFn$UbB@ii!K_pl4UeFP z?XC%qUiyGK|Geeb&EZEWzdj^@g%rAMV2Dpyufd2Mm=9vVw{4%W7aQ$LONLwh;B-3~$P#oEIki{VVz zRJ(0I%kW4Gx1;mw#KVT^qWP6d*bq%_jmfdySI&EZ**Y7QU3m%N5)jb&X zUpvGxY^XQ*VUfM(L^_n*XtxcmRqsVc<{J{X8zn(O|ibe5^SuFz6-EI4LCG& zH3SrjdPka}0$s<9Y*x5q)VD}p_ zSz(Npi%tH`&^z2f00ck)1V8`;KmY_l00ck)1V8`;J}!aym2@IFGSEB!-(-@_kE?)C z8V~>h5C8!X009sH0T2KI5C8!X_y7V2U$Su~@l_R9i?g_Gn7b{{>$p?*@ArPQF{#^T z(>8bQZLSeSPpZ4~=JNW?v$DGM(}M6~r9EfuEO-lBwZ-Z(cIRJAzmVo#TTonKXYI|) ziaP(F7xKKq_s0LPnB=PupaQD|0T2KI5C8!X009sH0T2KI5C8!Xh!UvK-vuOYrdEp2 ziQ2O5tySWBdl`CdhRhVo^p6Ahuc~29+D`L+$9BEUe&qPSkk83NZ~Xs~NxqC)9fW}Z z2!H?xfB*=900@8p2!H?xfB*5*dV$5@nz1P3Ck!rX99(5^e?JpB{+F{#KGz%n?=Z=o3oZ>T9t1!D1V8`; zKmY_l00ck)1V8`;E}_7|2z#ALBvPBrFA^;Mm%XBYCVwN7$Gj#s3ObjQ3Ye2H&vOdF z5`B`g0-wt%4PDM`Ze)Ug({by;;egCN)Sq`q+ukO+AELFe)ufY~8`;eVQR9C_%w@A; zZ~UKPlBr8r2~cDZ009sH0T2KI5C8!X009sHfy*s$(DF~cuN^LZUi>!$@{NpA;PZ;A zC>$YKg_DR(I7Pw*PQ$V&@eL)HQ{`Zot~PjCZR9zjp*J{56$DPfD&ZtqCOJ*xl{}XG zbaYufRx#mpNfkLsCo)}LLEs9wP$2niUdoD!pH9rtEIB>LVO`Ye;R2B!DG(IQ;haXl zCm<+!O(ygRz~*b?|C~5U$N!QL{QjT0ak;A-N)G}c00JNY0w4eaAOHd&00JNY0w24; EKZT9aPyhe` literal 0 HcmV?d00001