Improve error handling (#61)

htmlproofer/plugin.py

@@ -101,17 +101,18 @@ def on_post_page(self, output_content: str, page: Page, config: Config) -> None:
 
             url_status = self.get_url_status(url, page.file.src_path, all_element_ids, self.files, use_directory_urls)
 
-            if self.bad_url(url_status) is True:
-                error = f'invalid url - {url} [{url_status}] [{page.file.src_path}]'
-
-                is_error = self.is_error(self.config, url, url_status)
-                if self.config['raise_error'] and is_error:
-                    raise PluginError(error)
-                elif self.config['raise_error_after_finish'] and is_error and not self.invalid_links:
-                    log_error(error)
-                    self.invalid_links = True
-                if is_error:
-                    log_warning(error)
+            if self.bad_url(url_status) and self.is_error(self.config, url, url_status):
+                self.report_invalid_url(url, url_status, page.file.src_path)
+
+    def report_invalid_url(self, url, url_status, src_path):
+        error = f'invalid url - {url} [{url_status}] [{src_path}]'
+        if self.config['raise_error']:
+            raise PluginError(error)
+        elif self.config['raise_error_after_finish']:
+            log_error(error)
+            self.invalid_links = True
+        else:
+            log_warning(error)
 
     def get_external_url(self, url, scheme, src_path):
         try:
@@ -155,8 +156,12 @@ def get_url_status(
             # Markdown file, so disable target anchor validation in this case. Examples include:
             # ../..#BAD_ANCHOR style links to index.html and extra ../ inserted into relative
             # links.
-            if not self.is_url_target_valid(url, src_path, files):
-                return 404
+            is_valid = self.is_url_target_valid(url, src_path, files)
+            url_status = 404
+            if not is_valid and self.is_error(self.config, url, url_status):
+                log_warning(f"Unable to locate source file for: {url}")
+                return url_status
+            return 0
         return 0
 
     @staticmethod
@@ -168,7 +173,7 @@ def is_url_target_valid(url: str, src_path: str, files: Dict[str, File]) -> bool
         url_target, _, optional_anchor = match.groups()
         _, extension = os.path.splitext(url_target)
         if extension == ".html":
-            # URL is a link to another local Markdown file that may includes an anchor.
+            # URL is a link to another local Markdown file that may include an anchor.
             target_markdown = HtmlProoferPlugin.find_target_markdown(url_target, src_path, files)
             if target_markdown is None:
                 # The corresponding Markdown page was not found.
@@ -178,7 +183,6 @@ def is_url_target_valid(url: str, src_path: str, files: Dict[str, File]) -> bool
                 return False
         elif HtmlProoferPlugin.find_source_file(url_target, src_path, files) is None:
             return False
-
         return True
 
     @staticmethod
@@ -204,7 +208,6 @@ def find_source_file(url: str, src_path: str, files: Dict[str, File]) -> Optiona
         try:
             return files[search_path]
         except KeyError:
-            utils.log.warning(f"Unable to locate source file for: {url}")
             return None
 
     @staticmethod

tests/integration/docs/index.md

@@ -6,6 +6,7 @@
 
 * [Installation](#installation)
 * [Acknowledgement](#acknowledge)
+* [Browse Tests](../../../tests)
 
 ## Installation
 

tests/integration/mkdocs.yml

@@ -9,7 +9,10 @@ plugins:
         raise_error_after_finish: True
         raise_error_excludes:
           504: ['*']
-          404: ['https://www.mkdocs.org/user-guide/plugin',
-                '#acknowledge',
-                '../index.html#BAD_ANCHOR',
-                'page2.html#BAD_ANCHOR']
+          404: [
+            'https://www.mkdocs.org/user-guide/plugin',
+            '#acknowledge',
+            '../index.html#BAD_ANCHOR',
+            'page2.html#BAD_ANCHOR',
+            '../../../tests',
+          ]

tests/unit/test_plugin.py

@@ -9,6 +9,7 @@
 import pytest
 from requests import Response
 
+import htmlproofer.plugin
 from htmlproofer.plugin import HtmlProoferPlugin
 
 
@@ -316,3 +317,95 @@ def test_get_url_status__local_page_nested(plugin):
     assert plugin.get_url_status('foo/baz/nested.html#nested-two', 'index.md', set(), files, False) == 0
 
     assert plugin.get_url_status('/index.html', 'foo/baz/sibling.md', set(), files, False) == 0
+
+
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_get_url_status__excluded_non_existing_relative_url__no_warning(log_warning_mock, plugin):
+    url_status = 404
+    url = "non-existing.html"
+    src_path = "index.md"
+    files = {}
+    plugin.config['raise_error_excludes'][url_status] = [url]
+
+    status = plugin.get_url_status(url, src_path, set(), files, False)
+
+    log_warning_mock.assert_not_called()
+    assert 0 == status
+
+
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_get_url_status__excluded_existing_relative_url__no_warning(log_warning_mock, plugin):
+    url_status = 404
+    filename = "existing"
+    url = f"{filename}.html"
+    src_path = f"{filename}.md"
+    existing_page = Mock(spec=Page, markdown='')
+    files = {
+        os.path.normpath(file.url): file for file in Files([
+            Mock(spec=File, src_path=src_path, dest_path=url, url=url, page=existing_page)
+        ])
+    }
+    plugin.config['raise_error_excludes'][url_status] = [url]
+
+    status = plugin.get_url_status(url, src_path, set(), files, False)
+
+    log_warning_mock.assert_not_called()
+    assert 0 == status
+
+
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_get_url_status__non_existing_relative_url__warning_and_404(log_warning_mock, plugin):
+    expected_url_status = 404
+    url = "non-existing.html"
+    src_path = "index.md"
+    files = {}
+
+    status = plugin.get_url_status(url, src_path, set(), files, False)
+
+    log_warning_mock.assert_called_once()
+    assert expected_url_status == status
+
+
+def test_report_invalid_url__raise_error__highest_priority(plugin):
+    plugin.config['raise_error'] = True
+    plugin.config['raise_error_after_finish'] = True
+
+    with pytest.raises(PluginError):
+        plugin.report_invalid_url(url='', url_status=404, src_path="")
+
+
+@patch.object(htmlproofer.plugin, "log_error", autospec=True)
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_report_invalid_url__raise_error__raises_and_no_log(log_warning_mock, log_error_mock, plugin):
+    plugin.config['raise_error'] = True
+
+    with pytest.raises(PluginError):
+        plugin.report_invalid_url(url='', url_status=404, src_path="")
+    log_warning_mock.assert_not_called()
+    log_error_mock.assert_not_called()
+
+
+@patch.object(htmlproofer.plugin, "log_error", autospec=True)
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_report_invalid_url__raise_error_after_finish__log_error_is_called(log_warning_mock, log_error_mock, plugin):
+    plugin.config['raise_error'] = False
+    plugin.config['raise_error_after_finish'] = True
+
+    plugin.report_invalid_url(url='', url_status=404, src_path="")
+
+    log_warning_mock.assert_not_called()
+    log_error_mock.assert_called_once()
+    assert plugin.invalid_links
+
+
+@patch.object(htmlproofer.plugin, "log_error", autospec=True)
+@patch.object(htmlproofer.plugin, "log_warning", autospec=True)
+def test_report_invalid_url__not_raise_error__only_log_warning_is_called(log_warning_mock, log_error_mock, plugin):
+    plugin.config['raise_error'] = False
+    plugin.config['raise_error_after_finish'] = False
+
+    plugin.report_invalid_url(url='', url_status=404, src_path="")
+
+    log_warning_mock.assert_called_once()
+    log_error_mock.assert_not_called()
+    assert not plugin.invalid_links