"Send test notification" button - improved error handling, code refactor

changedetectionio/apprise_plugin/__init__.py

@@ -1,6 +1,8 @@
 # include the decorator
 from apprise.decorators import notify
 from loguru import logger
+from requests.structures import CaseInsensitiveDict
+
 
 @notify(on="delete")
 @notify(on="deletes")
@@ -13,70 +15,86 @@
 def apprise_custom_api_call_wrapper(body, title, notify_type, *args, **kwargs):
     import requests
     import json
+    import re
+
     from urllib.parse import unquote_plus
     from apprise.utils.parse import parse_url as apprise_parse_url
-    from apprise import URLBase
 
     url = kwargs['meta'].get('url')
+    schema = kwargs['meta'].get('schema').lower().strip()
 
-    if url.startswith('post'):
-        r = requests.post
-    elif url.startswith('get'):
-        r = requests.get
-    elif url.startswith('put'):
-        r = requests.put
-    elif url.startswith('delete'):
-        r = requests.delete
-
-    url = url.replace('post://', 'http://')
-    url = url.replace('posts://', 'https://')
-    url = url.replace('put://', 'http://')
-    url = url.replace('puts://', 'https://')
-    url = url.replace('get://', 'http://')
-    url = url.replace('gets://', 'https://')
-    url = url.replace('put://', 'http://')
-    url = url.replace('puts://', 'https://')
-    url = url.replace('delete://', 'http://')
-    url = url.replace('deletes://', 'https://')
-
-    headers = {}
-    params = {}
+    # Choose POST, GET etc from requests
+    method =  re.sub(rf's$', '', schema)
+    requests_method = getattr(requests, method)
+
+    headers = CaseInsensitiveDict({})
+    params = CaseInsensitiveDict({}) # Added to requests
     auth = None
+    has_error = False
+
 
     # Convert /foobar?+some-header=hello to proper header dictionary
     results = apprise_parse_url(url)
-    if results:
-        # Add our headers that the user can potentially over-ride if they wish
-        # to to our returned result set and tidy entries by unquoting them
-        headers = {unquote_plus(x): unquote_plus(y)
-                   for x, y in results['qsd+'].items()}
-
-        # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#get-parameter-manipulation
-        # In Apprise, it relies on prefixing each request arg with "-", because it uses say &method=update as a flag for apprise
-        # but here we are making straight requests, so we need todo convert this against apprise's logic
-        for k, v in results['qsd'].items():
-            if not k.strip('+-') in results['qsd+'].keys():
-                params[unquote_plus(k)] = unquote_plus(v)
-
-        # Determine Authentication
-        auth = ''
-        if results.get('user') and results.get('password'):
-            auth = (unquote_plus(results.get('user')), unquote_plus(results.get('user')))
-        elif results.get('user'):
-            auth = (unquote_plus(results.get('user')))
-
-    # Try to auto-guess if it's JSON
-    h = 'application/json; charset=utf-8'
+
+    # Add our headers that the user can potentially over-ride if they wish
+    # to to our returned result set and tidy entries by unquoting them
+    headers = {unquote_plus(x): unquote_plus(y)
+               for x, y in results['qsd+'].items()}
+
+    # https://github.com/caronc/apprise/wiki/Notify_Custom_JSON#get-parameter-manipulation
+    # In Apprise, it relies on prefixing each request arg with "-", because it uses say &method=update as a flag for apprise
+    # but here we are making straight requests, so we need todo convert this against apprise's logic
+    for k, v in results['qsd'].items():
+        if not k.strip('+-') in results['qsd+'].keys():
+            params[unquote_plus(k)] = unquote_plus(v)
+
+    # Determine Authentication
+    auth = ''
+    if results.get('user') and results.get('password'):
+        auth = (unquote_plus(results.get('user')), unquote_plus(results.get('user')))
+    elif results.get('user'):
+        auth = (unquote_plus(results.get('user')))
+
+    # If it smells like it could be JSON and no content-type was already set, offer a default content type.
+    if body and '{' in body[:100] and not headers.get('Content-Type'):
+        json_header = 'application/json; charset=utf-8'
+        try:
+            # Try if it's JSON
+            json.loads(body)
+            headers['Content-Type'] = json_header
+        except ValueError as e:
+            logger.warning(f"Could not automatically add '{json_header}' header to the notification because the document failed to parse as JSON: {e}")
+            pass
+
+    # POSTS -> HTTPS etc
+    if schema.lower().endswith('s'):
+        url = re.sub(rf'^{schema}', 'https', results.get('url'))
+    else:
+        url = re.sub(rf'^{schema}', 'http', results.get('url'))
+
+    status_str = ''
     try:
-        json.loads(body)
-        headers['Content-Type'] = h
-    except ValueError as e:
-        logger.warning(f"Could not automatically add '{h}' header to the {kwargs['meta'].get('schema')}:// notification because the document failed to parse as JSON: {e}")
-        pass
-
-    r(results.get('url'),
-      auth=auth,
-      data=body.encode('utf-8') if type(body) is str else body,
-      headers=headers,
-      params=params
-      )
+        r = requests_method(url,
+          auth=auth,
+          data=body.encode('utf-8') if type(body) is str else body,
+          headers=headers,
+          params=params
+        )
+
+        if r.status_code not in (requests.codes.created, requests.codes.ok):
+            status_str = f"Error sending '{method.upper()}' request to {url} - Status: {r.status_code}: '{r.reason}'"
+            logger.error(status_str)
+            has_error = True
+        else:
+            logger.info(f"Sent '{method.upper()}' request to {url}")
+            has_error = False
+
+    except requests.RequestException as e:
+        status_str = f"Error sending '{method.upper()}' request to {url} - {str(e)}"
+        logger.error(status_str)
+        has_error = True
+
+    if has_error:
+        raise TypeError(status_str)
+
+    return True

changedetectionio/flask_app.py

@@ -598,17 +598,31 @@ def ajax_callback_send_notification_test(watch_uuid=None):
 
             if 'notification_title' in request.form and request.form['notification_title'].strip():
                 n_object['notification_title'] = request.form.get('notification_title', '').strip()
+            elif datastore.data['settings']['application'].get('notification_title'):
+                n_object['notification_title'] = datastore.data['settings']['application'].get('notification_title')
+            else:
+                n_object['notification_title'] = "Test title"
 
             if 'notification_body' in request.form and request.form['notification_body'].strip():
                 n_object['notification_body'] = request.form.get('notification_body', '').strip()
+            elif datastore.data['settings']['application'].get('notification_body'):
+                n_object['notification_body'] = datastore.data['settings']['application'].get('notification_body')
+            else:
+                n_object['notification_body'] = "Test body"
 
+            n_object['as_async'] = False
             n_object.update(watch.extra_notification_token_values())
+            from .notification import process_notification
+            sent_obj = process_notification(n_object, datastore)
 
-            from . import update_worker
-            new_worker = update_worker.update_worker(update_q, notification_q, app, datastore)
-            new_worker.queue_notification_for_watch(notification_q=notification_q, n_object=n_object, watch=watch)
         except Exception as e:
-            return make_response(f"Error: str(e)", 400)
+            e_str = str(e)
+            # Remove this text which is not important and floods the container
+            e_str = e_str.replace(
+                "DEBUG - <class 'apprise.decorators.base.CustomNotifyPlugin.instantiate_plugin.<locals>.CustomNotifyPluginWrapper'>",
+                '')
+
+            return make_response(e_str, 400)
 
         return 'OK - Sent test notifications'
 

changedetectionio/notification.py

@@ -67,6 +67,10 @@ def process_notification(n_object, datastore):
 
     sent_objs = []
     from .apprise_asset import asset
+
+    if 'as_async' in n_object:
+        asset.async_mode = n_object.get('as_async')
+
     apobj = apprise.Apprise(debug=True, asset=asset)
 
     if not n_object.get('notification_urls'):
@@ -157,8 +161,6 @@ def process_notification(n_object, datastore):
             attach=n_object.get('screenshot', None)
         )
 
-        # Give apprise time to register an error
-        time.sleep(3)
 
         # Returns empty string if nothing found, multi-line string otherwise
         log_value = logs.getvalue()

changedetectionio/static/js/notifications.js

@@ -1,42 +1,52 @@
-$(document).ready(function() {
+$(document).ready(function () {
 
-  $('#add-email-helper').click(function (e) {
-    e.preventDefault();
-    email = prompt("Destination email");
-    if(email) {
-      var n = $(".notification-urls");
-      var p=email_notification_prefix;
-      $(n).val( $.trim( $(n).val() )+"\n"+email_notification_prefix+email );
-    }
-  });
-
-  $('#send-test-notification').click(function (e) {
-    e.preventDefault();
-
-    data = {
-      notification_body: $('#notification_body').val(),
-      notification_format: $('#notification_format').val(),
-      notification_title: $('#notification_title').val(),
-      notification_urls: $('.notification-urls').val(),
-      tags: $('#tags').val(),
-      window_url: window.location.href,
-    }
+    $('#add-email-helper').click(function (e) {
+        e.preventDefault();
+        email = prompt("Destination email");
+        if (email) {
+            var n = $(".notification-urls");
+            var p = email_notification_prefix;
+            $(n).val($.trim($(n).val()) + "\n" + email_notification_prefix + email);
+        }
+    });
 
+    $('#send-test-notification').click(function (e) {
+        e.preventDefault();
 
-    $.ajax({
-      type: "POST",
-      url: notification_base_url,
-      data : data,
-        statusCode: {
-        400: function(data) {
-          // More than likely the CSRF token was lost when the server restarted
-          alert(data.responseText);
+        data = {
+            notification_body: $('#notification_body').val(),
+            notification_format: $('#notification_format').val(),
+            notification_title: $('#notification_title').val(),
+            notification_urls: $('.notification-urls').val(),
+            tags: $('#tags').val(),
+            window_url: window.location.href,
         }
-      }
-    }).done(function(data){
-      console.log(data);
-      alert(data);
-    })
-  });
+
+        $('.notifications-wrapper .spinner').fadeIn();
+        $('#notification-test-log').show();
+        $.ajax({
+            type: "POST",
+            url: notification_base_url,
+            data: data,
+            statusCode: {
+                400: function (data) {
+                    $("#notification-test-log>span").text(data.responseText);
+                },
+            }
+        }).done(function (data) {
+            $("#notification-test-log>span").text(data);
+        }).fail(function (jqXHR, textStatus, errorThrown) {
+            // Handle connection refused or other errors
+            if (textStatus === "error" && errorThrown === "") {
+                console.error("Connection refused or server unreachable");
+                $("#notification-test-log>span").text("Error: Connection refused or server is unreachable.");
+            } else {
+                console.error("Error:", textStatus, errorThrown);
+                $("#notification-test-log>span").text("An error occurred: " + textStatus);
+            }
+        }).always(function () {
+            $('.notifications-wrapper .spinner').hide();
+        })
+    });
 });
 

changedetectionio/static/styles/scss/styles.scss

@@ -380,7 +380,15 @@ a.pure-button-selected {
 }
 
 .notifications-wrapper {
-  padding: 0.5rem 0 1rem 0;
+  padding-top: 0.5rem;
+  #notification-test-log {
+    padding-top: 1rem;
+    white-space: pre-wrap;
+    word-break: break-word;
+    overflow-wrap: break-word;
+    max-width: 100%;
+    box-sizing: border-box;
+  }
 }
 
 label {

changedetectionio/static/styles/styles.css

@@ -780,7 +780,14 @@ a.pure-button-selected {
   cursor: pointer; }
 
 .notifications-wrapper {
-  padding: 0.5rem 0 1rem 0; }
+  padding-top: 0.5rem; }
+  .notifications-wrapper #notification-test-log {
+    padding-top: 1rem;
+    white-space: pre-wrap;
+    word-break: break-word;
+    overflow-wrap: break-word;
+    max-width: 100%;
+    box-sizing: border-box; }
 
 label:hover {
   cursor: pointer; }

changedetectionio/templates/_common_fields.html

@@ -24,11 +24,13 @@
                               </ul>
                             </div>
                             <div class="notifications-wrapper">
-                              <a id="send-test-notification" class="pure-button button-secondary button-xsmall" >Send test notification</a>
+                              <a id="send-test-notification" class="pure-button button-secondary button-xsmall" >Send test notification</a> <div class="spinner"  style="display: none;"></div>
                             {% if emailprefix %}
                               <a id="add-email-helper" class="pure-button button-secondary button-xsmall" >Add email <img style="height: 1em; display: inline-block" src="{{url_for('static_content', group='images', filename='email.svg')}}" alt="Add an email address"> </a>
                             {% endif %}
                               <a href="{{url_for('notification_logs')}}" class="pure-button button-secondary button-xsmall" >Notification debug logs</a>
+                              <br>
+                                <div id="notification-test-log" style="display: none;"><span class="pure-form-message-inline">Processing..</span></div>
                             </div>
                         </div>
                         <div id="notification-customisation" class="pure-control-group">

changedetectionio/tests/test_add_replace_remove_filter.py

@@ -1,9 +1,9 @@
 #!/usr/bin/env python3
+
 import os.path
-import time
+
 from flask import url_for
 from .util import live_server_setup, wait_for_all_checks, wait_for_notification_endpoint_output
-from changedetectionio import html_tools
 
 
 def set_original(excluding=None, add_line=None):

changedetectionio/tests/test_notification.py

@@ -360,16 +360,18 @@ def test_global_send_test_notification(client, live_server, measure_memory_usage
     #live_server_setup(live_server)
     set_original_response()
     if os.path.isfile("test-datastore/notification.txt"):
-        os.unlink("test-datastore/notification.txt")
+        os.unlink("test-datastore/notification.txt") \
+
+    # 1995 UTF-8 content should be encoded
+    test_body = 'change detection is cool 网站监测 内容更新了'
 
     # otherwise other settings would have already existed from previous tests in this file
     res = client.post(
         url_for("settings_page"),
         data={
             "application-fetch_backend": "html_requests",
             "application-minutes_between_check": 180,
-            #1995 UTF-8 content should be encoded
-            "application-notification_body": 'change detection is cool 网站监测 内容更新了',
+            "application-notification_body": test_body,
             "application-notification_format": default_notification_format,
             "application-notification_urls": "",
             "application-notification_title": "New ChangeDetection.io Notification - {{ watch_url }}",
@@ -399,12 +401,10 @@ def test_global_send_test_notification(client, live_server, measure_memory_usage
     assert res.status_code != 400
     assert res.status_code != 500
 
-    # Give apprise time to fire
-    time.sleep(4)
 
     with open("test-datastore/notification.txt", 'r') as f:
         x = f.read()
-        assert 'change detection is cool 网站监测 内容更新了' in x
+        assert test_body in x
 
     os.unlink("test-datastore/notification.txt")