implement overhaul of error handling

hyperglass/command/construct.py

@@ -136,7 +136,13 @@ class Construct:
             query = json.dumps({"query_type": query_type, "afi": afi, "target": target})
         elif self.transport == "scrape":
             conf_command = self.device_commands(self.device.nos, afi, query_type)
-            query = conf_command.format(target=target)
+            afis = []
+            for afi in self.device.afis:
+                split_afi = afi.split("v")
+                afis.append(
+                    "".join([split_afi[0].upper(), "v", split_afi[1], " Unicast|"])
+                )
+            query = conf_command.format(target=target, afis="".join(afis))
         logger.debug(f"Constructed query: {query}")
         return query
 
@@ -154,6 +160,12 @@ class Construct:
             query = json.dumps({"query_type": query_type, "afi": afi, "target": target})
         elif self.transport == "scrape":
             conf_command = self.device_commands(self.device.nos, afi, query_type)
-            query = conf_command.format(target=target)
+            afis = []
+            for afi in self.device.afis:
+                split_afi = afi.split("v")
+                afis.append(
+                    "".join([split_afi[0].upper(), "v", split_afi[1], " Unicast|"])
+                )
+            query = conf_command.format(target=target, afis="".join(afis))
         logger.debug(f"Constructed query: {query}")
         return query

hyperglass/command/execute.py

@@ -24,7 +24,6 @@ from hyperglass.configuration import logzero_config  # noqa: F401
 from hyperglass.configuration import params
 from hyperglass.configuration import proxies
 from hyperglass.constants import Supported
-from hyperglass.constants import code
 from hyperglass.constants import protocol_map
 from hyperglass.exceptions import AuthError, RestError, ScrapeError
 
@@ -74,6 +73,7 @@ class Connect:
                     self.device_config.port,
                 ),
                 local_bind_address=("localhost", 0),
+                skip_tunnel_checkup=False,
             ) as tunnel:
                 logger.debug(f"Established tunnel with {self.device_config.proxy}")
                 scrape_host = {
@@ -99,16 +99,24 @@ class Connect:
                     NetmikoTimeoutError,
                     sshtunnel.BaseSSHTunnelForwarderError,
                 ) as scrape_error:
+                    logger.error(
+                        f"Error connecting to device {self.device_config.location}"
+                    )
                     raise ScrapeError(
+                        params.messages.connection_error,
                         device=self.device_config.location,
                         proxy=self.device_config.proxy,
-                        error_msg=scrape_error,
+                        error=scrape_error,
                     ) from None
                 except (NetMikoAuthenticationException, NetmikoAuthError) as auth_error:
+                    logger.error(
+                        f"Error authenticating to device {self.device_config.location}"
+                    )
                     raise AuthError(
+                        params.messages.connection_error,
                         device=self.device_config.location,
                         proxy=self.device_config.proxy,
-                        error_msg=auth_error,
+                        error=auth_error,
                     ) from None
         else:
             scrape_host = {
@@ -134,16 +142,32 @@ class Connect:
                 NetmikoTimeoutError,
                 sshtunnel.BaseSSHTunnelForwarderError,
             ) as scrape_error:
+                logger.error(
+                    f"Error connecting to device {self.device_config.location}"
+                )
                 raise ScrapeError(
-                    device=self.device_config.location, error_msg=scrape_error
+                    params.messages.connection_error,
+                    device=self.device_config.location,
+                    proxy=None,
+                    error=scrape_error,
                 ) from None
             except (NetMikoAuthenticationException, NetmikoAuthError) as auth_error:
+                logger.error(
+                    f"Error authenticating to device {self.device_config.location}"
+                )
                 raise AuthError(
-                    device=self.device_config.location, error_msg=auth_error
+                    params.messages.connection_error,
+                    device=self.device_config.location,
+                    proxy=None,
+                    error=auth_error,
                 ) from None
         if not response:
+            logger.error(f"No response from device {self.device_config.location}")
             raise ScrapeError(
-                device=self.device_config.location, error_msg="No response"
+                params.messages.connection_error,
+                device=self.device_config.location,
+                proxy=None,
+                error="No response",
             )
         logger.debug(f"Output for query: {self.query}:\n{response}")
         return response
@@ -151,6 +175,7 @@ class Connect:
     async def rest(self):
         """Sends HTTP POST to router running a hyperglass API agent"""
         logger.debug(f"Query parameters: {self.query}")
+
         uri = Supported.map_rest(self.device_config.nos)
         headers = {
             "Content-Type": "application/json",
@@ -163,8 +188,10 @@ class Connect:
             port=self.device_config.port,
             uri=uri,
         )
+
         logger.debug(f"HTTP Headers: {headers}")
         logger.debug(f"URL endpoint: {endpoint}")
+
         try:
             http_client = httpx.AsyncClient()
             raw_response = await http_client.post(
@@ -172,7 +199,7 @@ class Connect:
             )
             response = raw_response.text
 
-            logger.debug(f"HTTP status code: {status}")
+            logger.debug(f"HTTP status code: {raw_response.status_code}")
             logger.debug(f"Output for query {self.query}:\n{response}")
         except (
             httpx.exceptions.ConnectTimeout,
@@ -193,8 +220,11 @@ class Connect:
             OSError,
         ) as rest_error:
             logger.error(f"Error connecting to device {self.device_config.location}")
-            logger.error(rest_error)
+            raise RestError(
-            raise RestError(device=self.device_config.location, error_msg=rest_error)
+                params.messages.connection_error,
+                device=self.device_config.location,
+                error=rest_error,
+            )
         return response
 
 
@@ -206,20 +236,22 @@ class Execute:
     """
 
     def __init__(self, lg_data):
-        self.input_data = lg_data
+        self.query_data = lg_data
-        self.input_location = self.input_data["location"]
+        self.query_location = self.query_data["location"]
-        self.input_type = self.input_data["query_type"]
+        self.query_type = self.query_data["query_type"]
-        self.input_target = self.input_data["target"]
+        self.query_target = self.query_data["target"]
 
     def parse(self, raw_output, nos):
         """
+        Deprecating: see #16
+
         Splits BGP raw output by AFI, returns only IPv4 & IPv6 output for
         protocol-agnostic commands (Community & AS_PATH Lookups).
         """
         logger.debug("Parsing raw output...")
 
         parsed = raw_output
-        if self.input_type in ("bgp_community", "bgp_aspath"):
+        if self.query_type in ("bgp_community", "bgp_aspath"):
             logger.debug(f"Parsing raw output for device type {nos}")
             if nos in ("cisco_ios",):
                 delimiter = "For address family: "
@@ -236,33 +268,26 @@ class Execute:
         Initializes Execute.filter(), if input fails to pass filter,
         returns errors to front end. Otherwise, executes queries.
         """
-        device_config = getattr(devices, self.input_location)
+        device_config = getattr(devices, self.query_location)
 
-        logger.debug(f"Received query for {self.input_data}")
+        logger.debug(f"Received query for {self.query_data}")
-        logger.debug(f"Matched device config:\n{device_config}")
+        logger.debug(f"Matched device config: {device_config}")
 
         # Run query parameters through validity checks
-        validity, msg, status = getattr(Validate(device_config), self.input_type)(
+        validation = Validate(device_config, self.query_type, self.query_target)
-            self.input_target
+        valid_input = validation.validate_query()
-        )
+        if valid_input:
-        if not validity:
+            logger.debug(f"Validation passed for query: {self.query_data}")
-            logger.debug("Invalid query")
+            pass
-            return (msg, status)
+
-        connection = None
+        connect = None
         output = params.messages.general
 
-        logger.debug(f"Validity: {validity}, Message: {msg}, Status: {status}")
-
         transport = Supported.map_transport(device_config.nos)
-        connection = Connect(
+        connect = Connect(device_config, self.query_type, self.query_target, transport)
-            device_config, self.input_type, self.input_target, transport
-        )
         if Supported.is_rest(device_config.nos):
-            raw_output, status = await connection.rest()
+            output = await connect.rest()
         elif Supported.is_scrape(device_config.nos):
-            raw_output, status = await connection.scrape()
+            output = await connect.scrape()
-        output = self.parse(raw_output, device_config.nos)
 
-        logger.debug(f"Parsed output for device type {device_config.nos}:\n{output}")
+        return output
-
-        return (output, status)

hyperglass/command/validate.py

@@ -85,10 +85,14 @@ def ip_validate(target):
     try:
         valid_ip = ipaddress.ip_network(target)
         if valid_ip.is_reserved or valid_ip.is_unspecified or valid_ip.is_loopback:
-            raise ValueError
+            _exception = ValueError(params.messages.invalid_input)
-    except (ipaddress.AddressValueError, ValueError):
+            _exception.details = {}
+            raise _exception
+    except (ipaddress.AddressValueError, ValueError) as ip_error:
         logger.debug(f"IP {target} is invalid")
-        raise ValueError
+        _exception = ValueError(ip_error)
+        _exception.details = {}
+        raise _exception
     return valid_ip
 
 
@@ -108,16 +112,18 @@ def ip_blacklist(target):
             if ipaddress.ip_network(net).version == target_ver
         ]
         logger.debug(
-            f"IPv{target_ver} Blacklist Networks: {[str(n) for n in networks]}"
+            f"IPv{target_ver} Blacklist Networks: {[str(net) for net in networks]}"
         )
         for net in networks:
             blacklist_net = ipaddress.ip_network(net)
             if (
                 blacklist_net.network_address <= target.network_address
-                and blacklist_net.network_address >= target.broadcast_address
+                and blacklist_net.broadcast_address >= target.broadcast_address
             ):
-                logger.debug(f"Blacklist Match Found for {target} in {net}")
+                logger.debug(f"Blacklist Match Found for {target} in {str(net)}")
-                raise ValueError(params.messages.blacklist)
+                _exception = ValueError(params.messages.blacklist)
+                _exception.details = {"blacklisted_net": str(net)}
+                raise _exception
     return target
 
 
@@ -153,7 +159,7 @@ def ip_type_check(query_type, target, device):
         if prefix_attr["length"] > max_length:
             logger.debug("Failed max prefix length check")
             _exception = ValueError(params.messages.max_prefix)
-            _exception.details = {"max_length": params.features.max_prefix}
+            _exception.details = {"max_length": max_length}
             raise _exception
 
     # If device NOS is listed in requires_ipv6_cidr.toml, and query is
@@ -166,7 +172,7 @@ def ip_type_check(query_type, target, device):
     ):
         logger.debug("Failed requires IPv6 CIDR check")
         _exception = ValueError(params.messages.requires_ipv6_cidr)
-        _exception.details = {"device_name": device.location}
+        _exception.details = {"device_name": device.display_name}
         raise _exception
 
     # If query type is ping or traceroute, and query target is in CIDR
@@ -174,7 +180,7 @@ def ip_type_check(query_type, target, device):
     if query_type in ("ping", "traceroute") and IPType().is_cidr(target):
         logger.debug("Failed CIDR format for ping/traceroute check")
         _exception = ValueError(params.messages.directed_cidr)
-        _exception.details = {}
+        _exception.details = {"query_type": getattr(params.branding.text, query_type)}
         raise _exception
     return target
 
@@ -202,18 +208,20 @@ class Validate:
             ip_validate(self.target)
         except ValueError as unformatted_error:
             raise InputInvalid(
-                unformatted_error,
+                params.messages.invalid_input,
                 target=self.target,
                 query_type=getattr(params.branding.text, self.query_type),
                 **unformatted_error.details,
-            ) from None
+            )
 
         # If target is a member of the blacklist, return an error.
         try:
             ip_blacklist(self.target)
         except ValueError as unformatted_error:
             raise InputNotAllowed(
-                unformatted_error, target=self.target, **unformatted_error.details
+                params.messages.blacklist,
+                target=self.target,
+                **unformatted_error.details,
             )
 
         # Perform further validation of a valid IP address, return an
@@ -222,7 +230,7 @@ class Validate:
             ip_type_check(self.query_type, self.target, self.device)
         except ValueError as unformatted_error:
             raise InputNotAllowed(
-                unformatted_error, target=self.target, **unformatted_error.details
+                str(unformatted_error), target=self.target, **unformatted_error.details
             )
 
         return self.target
@@ -267,7 +275,7 @@ class Validate:
                 )
         return self.target
 
-    def valdiate_query(self):
+    def validate_query(self):
         if self.query_type in ("bgp_community", "bgp_aspath"):
             return self.validate_dual()
         else:

hyperglass/exceptions.py

@@ -2,8 +2,6 @@
 Custom exceptions for hyperglass
 """
 
-from typing import Dict
-
 from hyperglass.constants import code
 
 
@@ -12,7 +10,17 @@ class HyperglassError(Exception):
     hyperglass base exception
     """
 
-    pass
+    def __init__(self, message="", status=500, keywords={}):
+        self.message = message
+        self.status = status
+        self.keywords = keywords
+
+    def __dict__(self):
+        return {
+            "message": self.message,
+            "status": self.status,
+            "keywords": self.keywords,
+        }
 
 
 class ConfigError(HyperglassError):
@@ -21,9 +29,9 @@ class ConfigError(HyperglassError):
     """
 
     def __init__(self, unformatted_msg, kwargs={}):
-        self.message: unformatted_msg.format(**kwargs)
+        self.message = unformatted_msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(message=self.message, keywords=self.keywords)
 
     def __str__(self):
         return self.message
@@ -33,11 +41,11 @@ class ConfigInvalid(HyperglassError):
     """Raised when a config item fails type or option validation"""
 
     def __init__(self, **kwargs):
-        self.message: str = 'The value field "{field}" is invalid: {error_msg}'.format(
+        self.message = 'The value field "{field}" is invalid: {error_msg}'.format(
             **kwargs
         )
-        self.keywords: Dict = kwargs
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(message=self.message, keywords=self.keywords)
 
     def __str__(self):
         return self.message
@@ -49,12 +57,12 @@ class ConfigMissing(HyperglassError):
     """
 
     def __init__(self, kwargs={}):
-        self.message: str = (
+        self.message = (
             "{missing_item} is missing or undefined and is required to start "
             "hyperglass. Please consult the installation documentation."
         ).format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(message=self.message, keywords=self.keywords)
 
     def __str__(self):
         return self.message
@@ -63,11 +71,13 @@ class ConfigMissing(HyperglassError):
 class ScrapeError(HyperglassError):
     """Raised upon a scrape/netmiko error"""
 
-    def __init__(self, kwargs={}):
+    def __init__(self, msg, kwargs={}):
-        self.message: str = "".format(**kwargs)
+        self.message = msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.target_error
-        self.status: int = code.target_error
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(
+            message=self.message, status=self.status, keywords=self.keywords
+        )
 
     def __str__(self):
         return self.message
@@ -76,11 +86,13 @@ class ScrapeError(HyperglassError):
 class AuthError(HyperglassError):
     """Raised when authentication to a device fails"""
 
-    def __init__(self, kwargs={}):
+    def __init__(self, msg, kwargs={}):
-        self.message: str = "".format(**kwargs)
+        self.message = msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.target_error
-        self.status: int = code.target_error
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(
+            message=self.message, status=self.status, keywords=self.keywords
+        )
 
     def __str__(self):
         return self.message
@@ -89,11 +101,13 @@ class AuthError(HyperglassError):
 class RestError(HyperglassError):
     """Raised upon a rest API client error"""
 
-    def __init__(self, kwargs={}):
+    def __init__(self, msg, kwargs={}):
-        self.message: str = "".format(**kwargs)
+        self.message = msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.target_error
-        self.status: int = code.target_error
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(
+            message=self.message, status=self.status, keywords=self.keywords
+        )
 
     def __str__(self):
         return self.message
@@ -103,10 +117,12 @@ class InputInvalid(HyperglassError):
     """Raised when input validation fails"""
 
     def __init__(self, unformatted_msg, **kwargs):
-        self.message: str = unformatted_msg.format(**kwargs)
+        self.message = unformatted_msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.invalid
-        self.status: int = code.invalid
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.status)
+        super().__init__(
+            message=self.message, status=self.status, keywords=self.keywords
+        )
 
     def __str__(self):
         return self.message
@@ -119,25 +135,12 @@ class InputNotAllowed(HyperglassError):
     """
 
     def __init__(self, unformatted_msg, **kwargs):
-        self.message: str = unformatted_msg.format(**kwargs)
+        self.message = unformatted_msg.format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.invalid
-        self.status: int = code.invalid
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.status, self.message)
+        super().__init__(
-
+            message=self.message, status=self.status, keywords=self.keywords
-    def __str__(self):
+        )
-        return self.message
-
-
-class ParseError(HyperglassError):
-    """
-    Raised when an ouput parser encounters an error.
-    """
-
-    def __init__(self, kwargs={}):
-        self.message: str = "".format(**kwargs)
-        self.keywords: Dict = kwargs
-        self.status: int = code.target_error
-        super().__init__(self.message, self.keywords)
 
     def __str__(self):
         return self.message
@@ -149,10 +152,12 @@ class UnsupportedDevice(HyperglassError):
     """
 
     def __init__(self, kwargs={}):
-        self.message: str = "".format(**kwargs)
+        self.message = "".format(**kwargs)
-        self.keywords: Dict = kwargs
+        self.status = code.target_error
-        self.status: int = code.target_error
+        self.keywords = [value for value in kwargs.values()]
-        super().__init__(self.message, self.keywords)
+        super().__init__(
+            message=self.message, status=self.status, keywords=self.keywords
+        )
 
     def __str__(self):
         return self.message

hyperglass/hyperglass.py

@@ -17,6 +17,7 @@ from sanic import Sanic
 from sanic import response
 from sanic.exceptions import NotFound
 from sanic.exceptions import ServerError
+from sanic.exceptions import InvalidUsage
 from sanic_limiter import Limiter
 from sanic_limiter import RateLimitExceeded
 from sanic_limiter import get_remote_address
@@ -27,10 +28,16 @@ from hyperglass.command.execute import Execute
 from hyperglass.configuration import devices
 from hyperglass.configuration import logzero_config  # noqa: F401
 from hyperglass.configuration import params
-from hyperglass.configuration import display_networks
 from hyperglass.constants import Supported
 from hyperglass.constants import code
-from hyperglass.exceptions import HyperglassError
+from hyperglass.exceptions import (
+    HyperglassError,
+    AuthError,
+    ScrapeError,
+    RestError,
+    InputInvalid,
+    InputNotAllowed,
+)
 
 logger.debug(f"Configuration Parameters:\n {params.dict()}")
 
@@ -117,6 +124,35 @@ async def metrics(request):
     )
 
 
+@app.exception(InvalidUsage)
+async def handle_ui_errors(request, exception):
+    """Renders full error page for invalid URI"""
+    client_addr = get_remote_address(request)
+    error = exception.args[0]
+    status = error["status"]
+    logger.info(error)
+    count_errors.labels(
+        status,
+        code.get_reason(status),
+        client_addr,
+        request.json["query_type"],
+        request.json["location"],
+        request.json["target"],
+    ).inc()
+    logger.error(f'Error: {error["message"]}, Source: {client_addr}')
+    return response.json(
+        {"output": error["message"], "status": status, "keywords": error["keywords"]},
+        status=status,
+    )
+
+
+@app.exception(ServerError)
+async def handle_missing(request, exception):
+    """Renders full error page for invalid URI"""
+    logger.error(f"Error: {exception}")
+    return response.json(exception, status=code.invalid)
+
+
 @app.exception(NotFound)
 async def handle_404(request, exception):
     """Renders full error page for invalid URI"""
@@ -128,15 +164,6 @@ async def handle_404(request, exception):
     return response.html(html, status=404)
 
 
-@app.exception(ServerError)
-async def handle_408(request, exception):
-    """Renders full error page for invalid URI"""
-    client_addr = get_remote_address(request)
-    count_notfound.labels(exception, path, client_addr).inc()
-    logger.error(f"Error: {exception}, Source: {client_addr}")
-    return response.html(exception, status=408)
-
-
 @app.exception(RateLimitExceeded)
 async def handle_429(request, exception):
     """Renders full error page for too many site queries"""
@@ -196,17 +223,17 @@ async def hyperglass_main(request):
     # Return error if no target is specified
     if not lg_data["target"]:
         logger.debug("No input specified")
-        return response.html(params.messages.no_input, status=code.invalid)
+        raise handle_missing(request, params.messages.no_input)
 
     # Return error if no location is selected
     if lg_data["location"] not in devices.hostnames:
         logger.debug("No selection specified")
-        return response.html(params.messages.no_location, status=code.invalid)
+        raise handle_missing(request, params.messages.no_input)
 
     # Return error if no query type is selected
     if not Supported.is_supported_query(lg_data["query_type"]):
         logger.debug("No query specified")
-        return response.html(params.messages.no_query_type, status=code.invalid)
+        raise handle_missing(request, params.messages.no_input)
 
     # Get client IP address for Prometheus logging & rate limiting
     client_addr = get_remote_address(request)
@@ -233,13 +260,22 @@ async def hyperglass_main(request):
 
         # Pass request to execution module
         starttime = time.time()
+        try:
             cache_value = await Execute(lg_data).response()
+        except (
+            AuthError,
+            RestError,
+            ScrapeError,
+            InputInvalid,
+            InputNotAllowed,
+        ) as backend_error:
+            raise InvalidUsage(backend_error.__dict__())
+
         endtime = time.time()
+        elapsedtime = round(endtime - starttime, 4)
 
         if not cache_value:
-            raise handle_408(params.messages.request_timeout)
+            raise handle_ui_errors(request, params.messages.request_timeout)
-
-        elapsedtime = round(endtime - starttime, 4)
 
         logger.debug(
             f"Execution for query {cache_key} took {elapsedtime} seconds to run."
@@ -255,21 +291,11 @@ async def hyperglass_main(request):
     cache_response = await r_cache.get(cache_key)
 
     # Serialize stringified tuple response from cache
-    serialized_response = literal_eval(cache_response)
+    # serialized_response = literal_eval(cache_response)
-    response_output, response_status = serialized_response
+    # response_output, response_status = serialized_response
+    response_output = cache_response
 
     logger.debug(f"Cache match for: {cache_key}, returning cached entry")
     logger.debug(f"Cache Output: {response_output}")
-    logger.debug(f"Cache Status Code: {response_status}")
 
-    # If error, increment Prometheus metrics
+    return response.json({"output": response_output}, status=200)
-    if response_status in [405, 415, 504]:
-        count_errors.labels(
-            response_status,
-            code.get_reason(response_status),
-            client_addr,
-            lg_data["query_type"],
-            lg_data["location"],
-            lg_data["target"],
-        ).inc()
-    return response.json({"output": response_output}, status=response_status)