diff --git a/changedetectionio/blueprint/browser_steps/__init__.py b/changedetectionio/blueprint/browser_steps/__init__.py index 0c4b8743..52fff1f5 100644 --- a/changedetectionio/blueprint/browser_steps/__init__.py +++ b/changedetectionio/blueprint/browser_steps/__init__.py @@ -21,31 +21,154 @@ from changedetectionio.flask_app import login_optionally_required from loguru import logger browsersteps_sessions = {} +browsersteps_watch_to_session = {} # Maps watch_uuid -> browsersteps_session_id io_interface_context = None import json import hashlib from flask import Response import asyncio import threading +import time -def run_async_in_browser_loop(coro): - """Run async coroutine using the existing async worker event loop""" - from changedetectionio import worker_handler - - # Use the existing async worker event loop instead of creating a new one - if worker_handler.USE_ASYNC_WORKERS and worker_handler.async_loop and not worker_handler.async_loop.is_closed(): - logger.debug("Browser steps using existing async worker event loop") - future = asyncio.run_coroutine_threadsafe(coro, worker_handler.async_loop) - return future.result() - else: - # Fallback: create a new event loop (for sync workers or if async loop not available) - logger.debug("Browser steps creating temporary event loop") - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) +# Dedicated event loop for ALL browser steps sessions +_browser_steps_loop = None +_browser_steps_thread = None +_browser_steps_loop_lock = threading.Lock() + +def _start_browser_steps_loop(): + """Start a dedicated event loop for browser steps in its own thread""" + global _browser_steps_loop + + # Create and set the event loop for this thread + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + _browser_steps_loop = loop + + logger.debug("Browser steps event loop started") + + try: + # Run the loop forever - handles all browsersteps sessions + loop.run_forever() + except Exception as e: + logger.error(f"Browser steps event loop error: {e}") + finally: try: - return loop.run_until_complete(coro) + # Cancel all remaining tasks + pending = asyncio.all_tasks(loop) + for task in pending: + task.cancel() + + # Wait for tasks to finish cancellation + if pending: + loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) + except Exception as e: + logger.debug(f"Error during browser steps loop cleanup: {e}") finally: loop.close() + logger.debug("Browser steps event loop closed") + +def _ensure_browser_steps_loop(): + """Ensure the browser steps event loop is running""" + global _browser_steps_loop, _browser_steps_thread + + with _browser_steps_loop_lock: + if _browser_steps_thread is None or not _browser_steps_thread.is_alive(): + logger.debug("Starting browser steps event loop thread") + _browser_steps_thread = threading.Thread( + target=_start_browser_steps_loop, + daemon=True, + name="BrowserStepsEventLoop" + ) + _browser_steps_thread.start() + + # Wait for the loop to be ready + timeout = 5.0 + start_time = time.time() + while _browser_steps_loop is None: + if time.time() - start_time > timeout: + raise RuntimeError("Browser steps event loop failed to start") + time.sleep(0.01) + + logger.debug("Browser steps event loop thread started and ready") + +def run_async_in_browser_loop(coro): + """Run async coroutine using the dedicated browser steps event loop""" + _ensure_browser_steps_loop() + + if _browser_steps_loop and not _browser_steps_loop.is_closed(): + logger.debug("Browser steps using dedicated event loop") + future = asyncio.run_coroutine_threadsafe(coro, _browser_steps_loop) + return future.result() + else: + raise RuntimeError("Browser steps event loop is not available") + +def cleanup_expired_sessions(): + """Remove expired browsersteps sessions and cleanup their resources""" + global browsersteps_sessions, browsersteps_watch_to_session + + expired_session_ids = [] + + # Find expired sessions + for session_id, session_data in browsersteps_sessions.items(): + browserstepper = session_data.get('browserstepper') + if browserstepper and browserstepper.has_expired: + expired_session_ids.append(session_id) + + # Cleanup expired sessions + for session_id in expired_session_ids: + logger.debug(f"Cleaning up expired browsersteps session {session_id}") + session_data = browsersteps_sessions[session_id] + + # Cleanup playwright resources asynchronously + browserstepper = session_data.get('browserstepper') + if browserstepper: + try: + run_async_in_browser_loop(browserstepper.cleanup()) + except Exception as e: + logger.error(f"Error cleaning up session {session_id}: {e}") + + # Remove from sessions dict + del browsersteps_sessions[session_id] + + # Remove from watch mapping + for watch_uuid, mapped_session_id in list(browsersteps_watch_to_session.items()): + if mapped_session_id == session_id: + del browsersteps_watch_to_session[watch_uuid] + break + + if expired_session_ids: + logger.info(f"Cleaned up {len(expired_session_ids)} expired browsersteps session(s)") + +def cleanup_session_for_watch(watch_uuid): + """Cleanup a specific browsersteps session for a watch UUID""" + global browsersteps_sessions, browsersteps_watch_to_session + + session_id = browsersteps_watch_to_session.get(watch_uuid) + if not session_id: + logger.debug(f"No browsersteps session found for watch {watch_uuid}") + return + + logger.debug(f"Cleaning up browsersteps session {session_id} for watch {watch_uuid}") + + session_data = browsersteps_sessions.get(session_id) + if session_data: + browserstepper = session_data.get('browserstepper') + if browserstepper: + try: + run_async_in_browser_loop(browserstepper.cleanup()) + except Exception as e: + logger.error(f"Error cleaning up session {session_id} for watch {watch_uuid}: {e}") + + # Remove from sessions dict + del browsersteps_sessions[session_id] + + # Remove from watch mapping + del browsersteps_watch_to_session[watch_uuid] + + logger.debug(f"Cleaned up session for watch {watch_uuid}") + + # Opportunistically cleanup any other expired sessions + cleanup_expired_sessions() def construct_blueprint(datastore: ChangeDetectionStore): browser_steps_blueprint = Blueprint('browser_steps', __name__, template_folder="templates") @@ -123,6 +246,9 @@ def construct_blueprint(datastore: ChangeDetectionStore): if not watch_uuid: return make_response('No Watch UUID specified', 500) + # Cleanup any existing session for this watch + cleanup_session_for_watch(watch_uuid) + logger.debug("Starting connection with playwright") logger.debug("browser_steps.py connecting") @@ -131,6 +257,10 @@ def construct_blueprint(datastore: ChangeDetectionStore): browsersteps_sessions[browsersteps_session_id] = run_async_in_browser_loop( start_browsersteps_session(watch_uuid) ) + + # Store the mapping of watch_uuid -> browsersteps_session_id + browsersteps_watch_to_session[watch_uuid] = browsersteps_session_id + except Exception as e: if 'ECONNREFUSED' in str(e): return make_response('Unable to start the Playwright Browser session, is sockpuppetbrowser running? Network configuration is OK?', 401) diff --git a/changedetectionio/blueprint/ui/edit.py b/changedetectionio/blueprint/ui/edit.py index f8a2f6aa..e2edd557 100644 --- a/changedetectionio/blueprint/ui/edit.py +++ b/changedetectionio/blueprint/ui/edit.py @@ -238,6 +238,13 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe datastore.data['watching'][uuid] = watch_class(datastore_path=datastore.datastore_path, default=datastore.data['watching'][uuid]) flash(gettext("Updated watch - unpaused!") if request.args.get('unpause_on_save') else gettext("Updated watch.")) + # Cleanup any browsersteps session for this watch + try: + from changedetectionio.blueprint.browser_steps import cleanup_session_for_watch + cleanup_session_for_watch(uuid) + except Exception as e: + logger.debug(f"Error cleaning up browsersteps session: {e}") + # Re #286 - We wait for syncing new data to disk in another thread every 60 seconds # But in the case something is added we should save straight away datastore.needs_write_urgent = True @@ -325,8 +332,6 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe 'url': url_for('rss.rss_single_watch', uuid=watch['uuid'], token=app_rss_token) }, 'settings_application': datastore.data['settings']['application'], - 'system_has_playwright_configured': os.getenv('PLAYWRIGHT_DRIVER_URL'), - 'system_has_webdriver_configured': os.getenv('WEBDRIVER_URL'), 'ui_edit_stats_extras': collect_ui_edit_stats_extras(watch), 'visual_selector_data_ready': datastore.visualselector_data_is_ready(watch_uuid=uuid), 'timezone_default_config': datastore.data['settings']['application'].get('scheduler_timezone_default'), diff --git a/changedetectionio/blueprint/ui/templates/edit.html b/changedetectionio/blueprint/ui/templates/edit.html index b44e78e4..b6398638 100644 --- a/changedetectionio/blueprint/ui/templates/edit.html +++ b/changedetectionio/blueprint/ui/templates/edit.html @@ -206,7 +206,7 @@ Math: {{ 1 + 1 }}") }}
{% if capabilities.supports_browser_steps %} - {% if visual_selector_data_ready %} + {% if true %}
diff --git a/changedetectionio/content_fetchers/webdriver_selenium.py b/changedetectionio/content_fetchers/webdriver_selenium.py index 2399b712..73c6dad4 100644 --- a/changedetectionio/content_fetchers/webdriver_selenium.py +++ b/changedetectionio/content_fetchers/webdriver_selenium.py @@ -15,7 +15,7 @@ class fetcher(Fetcher): proxy_url = None # Capability flags - supports_browser_steps = True + supports_browser_steps = False supports_screenshots = True supports_xpath_element_data = True diff --git a/changedetectionio/tests/visualselector/test_fetch_data.py b/changedetectionio/tests/visualselector/test_fetch_data.py index f10519ae..072f5870 100644 --- a/changedetectionio/tests/visualselector/test_fetch_data.py +++ b/changedetectionio/tests/visualselector/test_fetch_data.py @@ -144,7 +144,6 @@ def test_basic_browserstep(client, live_server, measure_memory_usage, datastore_ def test_non_200_errors_report_browsersteps(client, live_server, measure_memory_usage, datastore_path): - four_o_four_url = url_for('test_endpoint', status_code=404, _external=True) four_o_four_url = four_o_four_url.replace('localhost.localdomain', 'cdio') four_o_four_url = four_o_four_url.replace('localhost', 'cdio') @@ -186,3 +185,65 @@ def test_non_200_errors_report_browsersteps(client, live_server, measure_memory_ url_for("ui.form_delete", uuid="all"), follow_redirects=True ) + +def test_browsersteps_edit_UI_startsession(client, live_server, measure_memory_usage, datastore_path): + + assert os.getenv('PLAYWRIGHT_DRIVER_URL'), "Needs PLAYWRIGHT_DRIVER_URL set for this test" + + # Add a watch first + test_url = url_for('test_interactive_html_endpoint', _external=True) + test_url = test_url.replace('localhost.localdomain', 'cdio') + test_url = test_url.replace('localhost', 'cdio') + + uuid = client.application.config.get('DATASTORE').add_watch(url=test_url, extras={'fetch_backend': 'html_webdriver', 'paused': True}) + + # Test starting a browsersteps session + res = client.get( + url_for("browser_steps.browsersteps_start_session", uuid=uuid), + follow_redirects=True + ) + + assert res.status_code == 200 + assert res.is_json + json_data = res.get_json() + assert 'browsersteps_session_id' in json_data + assert json_data['browsersteps_session_id'] # Not empty + + browsersteps_session_id = json_data['browsersteps_session_id'] + + # Verify the session exists in browsersteps_sessions + from changedetectionio.blueprint.browser_steps import browsersteps_sessions, browsersteps_watch_to_session + assert browsersteps_session_id in browsersteps_sessions + assert uuid in browsersteps_watch_to_session + assert browsersteps_watch_to_session[uuid] == browsersteps_session_id + + # Verify browsersteps UI shows up on edit page + res = client.get(url_for("ui.ui_edit.edit_page", uuid=uuid)) + assert b'browsersteps-click-start' in res.data, "Browsersteps manual UI shows up" + + # Session should still exist after GET (not cleaned up yet) + assert browsersteps_session_id in browsersteps_sessions + assert uuid in browsersteps_watch_to_session + + # Test cleanup happens on save (POST) + res = client.post( + url_for("ui.ui_edit.edit_page", uuid=uuid), + data={ + "url": test_url, + "tags": "", + 'fetch_backend': "html_webdriver", + "time_between_check_use_default": "y", + }, + follow_redirects=True + ) + assert b"Updated watch" in res.data + + # NOW verify the session was cleaned up after save + assert browsersteps_session_id not in browsersteps_sessions + assert uuid not in browsersteps_watch_to_session + + # Cleanup + client.get( + url_for("ui.form_delete", uuid="all"), + follow_redirects=True + )