From 1709e8f936cfe3d91bbb60bfac8723cbdf8c68ff Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Sat, 21 Oct 2023 09:41:51 +0200 Subject: [PATCH] UI - BrowserSteps - Show the screenshot of an error if it happened on a step, highlight which step had the error to make it easier to find out why the step didnt work, minor fixes to timeouts(#1883) --- changedetectionio/__init__.py | 2 - .../blueprint/browser_steps/__init__.py | 41 ++++++++++++++++--- .../blueprint/browser_steps/browser_steps.py | 4 +- changedetectionio/model/Watch.py | 14 ++++++- changedetectionio/static/js/browser-steps.js | 28 ++++++++++++- .../styles/scss/parts/_browser-steps.scss | 4 ++ changedetectionio/static/styles/styles.css | 3 ++ changedetectionio/store.py | 1 + changedetectionio/templates/edit.html | 4 +- changedetectionio/update_worker.py | 11 ++++- 10 files changed, 96 insertions(+), 16 deletions(-) diff --git a/changedetectionio/__init__.py b/changedetectionio/__init__.py index adcc7539..4cd622b5 100644 --- a/changedetectionio/__init__.py +++ b/changedetectionio/__init__.py @@ -622,7 +622,6 @@ def changedetection_app(config=None, datastore_o=None): if request.args.get('unpause_on_save'): extra_update_obj['paused'] = False - # Re #110, if they submit the same as the default value, set it to None, so we continue to follow the default # Assume we use the default value, unless something relevant is different, then use the form value # values could be None, 0 etc. @@ -708,7 +707,6 @@ def changedetection_app(config=None, datastore_o=None): # Only works reliably with Playwright visualselector_enabled = os.getenv('PLAYWRIGHT_DRIVER_URL', False) and is_html_webdriver - output = render_template("edit.html", available_processors=processors.available_processors(), browser_steps_config=browser_step_ui_config, diff --git a/changedetectionio/blueprint/browser_steps/__init__.py b/changedetectionio/blueprint/browser_steps/__init__.py index 931d8bbd..11fb208d 100644 --- a/changedetectionio/blueprint/browser_steps/__init__.py +++ b/changedetectionio/blueprint/browser_steps/__init__.py @@ -23,8 +23,10 @@ from distutils.util import strtobool from flask import Blueprint, request, make_response -import os import logging +import os +import re + from changedetectionio.store import ChangeDetectionStore from changedetectionio import login_optionally_required @@ -44,7 +46,7 @@ def construct_blueprint(datastore: ChangeDetectionStore): # We keep the playwright session open for many minutes - seconds_keepalive = int(os.getenv('BROWSERSTEPS_MINUTES_KEEPALIVE', 10)) * 60 + keepalive_seconds = int(os.getenv('BROWSERSTEPS_MINUTES_KEEPALIVE', 10)) * 60 browsersteps_start_session = {'start_time': time.time()} @@ -56,16 +58,18 @@ def construct_blueprint(datastore: ChangeDetectionStore): # Start the Playwright context, which is actually a nodejs sub-process and communicates over STDIN/STDOUT pipes io_interface_context = io_interface_context.start() + keepalive_ms = ((keepalive_seconds + 3) * 1000) + base_url = os.getenv('PLAYWRIGHT_DRIVER_URL', '') + a = "?" if not '?' in base_url else '&' + base_url += a + f"timeout={keepalive_ms}" - # keep it alive for 10 seconds more than we advertise, sometimes it helps to keep it shutting down cleanly - keepalive = "&timeout={}".format(((seconds_keepalive + 3) * 1000)) try: - browsersteps_start_session['browser'] = io_interface_context.chromium.connect_over_cdp( - os.getenv('PLAYWRIGHT_DRIVER_URL', '') + keepalive) + browsersteps_start_session['browser'] = io_interface_context.chromium.connect_over_cdp(base_url) except Exception as e: if 'ECONNREFUSED' in str(e): return make_response('Unable to start the Playwright Browser session, is it running?', 401) else: + # Other errors, bad URL syntax, bad reply etc return make_response(str(e), 401) proxy_id = datastore.get_preferred_proxy_for_watch(uuid=watch_uuid) @@ -118,6 +122,31 @@ def construct_blueprint(datastore: ChangeDetectionStore): print("Starting connection with playwright - done") return {'browsersteps_session_id': browsersteps_session_id} + @login_optionally_required + @browser_steps_blueprint.route("/browsersteps_image", methods=['GET']) + def browser_steps_fetch_screenshot_image(): + from flask import ( + make_response, + request, + send_from_directory, + ) + uuid = request.args.get('uuid') + step_n = int(request.args.get('step_n')) + + watch = datastore.data['watching'].get(uuid) + filename = f"step_before-{step_n}.jpeg" if request.args.get('type', '') == 'before' else f"step_{step_n}.jpeg" + + if step_n and watch and os.path.isfile(os.path.join(watch.watch_data_dir, filename)): + response = make_response(send_from_directory(directory=watch.watch_data_dir, path=filename)) + response.headers['Content-type'] = 'image/jpeg' + response.headers['Cache-Control'] = 'no-cache, no-store, must-revalidate' + response.headers['Pragma'] = 'no-cache' + response.headers['Expires'] = 0 + return response + + else: + return make_response('Unable to fetch image, is the URL correct? does the watch exist? does the step_type-n.jpeg exist?', 401) + # A request for an action was received @login_optionally_required @browser_steps_blueprint.route("/browsersteps_update", methods=['POST']) diff --git a/changedetectionio/blueprint/browser_steps/browser_steps.py b/changedetectionio/blueprint/browser_steps/browser_steps.py index e8e20280..8ef1ac19 100644 --- a/changedetectionio/blueprint/browser_steps/browser_steps.py +++ b/changedetectionio/blueprint/browser_steps/browser_steps.py @@ -138,13 +138,13 @@ class steppable_browser_interface(): def action_wait_for_text(self, selector, value): import json v = json.dumps(value) - self.page.wait_for_function(f'document.querySelector("body").innerText.includes({v});', timeout=90000) + self.page.wait_for_function(f'document.querySelector("body").innerText.includes({v});', timeout=30000) def action_wait_for_text_in_element(self, selector, value): import json s = json.dumps(selector) v = json.dumps(value) - self.page.wait_for_function(f'document.querySelector({s}).innerText.includes({v});', timeout=90000) + self.page.wait_for_function(f'document.querySelector({s}).innerText.includes({v});', timeout=30000) # @todo - in the future make some popout interface to capture what needs to be set # https://playwright.dev/python/docs/api/class-keyboard diff --git a/changedetectionio/model/Watch.py b/changedetectionio/model/Watch.py index a965ae94..2773a3bb 100644 --- a/changedetectionio/model/Watch.py +++ b/changedetectionio/model/Watch.py @@ -4,6 +4,7 @@ import os import re import time import uuid +from pathlib import Path # Allowable protocols, protects against javascript: etc # file:// is further checked by ALLOW_FILE_URI @@ -18,6 +19,7 @@ from changedetectionio.notification import ( base_config = { 'body': None, + 'browser_steps_last_error_step': None, 'check_unique_lines': False, # On change-detected, compare against all history if its something new 'check_count': 0, 'date_created': None, @@ -25,8 +27,8 @@ base_config = { 'extract_text': [], # Extract text by regex after filters 'extract_title_as_title': False, 'fetch_backend': 'system', # plaintext, playwright etc - 'processor': 'text_json_diff', # could be restock_diff or others from .processors 'fetch_time': 0.0, + 'processor': 'text_json_diff', # could be restock_diff or others from .processors 'filter_failure_notification_send': strtobool(os.getenv('FILTER_FAILURE_NOTIFICATION_SEND_DEFAULT', 'True')), 'filter_text_added': True, 'filter_text_replaced': True, @@ -490,3 +492,13 @@ class model(dict): filepath = os.path.join(self.watch_data_dir, 'last-fetched.br') with open(filepath, 'wb') as f: f.write(brotli.compress(contents, mode=brotli.MODE_TEXT)) + + @property + def get_browsersteps_available_screenshots(self): + "For knowing which screenshots are available to show the user in BrowserSteps UI" + available = [] + for f in Path(self.watch_data_dir).glob('step_before-*.jpeg'): + step_n=re.search(r'step_before-(\d+)', f.name) + if step_n: + available.append(step_n.group(1)) + return available diff --git a/changedetectionio/static/js/browser-steps.js b/changedetectionio/static/js/browser-steps.js index 5540ef5c..6da56b6a 100644 --- a/changedetectionio/static/js/browser-steps.js +++ b/changedetectionio/static/js/browser-steps.js @@ -321,8 +321,14 @@ $(document).ready(function () { var s = '
' + 'Apply '; if (i > 0) { // The first step never gets these (Goto-site) - s += 'Clear ' + - 'Remove'; + s += `Clear ` + + `Remove`; + + // if a screenshot is available + if (browser_steps_available_screenshots.includes(i.toString())) { + var d = (browser_steps_last_error_step === i+1) ? 'before' : 'after'; + s += ` Pic `; + } } s += '
'; $(this).append(s) @@ -437,6 +443,24 @@ $(document).ready(function () { }); + $('ul#browser_steps li .control .show-screenshot').click(function (element) { + var step_n = $(event.currentTarget).data('step-index'); + w = window.open(this.href, "_blank", "width=640,height=480"); + const t = $(event.currentTarget).data('type'); + + const url = browser_steps_fetch_screenshot_image_url + `&step_n=${step_n}&type=${t}`; + w.document.body.innerHTML = ` + + + Browser Step at step ${step_n} from last run. + + `; + w.document.title = `Browser Step at step ${step_n} from last run.`; + }); + + if (browser_steps_last_error_step) { + $("ul#browser_steps>li:nth-child("+browser_steps_last_error_step+")").addClass("browser-step-with-error"); + } $("ul#browser_steps select").change(function () { set_greyed_state(); diff --git a/changedetectionio/static/styles/scss/parts/_browser-steps.scss b/changedetectionio/static/styles/scss/parts/_browser-steps.scss index 4d64ce38..f8e8914d 100644 --- a/changedetectionio/static/styles/scss/parts/_browser-steps.scss +++ b/changedetectionio/static/styles/scss/parts/_browser-steps.scss @@ -6,6 +6,10 @@ } li { + &.browser-step-with-error { + background-color: #ffd6d6; + border-radius: 4px; + } &:not(:first-child) { &:hover { opacity: 1.0; diff --git a/changedetectionio/static/styles/styles.css b/changedetectionio/static/styles/styles.css index 8898b014..33071546 100644 --- a/changedetectionio/static/styles/styles.css +++ b/changedetectionio/static/styles/styles.css @@ -26,6 +26,9 @@ #browser_steps li { list-style: decimal; padding: 5px; } + #browser_steps li.browser-step-with-error { + background-color: #ffd6d6; + border-radius: 4px; } #browser_steps li:not(:first-child):hover { opacity: 1.0; } #browser_steps li .control { diff --git a/changedetectionio/store.py b/changedetectionio/store.py index df541af9..e476afae 100644 --- a/changedetectionio/store.py +++ b/changedetectionio/store.py @@ -244,6 +244,7 @@ class ChangeDetectionStore: import pathlib self.__data['watching'][uuid].update({ + 'browser_steps_last_error_step' : None, 'check_count': 0, 'fetch_time' : 0.0, 'has_ldjson_price_data': None, diff --git a/changedetectionio/templates/edit.html b/changedetectionio/templates/edit.html index 8abe08fe..54733d85 100644 --- a/changedetectionio/templates/edit.html +++ b/changedetectionio/templates/edit.html @@ -4,8 +4,10 @@ {% from '_common_fields.jinja' import render_common_settings_form %}