Defensive coding

README.md

@@ -34,7 +34,7 @@ deactivate
 cp config-example.yml config.yml
 ```
 
-Then edit `config.yml` in your text editor of choice to set up the software as you like it.
+Then edit `config.yml` in your text editor of choice to set up the software as you like it. Mostly, this will involve enabling or disabling the various providers of spot and alert data.
 
 `config.yml` has some entries for QRZ.com username & password, and Clublog API keys. If provided, these allow Spothole to retrieve more information about DX spots, such as the country their callsign corresponds to. The software will work just fine without them, but you may find a few country flags etc. are less accurate or missing.
 
@@ -57,6 +57,8 @@ If you see some errors on startup, check your configuration, e.g. in case you ha
 
 ### systemd configuration
 
+If you want Spothole to run automatically on startup on a Linux distribution that uses `systemd`, follow the instructions here. For distros that don't use `systemd`, or Windows/OSX/etc., you can find generic instructions for your OS online.
+
 Create a file at `/etc/systemd/system/spothole.service`. Give it the following content, adjusting for the user you want to run it as and the directory in which you have installed it:
 
 ```
@@ -87,7 +89,9 @@ Check the service has started up correctly with `sudo journalctl -u spothole -f`
 
 ### nginx Reverse Proxy configuration
 
-It's best not to serve Spothole directly on port 80, as that requires root privileges and prevents us using HTTPS, amongst other reasons. To set up nginx as a reverse proxy that sits in front of Spothole, first ensure it's installed e.g. `sudo apt install nginx`, and enabled e.g. `sudo systemd enable nginx`.
+Web servers generally serve their pages from port 80. However, it's best not to serve Spothole's web interface directly on port 80, as that requires root privileges on a Linux system. It also and prevents us using HTTPS to serve a secure site, since Spothole itself doesn't directly support acting as an HTTPS server. The normal solution to this is to use a "reverse proxy" setup, where a general web server handles HTTP and HTTP requests (to port 80 & 443 respectively), then passes on the request to the back-end application (in this case Spothole). nginx is a common choice for this general web server.
+
+To set up nginx as a reverse proxy that sits in front of Spothole, first ensure it's installed e.g. `sudo apt install nginx`, and enabled e.g. `sudo systemd enable nginx`.
 
 Create a file at `/etc/nginx/sites-available/` called `spothole`. Give it the following contents, replacing `spothole.app` with the domain name on which you want to run Spothole. If you changed the port on which Spothole runs, update that on the "proxy_pass" line too.
 
@@ -140,8 +144,8 @@ Once that's working, [install certbot](https://certbot.eff.org/instructions?ws=n
 Various approaches exist to writing your own client, but in general:
 
 * Refer to the API docs. These are built on an OpenAPI definition file (`/webassets/apidocs/openapi.yml`), which you can automatically use to generate a client skeleton using various software.
-* Call the main "spots" API to get the data you want. Apply filters if necessary.
-* Call the "options" API to get an idea of which bands, modes etc. the server knows about. You might want to do that first before calling the spots API.
+* Call the main "spots" or "alerts" API endpoints to get the data you want. Apply filters if necessary.
+* Call the "options" API to get an idea of which bands, modes etc. the server knows about. You might want to do that first before calling the spots/alerts APIs, to allow you to populate your filters correctly.
 * Refer to the provided HTML/JS interface for a reference
 * Let me know if you get stuck, I'm happy to help!
 
@@ -178,19 +182,21 @@ To navigate your way around the source code, this list may help.
 
 ### Extending the server
 
-Spothole is designed to be easily extensible. If you want to write your own provider, simply add a module to the `providers` package containing your class. (Currently, in order to be loaded correctly, the module (file) name should be the same as the class name, but lower case.)
+Spothole is designed to be easily extensible. If you want to write your own spot provider, for example, simply add a module to the `spotproviders` package containing your class. (Currently, in order to be loaded correctly, the module (file) name should be the same as the class name, but lower case.)
 
-Your class should extend "Provider"; if it operates by polling an HTTP Server on a timer, it can instead extend "HTTPProvider" where some of the work is done for you.
+Your class should extend "SpotProvider"; if it operates by polling an HTTP Server on a timer, it can instead extend "HTTPSpotProvider" where some of the work is done for you.
 
 The class will need to implement a constructor that takes in the `provider_config` and provides it to the superclass constructor, while also taking any other config parameters it needs.
 
-If you're extending the base `Provider` class, you will need to implement `start()` and `stop()` methods that start and stop a separate thread which handles the provider's processing needs. The thread should call `submit()` or `submit_batch()` when it has one or more spots to report.
+If you're extending the base `SpotProvider` class, you will need to implement `start()` and `stop()` methods that start and stop a separate thread which handles the provider's processing needs. The thread should call `submit()` or `submit_batch()` when it has one or more spots to report.
 
-If you're extending the `HTTPProvider` class, you will need to provide a URI to query and an interval to the superclass constructor. You'll then need to implement the `http_response_to_spots()` method which is called when new data is retrieved. Your implementation should then call `submit()` or `submit_batch()` when it has one or more spots to report.
+If you're extending the `HTTPSpotProvider` class, you will need to provide a URI to query and an interval to the superclass constructor. You'll then need to implement the `http_response_to_spots()` method which is called when new data is retrieved. Your implementation should then call `submit()` or `submit_batch()` when it has one or more spots to report.
 
 When constructing spots, use the comments in the Spot class and the existing implementations as an example. All parameters are optional, but you will at least want to provide a `time` (which must be timezone-aware) and a `dx_call`.
 
-Finally, simply add the appropriate config to the `providers` section of `config.yml`, and your provider should be instantiated on startup.
+Finally, simply add the appropriate config to the `spot_providers` section of `config.yml`, and your provider should be instantiated on startup.
+
+The same approach as above is also used for alert providers.
 
 ### Thanks
 

core/cleanup.py

@@ -29,16 +29,27 @@ class CleanupTimer:
     # Perform cleanup and reschedule next timer
     def cleanup(self):
         try:
-            # Perform cleanup
+            # Perform cleanup via letting the data expire
             self.spots.expire()
             self.alerts.expire()
 
-            # Alerts can persist in the system for a while, so we want to explicitly clean up any alerts that have
-            # expired
+            # Explicitly clean up any spots and alerts that have expired
+            for id in list(self.spots.iterkeys()):
+                try:
+                    spot = self.spots[id]
+                    if spot.expired():
+                        self.spots.delete(id)
+                except KeyError:
+                    # Must have already been deleted, OK with that
+                    pass
             for id in list(self.alerts.iterkeys()):
-                alert = self.alerts[id]
-                if alert.expired():
-                    self.alerts.delete(id)
+                try:
+                    alert = self.alerts[id]
+                    if alert.expired():
+                        self.alerts.delete(id)
+                except KeyError:
+                    # Must have already been deleted, OK with that
+                    pass
 
             self.status = "OK"
             self.last_cleanup_time = datetime.now(pytz.UTC)

core/lookup_helper.py

@@ -455,6 +455,11 @@ class LookupHelper:
                     # QRZ had no info for the call, that's OK. Cache a None so we don't try to look this up again
                     self.QRZ_CALLSIGN_DATA_CACHE.add(call, None, expire=604800)  # 1 week in seconds
                     return None
+            except (Exception):
+                # General exception like a timeout when communicating with QRZ. Return None this time, but don't cache
+                # that, so we can try again next time.
+                logging.error("Exception when looking up QRZ data")
+                return None
         else:
             return None
 

data/alert.py

@@ -137,7 +137,7 @@ class Alert:
         return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True)
 
     # Decide if this alert has expired (in which case it should not be added to the system in the first place, and not
-    # returned by the web server if later requested, and removed by the cleanup functions. "Expired" is defined as
+    # returned by the web server if later requested, and removed by the cleanup functions). "Expired" is defined as
     # either having an end_time in the past, or if it only has a start_time, then that start time was more than 3 hours
     # ago. If it somehow doesn't have a start_time either, it is considered to be expired.
     def expired(self):

data/spot.py

@@ -4,11 +4,12 @@ import json
 import logging
 import re
 from dataclasses import dataclass
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import pytz
 from pyhamtools.locator import locator_to_latlong, latlong_to_locator
 
+from core.config import MAX_SPOT_AGE
 from core.lookup_helper import lookup_helper
 from core.sig_utils import get_icon_for_sig, get_sig_ref_info, ANY_SIG_REGEX, get_ref_regex_for_sig
 from data.sig_ref import SIGRef
@@ -243,13 +244,15 @@ class Spot:
         if not self.sig and self.sig_refs and len(self.sig_refs) > 0:
             self.sig = self.sig_refs[0].sig.upper()
 
-            # See if we already have a SIG reference, but the comment looks like it contains more for the same SIG. This
+        # See if we already have a SIG reference, but the comment looks like it contains more for the same SIG. This
         # should catch e.g. POTA comments like "2-fer: GB-0001 GB-0002".
-        if self.comment and self.sig_refs and len(self.sig_refs) > 0:
+        if self.comment and self.sig_refs and len(self.sig_refs) > 0 and self.sig_refs[0].sig:
             sig = self.sig_refs[0].sig.upper()
-            all_comment_ref_matches = re.finditer(r"(^|\W)(" + get_ref_regex_for_sig(sig) + r")(^|\W)", self.comment, re.IGNORECASE)
-            for ref_match in all_comment_ref_matches:
-                self.append_sig_ref_if_missing(SIGRef(id=ref_match.group(2).upper(), sig=sig))
+            regex = get_ref_regex_for_sig(sig)
+            if regex:
+                all_comment_ref_matches = re.finditer(r"(^|\W)(" + regex + r")(^|\W)", self.comment, re.IGNORECASE)
+                for ref_match in all_comment_ref_matches:
+                    self.append_sig_ref_if_missing(SIGRef(id=ref_match.group(2).upper(), sig=sig))
 
         # See if the comment looks like it contains any SIGs (and optionally SIG references) that we can
         # add to the spot. This should catch cluster spot comments like "POTA GB-0001 WWFF GFF-0001" and e.g. POTA
@@ -352,9 +355,10 @@ class Spot:
 
         # DX Location is "good" if it is from a spot, or from QRZ if the callsign doesn't contain a slash, so the operator
         # is likely at home.
-        self.dx_location_good = (self.dx_location_source == "SPOT" or self.dx_location_source == "SIG REF LOOKUP"
-                                 or self.dx_location_source == "WAB/WAI GRID"
-                                 or (self.dx_location_source == "HOME QTH" and not "/" in self.dx_call))
+        self.dx_location_good = self.dx_latitude and self.dx_longitude and (
+                    self.dx_location_source == "SPOT" or self.dx_location_source == "SIG REF LOOKUP"
+                    or self.dx_location_source == "WAB/WAI GRID"
+                    or (self.dx_location_source == "HOME QTH" and not "/" in self.dx_call))
 
         # DE with no digits and APRS servers starting "T2" are not things we can look up location for
         if self.de_call and any(char.isdigit() for char in self.de_call) and not (self.de_call.startswith("T2") and self.source == "APRS-IS"):
@@ -400,3 +404,10 @@ class Spot:
             if sig_ref.id == new_sig_ref.id and sig_ref.sig == new_sig_ref.sig:
                 return
         self.sig_refs.append(new_sig_ref)
+
+    # Decide if this spot has expired (in which case it should not be added to the system in the first place, and not
+    # returned by the web server if later requested, and removed by the cleanup functions). "Expired" is defined as
+    # either having a time further ago than the server's MAX_SPOT_AGE. If it somehow doesn't have a time either, it is
+    # considered to be expired.
+    def expired(self):
+        return not self.time or self.time < (datetime.now(pytz.UTC) - timedelta(seconds=MAX_SPOT_AGE)).timestamp()

server/webserver.py

@@ -422,7 +422,6 @@ class WebServer:
             if a is not None:
                 alerts.append(a)
         # We never want alerts that seem to be in the past
-        alerts = list(filter(lambda alert: not alert.expired(), alerts))
         alerts = sorted(alerts, key=lambda alert: (alert.start_time if alert and alert.start_time else 0))
         for k in query.keys():
             match k:

spotproviders/gma.py

@@ -41,42 +41,47 @@ class GMA(HTTPSpotProvider):
                         dx_longitude=float(source_spot["LON"]) if (source_spot["LON"] and source_spot["LON"] != "") else None)
 
             # GMA doesn't give what programme (SIG) the reference is for until we separately look it up.
-            ref_response = SEMI_STATIC_URL_DATA_CACHE.get(self.REF_INFO_URL_ROOT + source_spot["REF"],
-                                                   headers=HTTP_HEADERS)
-            # Sometimes this is blank, so handle that
-            if ref_response.text is not None and ref_response.text != "":
-                ref_info = ref_response.json()
-                # If this is POTA, SOTA or WWFF data we already have it through other means, so ignore. POTA and WWFF
-                # spots come through with reftype=POTA or reftype=WWFF. SOTA is harder to figure out because both SOTA
-                # and GMA summits come through with reftype=Summit, so we must check for the presence of a "sota" entry
-                # to determine if it's a SOTA summit.
-                if "reftype" in ref_info and ref_info["reftype"] not in ["POTA", "WWFF"] and (ref_info["reftype"] != "Summit" or ref_info["sota"] == ""):
-                    match ref_info["reftype"]:
-                        case "Summit":
-                            spot.sig_refs[0].sig = "GMA"
-                            spot.sig = "GMA"
-                        case "IOTA Island":
-                            spot.sig_refs[0].sig = "IOTA"
-                            spot.sig = "IOTA"
-                        case "Lighthouse (ILLW)":
-                            spot.sig_refs[0].sig = "ILLW"
-                            spot.sig = "ILLW"
-                        case "Lighthouse (ARLHS)":
-                            spot.sig_refs[0].sig = "ARLHS"
-                            spot.sig = "ARLHS"
-                        case "Castle":
-                            spot.sig_refs[0].sig = "WCA"
-                            spot.sig = "WCA"
-                        case "Mill":
-                            spot.sig_refs[0].sig = "MOTA"
-                            spot.sig = "MOTA"
-                        case _:
-                            logging.warn("GMA spot found with ref type " + ref_info[
-                                "reftype"] + ", developer needs to add support for this!")
-                            spot.sig_refs[0].sig = ref_info["reftype"]
-                            spot.sig = ref_info["reftype"]
+            if "REF" in source_spot:
+                try:
+                    ref_response = SEMI_STATIC_URL_DATA_CACHE.get(self.REF_INFO_URL_ROOT + source_spot["REF"],
+                                                                  headers=HTTP_HEADERS)
+                    # Sometimes this is blank, so handle that
+                    if ref_response.text is not None and ref_response.text != "":
+                        ref_info = ref_response.json()
+                        # If this is POTA, SOTA or WWFF data we already have it through other means, so ignore. POTA and WWFF
+                        # spots come through with reftype=POTA or reftype=WWFF. SOTA is harder to figure out because both SOTA
+                        # and GMA summits come through with reftype=Summit, so we must check for the presence of a "sota" entry
+                        # to determine if it's a SOTA summit.
+                        if "reftype" in ref_info and ref_info["reftype"] not in ["POTA", "WWFF"] and (
+                                ref_info["reftype"] != "Summit" or ref_info["sota"] == ""):
+                            match ref_info["reftype"]:
+                                case "Summit":
+                                    spot.sig_refs[0].sig = "GMA"
+                                    spot.sig = "GMA"
+                                case "IOTA Island":
+                                    spot.sig_refs[0].sig = "IOTA"
+                                    spot.sig = "IOTA"
+                                case "Lighthouse (ILLW)":
+                                    spot.sig_refs[0].sig = "ILLW"
+                                    spot.sig = "ILLW"
+                                case "Lighthouse (ARLHS)":
+                                    spot.sig_refs[0].sig = "ARLHS"
+                                    spot.sig = "ARLHS"
+                                case "Castle":
+                                    spot.sig_refs[0].sig = "WCA"
+                                    spot.sig = "WCA"
+                                case "Mill":
+                                    spot.sig_refs[0].sig = "MOTA"
+                                    spot.sig = "MOTA"
+                                case _:
+                                    logging.warn("GMA spot found with ref type " + ref_info[
+                                        "reftype"] + ", developer needs to add support for this!")
+                                    spot.sig_refs[0].sig = ref_info["reftype"]
+                                    spot.sig = ref_info["reftype"]
 
-                    # Add to our list. Don't worry about de-duping, removing old spots etc. at this point; other code will do
-                    # that for us.
-                    new_spots.append(spot)
+                            # Add to our list. Don't worry about de-duping, removing old spots etc. at this point; other code will do
+                            # that for us.
+                            new_spots.append(spot)
+                except:
+                    logging.warn("Exception when looking up " + self.REF_INFO_URL_ROOT + source_spot["REF"] + ", ignoring this spot for now")
         return new_spots

spotproviders/spot_provider.py

@@ -34,8 +34,9 @@ class SpotProvider:
             if datetime.fromtimestamp(spot.time, pytz.UTC) > self.last_spot_time:
                 # Fill in any blanks
                 spot.infer_missing()
-                # Add to the list
-                self.spots.add(spot.id, spot, expire=MAX_SPOT_AGE)
+                # Add to the list, provided it heas not already expired.
+                if not spot.expired():
+                    self.spots.add(spot.id, spot, expire=MAX_SPOT_AGE)
         self.last_spot_time = datetime.fromtimestamp(max(map(lambda s: s.time, spots)), pytz.UTC)
 
     # Submit a single spot retrieved from the provider. This will be added to the list regardless of its age. Spots
@@ -44,9 +45,10 @@ class SpotProvider:
     def submit(self, spot):
         # Fill in any blanks
         spot.infer_missing()
-        # Add to the list
-        self.spots.add(spot.id, spot, expire=MAX_SPOT_AGE)
-        self.last_spot_time = datetime.fromtimestamp(spot.time, pytz.UTC)
+        # Add to the list, provided it heas not already expired.
+        if not spot.expired():
+            self.spots.add(spot.id, spot, expire=MAX_SPOT_AGE)
+            self.last_spot_time = datetime.fromtimestamp(spot.time, pytz.UTC)
 
     # Stop any threads and prepare for application shutdown
     def stop(self):