From fe3eb30892618a3993e82316bb24440f5ac4b3ec Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 9 Aug 2015 18:47:36 +0200 Subject: [PATCH] Reorganize exceptions in urlutils. - Instead of ValueError, a new InvalidUrlError is raised with invalid URLs. - FuzzyUrlError got removed as it's basically the same as InvalidUrlError. --- qutebrowser/app.py | 4 +- qutebrowser/browser/commands.py | 6 +-- qutebrowser/browser/urlmarks.py | 2 +- qutebrowser/config/configtypes.py | 2 +- qutebrowser/utils/urlutils.py | 49 +++++++++++------------- tests/utils/test_urlutils.py | 62 +++++++++++++++++-------------- 6 files changed, 62 insertions(+), 63 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 927d43b29..6ca23ba21 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -272,7 +272,7 @@ def process_pos_args(args, via_ipc=False, cwd=None): log.init.debug("Startup URL {}".format(cmd)) try: url = urlutils.fuzzy_url(cmd, cwd, relative=True) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: message.error('current', "Error in startup argument '{}': " "{}".format(cmd, e)) else: @@ -302,7 +302,7 @@ def _open_startpage(win_id=None): for urlstr in config.get('general', 'startpage'): try: url = urlutils.fuzzy_url(urlstr, do_search=False) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: message.error('current', "Error when opening startpage: " "{}".format(e)) tabbed_browser.tabopen(QUrl('about:blank')) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index ef7a16933..5a07dbb56 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -303,7 +303,7 @@ class CommandDispatcher: else: try: url = urlutils.fuzzy_url(url) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: raise cmdexc.CommandError(e) if tab or bg or window: self._open(url, tab, bg, window) @@ -869,7 +869,7 @@ class CommandDispatcher: log.misc.debug("{} contained: '{}'".format(target, text)) try: url = urlutils.fuzzy_url(text) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: raise cmdexc.CommandError(e) self._open(url, tab, bg, window) @@ -1063,7 +1063,7 @@ class CommandDispatcher: """ try: url = urlutils.fuzzy_url(url) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: raise cmdexc.CommandError(e) self._open(url, tab, bg, window) diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py index 735f05e13..cacc8d9c0 100644 --- a/qutebrowser/browser/urlmarks.py +++ b/qutebrowser/browser/urlmarks.py @@ -225,7 +225,7 @@ class QuickmarkManager(UrlMarkManager): urlstr = self.marks[name] try: url = urlutils.fuzzy_url(urlstr, do_search=False) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: raise InvalidUrlError( "Invalid URL for quickmark {}: {}".format(name, str(e))) return url diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index dd4f19d79..5dfc43692 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -1113,7 +1113,7 @@ class FuzzyUrl(BaseType): from qutebrowser.utils import urlutils try: self.transform(value) - except urlutils.FuzzyUrlError as e: + except urlutils.InvalidUrlError as e: raise configexc.ValidationError(value, str(e)) def transform(self, value): diff --git a/qutebrowser/utils/urlutils.py b/qutebrowser/utils/urlutils.py index 5d8987138..16acbb9aa 100644 --- a/qutebrowser/utils/urlutils.py +++ b/qutebrowser/utils/urlutils.py @@ -37,6 +37,22 @@ from qutebrowser.commands import cmdexc # https://github.com/The-Compiler/qutebrowser/issues/108 +class InvalidUrlError(ValueError): + + """Error raised if a function got an invalid URL. + + Inherits ValueError because that was the exception originally used for + that, so there still might be some code around which checks for that. + """ + + def __init__(self, url): + if url.isValid(): + raise ValueError("Got valid URL {}!".format(url.toDisplayString())) + self.url = url + self.msg = get_errstring(url) + super().__init__(self.msg) + + def _parse_search_term(s): """Get a search engine name and search term from a string. @@ -185,7 +201,7 @@ def fuzzy_url(urlstr, cwd=None, relative=False, do_search=True): qtutils.ensure_valid(url) else: if not url.isValid(): - raise FuzzyUrlError("Invalid URL '{}'!".format(urlstr), url) + raise InvalidUrlError(url) return url @@ -355,7 +371,7 @@ def host_tuple(url): This is suitable to identify a connection, e.g. for SSL errors. """ if not url.isValid(): - raise ValueError(get_errstring(url)) + raise InvalidUrlError(url) scheme, host, port = url.scheme(), url.host(), url.port() assert scheme if not host: @@ -405,9 +421,9 @@ def same_domain(url1, url2): True if the domains are the same, False otherwise. """ if not url1.isValid(): - raise ValueError(get_errstring(url1)) + raise InvalidUrlError(url1) if not url2.isValid(): - raise ValueError(get_errstring(url2)) + raise InvalidUrlError(url2) suffix1 = url1.topLevelDomain() suffix2 = url2.topLevelDomain() @@ -422,29 +438,6 @@ def same_domain(url1, url2): return domain1 == domain2 -class FuzzyUrlError(Exception): - - """Exception raised by fuzzy_url on problems. - - Attributes: - msg: The error message to use. - url: The QUrl which caused the error. - """ - - def __init__(self, msg, url=None): - super().__init__(msg) - if url is not None and url.isValid(): - raise ValueError("Got valid URL {}!".format(url.toDisplayString())) - self.url = url - self.msg = msg - - def __str__(self): - if self.url is None or not self.url.errorString(): - return self.msg - else: - return '{}: {}'.format(self.msg, self.url.errorString()) - - class IncDecError(Exception): """Exception raised by incdec_number on problems. @@ -476,7 +469,7 @@ def incdec_number(url, incdec): Raises IncDecError if the url contains no number. """ if not url.isValid(): - raise ValueError(get_errstring(url)) + raise InvalidUrlError(url) path = url.path() # Get the last number in a string diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py index cb07ca80e..cc2478133 100644 --- a/tests/utils/test_urlutils.py +++ b/tests/utils/test_urlutils.py @@ -219,7 +219,7 @@ class TestFuzzyUrl: @pytest.mark.parametrize('do_search, exception', [ (True, qtutils.QtValueError), - (False, urlutils.FuzzyUrlError), + (False, urlutils.InvalidUrlError), ]) def test_invalid_url(self, do_search, exception, is_url_mock, monkeypatch): """Test with an invalid URL.""" @@ -469,34 +469,40 @@ def test_host_tuple(qurl, tpl): assert urlutils.host_tuple(qurl) == tpl -@pytest.mark.parametrize('url, raising, has_err_string', [ - (None, False, False), - (QUrl(), False, False), - (QUrl('http://www.example.com/'), True, False), - (QUrl('://'), False, True), -]) -def test_fuzzy_url_error(url, raising, has_err_string): - """Test FuzzyUrlError. +class TestInvalidUrlError: - Args: - url: The URL to pass to FuzzyUrlError. - raising; True if the FuzzyUrlError should raise itself. - has_err_string: Whether the QUrl is expected to have errorString set. - """ - if raising: - expected_exc = ValueError - else: - expected_exc = urlutils.FuzzyUrlError + @pytest.mark.parametrize('url, raising, has_err_string', [ + (QUrl(), False, False), + (QUrl('http://www.example.com/'), True, False), + (QUrl('://'), False, True), + ]) + def test_invalid_url_error(self, url, raising, has_err_string): + """Test InvalidUrlError. - with pytest.raises(expected_exc) as excinfo: - raise urlutils.FuzzyUrlError("Error message", url) - - if not raising: - if has_err_string: - expected_text = "Error message: " + url.errorString() + Args: + url: The URL to pass to InvalidUrlError. + raising; True if the InvalidUrlError should raise itself. + has_err_string: Whether the QUrl is expected to have errorString + set. + """ + if raising: + expected_exc = ValueError else: - expected_text = "Error message" - assert str(excinfo.value) == expected_text + expected_exc = urlutils.InvalidUrlError + + with pytest.raises(expected_exc) as excinfo: + raise urlutils.InvalidUrlError(url) + + if not raising: + expected_text = "Invalid URL" + if has_err_string: + expected_text += " - " + url.errorString() + assert str(excinfo.value) == expected_text + + def test_value_error_subclass(self): + """Make sure InvalidUrlError is a ValueError subclass.""" + with pytest.raises(ValueError): + raise urlutils.InvalidUrlError(QUrl()) @pytest.mark.parametrize('are_same, url1, url2', [ @@ -522,7 +528,7 @@ def test_same_domain(are_same, url1, url2): ]) def test_same_domain_invalid_url(url1, url2): """Test same_domain with invalid URLs.""" - with pytest.raises(ValueError): + with pytest.raises(urlutils.InvalidUrlError): urlutils.same_domain(QUrl(url1), QUrl(url2)) class TestIncDecNumber: @@ -570,7 +576,7 @@ class TestIncDecNumber: def test_invalid_url(self): """Test if incdec_number rejects an invalid URL.""" - with pytest.raises(ValueError): + with pytest.raises(urlutils.InvalidUrlError): urlutils.incdec_number(QUrl(""), "increment") def test_wrong_mode(self):