WIP: Properly signal scheme errors

qutebrowser/browser/qutescheme.py

@@ -60,36 +60,39 @@ csrf_token = None
 _HANDLERS = {}
 
 
-class NoHandlerFound(Exception):
+class Error(Exception):
 
-    """Raised when no handler was found for the given URL."""
+    """Exception for generic errors on a qute:// page."""
 
     pass
 
 
-class QuteSchemeOSError(Exception):
+class NotFoundError(Error):
 
-    """Called when there was an OSError inside a handler."""
+    """Raised when the given URL was not found."""
 
     pass
 
 
-class QuteSchemeError(Exception):
+class SchemeOSError(Error):
 
-    """Exception to signal that a handler should return an ErrorReply.
+    """Raised when there was an OSError inside a handler."""
 
-    Attributes correspond to the arguments in
+    pass
-    networkreply.ErrorNetworkReply.
 
-    Attributes:
-        errorstring: Error string to print.
-        error: Numerical error value.
-    """
 
-    def __init__(self, errorstring, error):
+class UrlInvalidError(Error):
-        self.errorstring = errorstring
+
-        self.error = error
+    """Raised when an invalid URL was opened."""
-        super().__init__(errorstring)
+
+    pass
+
+
+class RequestDeniedError(Error):
+
+    """Raised when the request is forbidden."""
+
+    pass
 
 
 class Redirect(Exception):
@@ -180,13 +183,13 @@ def data_for_url(url):
     try:
         handler = _HANDLERS[host]
     except KeyError:
-        raise NoHandlerFound(url)
+        raise NotFoundError("No handler found for {}".format(
+            url.toDisplayString()))
 
     try:
         mimetype, data = handler(url)
     except OSError as e:
-        # FIXME:qtwebengine how to handle this?
+        raise SchemeOSError(e)
-        raise QuteSchemeOSError(e)
 
     assert mimetype is not None, url
     if mimetype == 'text/html' and isinstance(data, str):
@@ -262,13 +265,13 @@ def qute_history(url):
             offset = QUrlQuery(url).queryItemValue("offset")
             offset = int(offset) if offset else None
         except ValueError as e:
-            raise QuteSchemeError("Query parameter offset is invalid", e)
+            raise UrlInvalidError("Query parameter offset is invalid")
         # Use start_time in query or current time.
         try:
             start_time = QUrlQuery(url).queryItemValue("start_time")
             start_time = float(start_time) if start_time else time.time()
         except ValueError as e:
-            raise QuteSchemeError("Query parameter start_time is invalid", e)
+            raise UrlInvalidError("Query parameter start_time is invalid")
 
         return 'text/html', json.dumps(history_data(start_time, offset))
     else:
@@ -290,7 +293,7 @@ def qute_javascript(url):
         path = "javascript" + os.sep.join(path.split('/'))
         return 'text/html', utils.read_file(path, binary=False)
     else:
-        raise QuteSchemeError("No file specified", ValueError())
+        raise UrlInvalidError("No file specified")
 
 
 @add_handler('pyeval')
@@ -379,7 +382,7 @@ def qute_help(url):
         try:
             bdata = utils.read_file(path, binary=True)
         except OSError as e:
-            raise QuteSchemeOSError(e)
+            raise SchemeOSError(e)
         mimetype, _encoding = mimetypes.guess_type(urlpath)
         assert mimetype is not None, url
         return mimetype, bdata
@@ -461,8 +464,7 @@ def qute_settings(url):
     if url.path() == '/set':
         if url.password() != csrf_token:
             message.error("Invalid CSRF token for qute://settings!")
-            raise QuteSchemeError("Invalid CSRF token!",
+            raise RequestDeniedError("Invalid CSRF token!")
-                                  QNetworkReply.ContentAccessDenied)
         return _qute_settings_set(url)
 
     # Requests to qute://settings/set should only be allowed from

qutebrowser/browser/webengine/webenginequtescheme.py

@@ -80,16 +80,19 @@ class QuteSchemeHandler(QWebEngineUrlSchemeHandler):
         log.misc.debug("Got request for {}".format(url.toDisplayString()))
         try:
             mimetype, data = qutescheme.data_for_url(url)
-        except qutescheme.NoHandlerFound:
+        except qutescheme.NotFoundError:
-            log.misc.debug("No handler found for {}".format(
+            log.misc.exception("Error while handling qute://* URL")
-                url.toDisplayString()))
             job.fail(QWebEngineUrlRequestJob.UrlNotFound)
-        except qutescheme.QuteSchemeOSError:
+        except qutescheme.UrlInvalidError:
-            # FIXME:qtwebengine how do we show a better error here?
+            log.misc.exception("Error while handling qute://* URL")
+            job.fail(QWebEngineUrlRequestJob.UrlInvalid)
+        except qutescheme.RequestDeniedError:
+            log.misc.exception("Error while handling qute://* URL")
+            job.fail(QWebEngineUrlRequestJob.RequestDenied)
+        except qutescheme.SchemeOSError:
             log.misc.exception("OSError while handling qute://* URL")
             job.fail(QWebEngineUrlRequestJob.UrlNotFound)
-        except qutescheme.QuteSchemeError:
+        except qutescheme.Error:
-            # FIXME:qtwebengine how do we show a better error here?
             log.misc.exception("Error while handling qute://* URL")
             job.fail(QWebEngineUrlRequestJob.RequestFailed)
         except qutescheme.Redirect as e:

qutebrowser/browser/webkit/network/webkitqutescheme.py

@@ -59,15 +59,21 @@ def handler(request, operation, current_url):
 
     try:
         mimetype, data = qutescheme.data_for_url(url)
-    except qutescheme.NoHandlerFound:
+    except qutescheme.NotFoundError as e:
-        errorstr = "No handler found for {}!".format(url.toDisplayString())
-        return networkreply.ErrorNetworkReply(
-            request, errorstr, QNetworkReply.ContentNotFoundError)
-    except qutescheme.QuteSchemeOSError as e:
         return networkreply.ErrorNetworkReply(
             request, str(e), QNetworkReply.ContentNotFoundError)
-    except qutescheme.QuteSchemeError as e:
+    except qutescheme.SchemeOSError as e:
-        return networkreply.ErrorNetworkReply(request, e.errorstring, e.error)
+        return networkreply.ErrorNetworkReply(
+            request, str(e), QNetworkReply.ContentNotFoundError)
+    except qutescheme.UrlInvalidError as e:
+        return networkreply.ErrorNetworkReply(
+            request, str(e), QNetworkReply.ContentOperationNotPermittedError)
+    except qutescheme.RequestDeniedError as e:
+        return networkreply.ErrorNetworkReply(
+            request, str(e), QNetworkReply.ContentAccessDenied)
+    except qutescheme.Error as e:
+        return networkreply.ErrorNetworkReply(
+            request, str(e), QNetworkReply.InternalServerError)
     except qutescheme.Redirect as e:
         qtutils.ensure_valid(e.url)
         return networkreply.RedirectNetworkReply(e.url)
@@ -86,9 +92,8 @@ def qute_pdfjs(url):
         # information, as the failed pdfjs requests are still in the log.
         log.misc.warning(
             "pdfjs resource requested but not found: {}".format(e.path))
-        raise qutescheme.QuteSchemeError("Can't find pdfjs resource "
+        raise qutescheme.NotFoundError("Can't find pdfjs resource '{}'".format(
-                                         "'{}'".format(e.path),
+            e.path))
-                                         QNetworkReply.ContentNotFoundError)
     else:
         mimetype, _encoding = mimetypes.guess_type(url.fileName())
         assert mimetype is not None, url

tests/unit/browser/test_qutescheme.py

@@ -61,13 +61,13 @@ class TestJavascriptHandler:
     def test_qutejavascript_404(self):
         url = QUrl("qute://javascript/404.js")
 
-        with pytest.raises(qutescheme.QuteSchemeOSError):
+        with pytest.raises(qutescheme.SchemeOSError):
             qutescheme.data_for_url(url)
 
     def test_qutejavascript_empty_query(self):
         url = QUrl("qute://javascript")
 
-        with pytest.raises(qutescheme.QuteSchemeError):
+        with pytest.raises(qutescheme.InvalidURLError):
             qutescheme.qute_javascript(url)
 
 

tests/unit/browser/webkit/network/test_webkitqutescheme.py

@@ -56,7 +56,7 @@ class TestPDFJSHandler:
     def test_nonexisting_resource(self, caplog):
         """Test with a resource that does not exist."""
         with caplog.at_level(logging.WARNING, 'misc'):
-            with pytest.raises(qutescheme.QuteSchemeError):
+            with pytest.raises(qutescheme.NotFoundError):
                 qutescheme.data_for_url(QUrl('qute://pdfjs/no/file.html'))
         assert len(caplog.records) == 1
         assert (caplog.records[0].message ==