From 486022f764b7495cdc1f4014711bcae3e505c505 Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Thu, 31 Aug 2023 08:43:09 -0600
Subject: [PATCH] Adding try/except around BeautifulSoup parsing.  Testing
 finding HTML documents.

---
 app/src/components/Download.js        |  5 ++--
 wrolpi/scrape_downloader.py           | 19 +++++++++----
 wrolpi/tags/test_scrape_downloader.py | 40 ++++++++++++++++++++++-----
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/app/src/components/Download.js b/app/src/components/Download.js
index 27408a5c..95253d0c 100644
--- a/app/src/components/Download.js
+++ b/app/src/components/Download.js
@@ -531,9 +531,10 @@ class ScrapeDownloader extends Downloader {
                     </Grid.Column>
                     <Grid.Column width={6}>
                         <SForm.Field required>
-                            <label>Depth
+                            <label>
+                                Depth
                                 <HelpPopup
-                                    content='Search the URLs provided, and any pages they contain up to this depth. Warning! This can be exponential!'/>
+                                    content='Search the URLs provided, and any URLs they contain up to this depth. Warning! This can be exponential!'/>
                             </label>
                             <FormDropdown selection
                                           name='depth'
diff --git a/wrolpi/scrape_downloader.py b/wrolpi/scrape_downloader.py
index eeaa7b9a..bd75f4c4 100644
--- a/wrolpi/scrape_downloader.py
+++ b/wrolpi/scrape_downloader.py
@@ -17,9 +17,10 @@ def resolve_url(parent_url: str, url: str):
 
     parsed = urlparse(url)
     if parsed.scheme:
-        # URL is absolute.
+        # URL is complete or external.
         return url
     elif parsed.path.startswith('/'):
+        # URL is absolute from domain.
         return f'{parsed_domain.scheme}://{parsed_domain.netloc}{url}'
 
     # Assume URL is relative to the parent URL page.
@@ -76,13 +77,24 @@ class ScrapeHTMLDownloader(Downloader):
             local_urls = urls.copy()
             urls = list()
             for url in local_urls:
+                if page_count >= max_pages:
+                    logger.warning('Reached max page count.')
+                    break
+
+                # Get the HTML of the URL.
                 content = await self.fetch_http(url)
                 page_count += 1
                 if not content:
                     logger.error(f'Failed to download url: {url}')
                     continue
 
-                soup = bs4.BeautifulSoup(content, 'html.parser')
+                # Find all anchors, search for matching files, and more URLs to search.
+                try:
+                    soup = bs4.BeautifulSoup(content, 'html.parser')
+                except Exception as e:
+                    logger.error(f'Failed to parse HTML from {url}', exc_info=e)
+                    continue
+
                 for a in soup.find_all('a'):
                     try:
                         href: str = a['href']
@@ -96,9 +108,6 @@ class ScrapeHTMLDownloader(Downloader):
                         download_urls.append(child_url)
                     else:
                         urls.append(child_url)
-                if page_count >= max_pages:
-                    logger.warning('Reached max page count.')
-                    break
 
         if not download_urls:
             return DownloadResult(
diff --git a/wrolpi/tags/test_scrape_downloader.py b/wrolpi/tags/test_scrape_downloader.py
index 47d9649c..8f960046 100644
--- a/wrolpi/tags/test_scrape_downloader.py
+++ b/wrolpi/tags/test_scrape_downloader.py
@@ -20,10 +20,18 @@ EXAMPLE_HTTP = '''
 '''
 
 
+async def fake_fetch_http(self, url: str):
+    return EXAMPLE_HTTP
+
+
+async def fake_file_do_download(*a, **kwargs):
+    return DownloadResult(success=True)
+
+
 async def test_scrape_html_downloader(test_directory, test_session, test_download_manager, assert_download_urls):
+    """User can scrape for PDF files."""
     test_download_manager.register_downloader(ScrapeHTMLDownloader())
     test_download_manager.register_downloader(FileDownloader())
-
     destination = test_directory / 'destination'
     destination.mkdir()
 
@@ -36,12 +44,6 @@ async def test_scrape_html_downloader(test_directory, test_session, test_downloa
                                           sub_downloader_name='file')
     assert_download_urls(['https://example.com/dir', ])
 
-    async def fake_fetch_http(self, url: str):
-        return EXAMPLE_HTTP
-
-    async def fake_file_do_download(*a, **kwargs):
-        return DownloadResult(success=True)
-
     with mock.patch('wrolpi.scrape_downloader.ScrapeHTMLDownloader.fetch_http', fake_fetch_http), \
             mock.patch('wrolpi.files.downloader.FileDownloader.do_download', fake_file_do_download):
         await test_download_manager.wait_for_all_downloads()
@@ -53,3 +55,27 @@ async def test_scrape_html_downloader(test_directory, test_session, test_downloa
         'https://example.com/two.pdf',
         'https://example.com/dir/three.pdf',
     ])
+
+
+async def test_scrape_html_downloader_html(test_directory, test_session, test_download_manager, assert_download_urls):
+    """User can also scrape for HTML files."""
+    test_download_manager.register_downloader(ScrapeHTMLDownloader())
+    test_download_manager.register_downloader(FileDownloader())
+
+    settings = {
+        'suffix': '.html',
+        'destination': str(test_directory),
+    }
+    test_download_manager.create_download('https://example.com/dir', 'scrape_html', settings=settings,
+                                          sub_downloader_name='file')
+    assert_download_urls(['https://example.com/dir', ])
+
+    with mock.patch('wrolpi.scrape_downloader.ScrapeHTMLDownloader.fetch_http', fake_fetch_http), \
+            mock.patch('wrolpi.files.downloader.FileDownloader.do_download', fake_file_do_download):
+        await test_download_manager.wait_for_all_downloads()
+        await asyncio.sleep(1)
+
+    assert_download_urls([
+        'https://example.com/dir',
+        'https://example.com/dir/other.html',
+    ])