From cb024ab57d05a5853ee23716319461a1ba16f3ff Mon Sep 17 00:00:00 2001 From: Roland Date: Tue, 8 Oct 2024 17:19:41 -0600 Subject: [PATCH] Changing archive download process to output html file, rather than capturing stdout. --- modules/archive/__init__.py | 60 +++++++++++++++++++--------------- wrolpi/common.py | 3 +- wrolpi/downloader.py | 10 +++--- wrolpi/test/test_downloader.py | 46 ++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 31 deletions(-) diff --git a/modules/archive/__init__.py b/modules/archive/__init__.py index fb0ed6db..1e5554ff 100644 --- a/modules/archive/__init__.py +++ b/modules/archive/__init__.py @@ -9,7 +9,7 @@ from selenium import webdriver from sqlalchemy import not_ from sqlalchemy.orm import Session -from wrolpi.cmd import SINGLE_FILE_BIN, CHROMIUM, READABILITY_BIN +from wrolpi.cmd import READABILITY_BIN, SINGLE_FILE_BIN, CHROMIUM from wrolpi.common import logger, register_modeler, register_refresh_cleanup, limit_concurrent, split_lines_by_length, \ slow_logger, html_screenshot, get_title_from_html from wrolpi.db import optional_session, get_db_session @@ -71,33 +71,41 @@ class ArchiveDownloader(Downloader, ABC): async def do_singlefile(self, download: Download) -> Tuple[bytes, dict]: """Create a Singlefile from the archive's URL.""" - cmd = ('/usr/bin/nice', '-n15', # Nice above map importing, but very low priority. - str(SINGLE_FILE_BIN), - download.url, - '--browser-executable-path', CHROMIUM, - '--browser-args', '["--no-sandbox"]', - '--dump-content', - '--user-agent', USER_AGENT, - ) - return_code, logs, stdout = await self.process_runner( - download.id, - download.url, - cmd, - pathlib.Path('/home/wrolpi'), - ) - if return_code != 0: - raise RuntimeError(f'Archive singlefile exited with {return_code}') + with tempfile.NamedTemporaryFile(suffix='.html', delete=False) as output: + output = pathlib.Path(output.name) + cmd = ('/usr/bin/nice', '-n15', # Nice above map importing, but very low priority. + str(SINGLE_FILE_BIN), + '--browser-executable-path', CHROMIUM, + '--browser-args', '["--no-sandbox"]', + '--user-agent', USER_AGENT, + download.url, + output, + ) + return_code, logs, stdout = await self.process_runner( + download.id, + download.url, + cmd, + pathlib.Path('/home/wrolpi'), + ) - if not stdout or SINGLEFILE_HEADER.encode() not in stdout[:1000]: - if logs and (stderr := logs.get('stderr')): - e = ChildProcessError(stderr.decode()[:1000]) - raise RuntimeError(f'Singlefile created was invalid: {download.url}') from e - if stdout: - e = ChildProcessError(stdout.decode()[:1000]) - raise RuntimeError(f'Singlefile created was invalid: {download.url}') from e - raise RuntimeError(f'Singlefile created was invalid: {download.url}') + stdout = stdout.decode() + stderr = logs['stderr'].decode() + log_output = stderr or stdout or 'No stderr or stdout!' - return stdout, logs + if return_code != 0: + e = ChildProcessError(log_output[:1000]) + raise RuntimeError(f'Archive singlefile exited with {return_code}') from e + + content = output.read_bytes() if output.is_file() else '' + if not content: + e = ChildProcessError(log_output[:1000]) + raise RuntimeError(f'Singlefile created was empty: {download.url}') from e + + if SINGLEFILE_HEADER.encode() not in content: + e = ChildProcessError(log_output[:1000]) + raise RuntimeError(f'Singlefile created was invalid: {download.url}') from e + + return content, logs async def do_readability(self, download: Download, html: bytes) -> dict: """Extract the readability dict from the provided HTML.""" diff --git a/wrolpi/common.py b/wrolpi/common.py index 7926cc0e..20b71120 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -440,6 +440,7 @@ class ConfigFile: @param overwrite: Will overwrite the config file even if it was not imported successfully. """ from wrolpi.api_utils import api_app + from wrolpi.events import Events file = file or self.get_file() if self.file_name not in file.name: @@ -449,6 +450,7 @@ class ConfigFile: if file.exists() and overwrite is False: if not self.successful_import: + Events.send_config_save_failed(f'Failed to save config: {rel_path}') raise RuntimeError(f'Refusing to save config because it was never successfully imported! {rel_path}') version = self.read_config_file(file).get('version') if version and version > self.version: @@ -489,7 +491,6 @@ class ConfigFile: message = f'Failed to save config: {rel_path}' logger_.critical(message, exc_info=e) if send_events: - from wrolpi.events import Events Events.send_config_save_failed(message) raise e diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 02234fe7..7d50a9fc 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -296,7 +296,7 @@ class Downloader: Global timeout takes precedence over the timeout argument, unless it is 0. (Smaller global timeout wins) """ str_cmd = " ".join([str(i) for i in cmd]) - logger.debug(f'{self} launching download process with args: {str_cmd}') + logger.info(f'{self} launching download process with args: {str_cmd}') start = now() proc = await asyncio.create_subprocess_exec(*cmd, stdout=asyncio.subprocess.PIPE, @@ -310,7 +310,7 @@ class Downloader: timeout = WROLPI_CONFIG.download_timeout or timeout or self.timeout logger.debug(f'{self} launched download process {pid=} {timeout=} for {url}') - stdout, stderr = None, None + stdout, stderr = b'', b'' try: while True: task = asyncio.Task(proc.communicate()) @@ -346,8 +346,10 @@ class Downloader: finally: # Output all logs from the process. # TODO is there a way to stream this output while the process is running? - logger.debug(f'Download exited with {proc.returncode}') - logs = {'stdout': stdout or '', 'stderr': stderr or ''} + stdout_len = len(stdout) + stderr_len = len(stderr) + logger.debug(f'Download exited with {proc.returncode} and stdout={stdout_len} stderr={stderr_len}') + logs = {'stdout': stdout, 'stderr': stderr} return proc.returncode, logs, stdout diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index 34527318..38c0de95 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -1,5 +1,7 @@ import asyncio import json +import pathlib +import tempfile from abc import ABC from datetime import datetime from http import HTTPStatus @@ -297,6 +299,50 @@ async def test_process_runner_timeout(async_client, test_session, test_directory assert 2 < elapsed.total_seconds() < 4 +GOOD_SCRIPT = '''#! /usr/bin/env bash +echo "standard output" +echo "standard error" >&2 +''' + +BAD_SCRIPT = '''#! /usr/bin/env bash +echo "new standard output" +echo "new standard error" >&2 +exit 123 +''' + + +@pytest.mark.asyncio +async def test_process_runner_output(async_client, test_directory, test_downloader): + with tempfile.TemporaryDirectory() as directory: + directory = pathlib.Path(directory) + script = directory / 'echo_script.sh' + script.write_text(GOOD_SCRIPT) + cmd = ('/bin/bash', script) + return_code, logs, stdout = await test_downloader.process_runner( + 1, + 'https://example.com', + cmd, + directory, + timeout=1 + ) + assert return_code == 0 + assert logs['stdout'] == b'standard output\n' + assert logs['stderr'] == b'standard error\n' + + script.write_text(BAD_SCRIPT) + cmd = ('/bin/bash', script) + return_code, logs, stdout = await test_downloader.process_runner( + 1, + 'https://example.com', + cmd, + directory, + timeout=1 + ) + assert return_code == 123 + assert logs['stdout'] == b'new standard output\n' + assert logs['stderr'] == b'new standard error\n' + + @pytest.mark.asyncio async def test_crud_download(async_client, test_session, test_download_manager, test_downloader): """Test the ways that Downloads can be created."""