From d33a50660d92d43cc83d13c8ea6c420dd99c9728 Mon Sep 17 00:00:00 2001 From: "James R. Barlow" Date: Sat, 10 Dec 2016 15:24:24 -0800 Subject: [PATCH] Replace most sys.exit() with raising exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because ruffus doesn’t handle exceptions well I tended to call sys.exit to make sure we got out of dodge when needed. However, sys.exit is not ideal for the Python API this is moving towards, so this introduces proper exceptions for the various cases that retain suggested error codes. Only __main__.py should call sys.exit now, everyone else has to throw an exception. For now the worker raising a fatal exception is logging messages rather than passing an exception object with the fatal error message, mainly because ruffus doesn’t properly marshall the exception object so we just check “what is the name of the exception class that caused ruffus to thrown an RethrownJobError”? Also fixed along the way was the wrong return code being shown for encrypted PDF checking, and incorrect use of str.find (e.output.find) in boolean logic (str.find returns -1 on failure to find, which is True). --- ocrmypdf/__init__.py | 17 ----------------- ocrmypdf/__main__.py | 25 +++++++++++++++---------- ocrmypdf/exe/qpdf.py | 28 +++++++++++++++++++--------- ocrmypdf/exe/tesseract.py | 23 ++++++++++++++--------- ocrmypdf/exe/unpaper.py | 8 ++++---- ocrmypdf/pipeline.py | 24 ++++++++++-------------- tests/test_main.py | 16 +++++++--------- 7 files changed, 69 insertions(+), 72 deletions(-) diff --git a/ocrmypdf/__init__.py b/ocrmypdf/__init__.py index 3c255f56..8a52d3f3 100644 --- a/ocrmypdf/__init__.py +++ b/ocrmypdf/__init__.py @@ -1,4 +1,3 @@ -from enum import IntEnum import os import pkg_resources @@ -7,21 +6,5 @@ PROGRAM_NAME = 'ocrmypdf' VERSION = pkg_resources.get_distribution('ocrmypdf').version -class ExitCode(IntEnum): - ok = 0 - bad_args = 1 - input_file = 2 - missing_dependency = 3 - invalid_output_pdf = 4 - file_access_error = 5 - already_done_ocr = 6 - child_process_error = 7 - encrypted_pdf = 8 - other_error = 15 - ctrl_c = 130 - - def page_number(input_file): return int(os.path.basename(input_file)[0:6]) - - diff --git a/ocrmypdf/__main__.py b/ocrmypdf/__main__.py index 0360c219..127ca262 100755 --- a/ocrmypdf/__main__.py +++ b/ocrmypdf/__main__.py @@ -3,6 +3,7 @@ from contextlib import suppress from tempfile import mkdtemp +from collections.abc import Sequence import sys import os import re @@ -28,8 +29,10 @@ from .pdfa import file_claims_pdfa from .helpers import is_iterable_notstr, re_symlink from .exe import tesseract from .exe import qpdf -from . import ExitCode, PROGRAM_NAME, VERSION -from collections.abc import Sequence +from . import PROGRAM_NAME, VERSION + +from .exceptions import * +from . import exceptions as ocrmypdf_exceptions warnings.simplefilter('ignore', pypdf.utils.PdfReadWarning) @@ -52,10 +55,6 @@ if tesseract.version() < MINIMUM_TESS_VERSION: sys.exit(ExitCode.missing_dependency) -class MissingDependencyException(Exception): - pass - - # ------------- # Parser @@ -289,11 +288,11 @@ def check_options_preprocessing(options, log): from .exe import unpaper try: if unpaper.version() < '6.1': - raise MissingDependencyException( + raise MissingDependencyError( "The installed 'unpaper' is not supported. " "Install version 6.1 or newer.") except FileNotFoundError: - raise MissingDependencyException( + raise MissingDependencyError( "Install the 'unpaper' program to use --clean, --clean-final.") if options.clean and \ @@ -326,7 +325,7 @@ def check_options(options, log): except argparse.ArgumentError as e: log.error(e) sys.exit(ExitCode.bad_args) - except MissingDependencyException as e: + except MissingDependencyError as e: log.error(e) sys.exit(ExitCode.missing_dependency) @@ -408,7 +407,7 @@ def do_ruffus_exception(ruffus_five_tuple, options, log): msg = "Error occurred while running this command:" log.error(msg + '\n' + exc_value) return ExitCode.child_process_error - elif exc_name == 'ocrmypdf.main.PdfMergeFailedError': + elif exc_name == 'ocrmypdf.exceptions.PdfMergeFailedError': log.error(textwrap.dedent("""\ Failed to merge PDF image layer with OCR layer @@ -419,6 +418,10 @@ def do_ruffus_exception(ruffus_five_tuple, options, log): ocrmypdf --pdf-renderer tesseract [..other args..] """)) return ExitCode.input_file + elif exc_name.startswith('ocrmypdf.exceptions.'): + base_exc_name = exc_name.replace('ocrmypdf.exceptions.', '') + exc_class = getattr(ocrmypdf_exceptions, base_exc_name) + return exc_class.exit_code elif exc_name == 'PyPDF2.utils.PdfReadError' and \ 'not been decrypted' in exc_value: log.error(textwrap.dedent("""\ @@ -532,6 +535,8 @@ def run_pipeline(): return ExitCode.other_error else: return exitcode + except ExitCodeException as e: + return e.exit_code except Exception as e: _log.error(e) return ExitCode.other_error diff --git a/ocrmypdf/exe/qpdf.py b/ocrmypdf/exe/qpdf.py index f7219095..341fef4f 100644 --- a/ocrmypdf/exe/qpdf.py +++ b/ocrmypdf/exe/qpdf.py @@ -7,7 +7,8 @@ import sys import os import re -from .. import ExitCode +from ..exceptions import InputFileError, SubprocessOutputError, \ + MissingDependencyError, EncryptedPdfError from . import get_program @@ -21,9 +22,10 @@ def version(): versions = check_output( args_qpdf, close_fds=True, universal_newlines=True, stderr=STDOUT) - except CalledProcessError: - print("Could not find qpdf executable on system PATH.") - sys.exit(ExitCode.missing_dependency) + except CalledProcessError as e: + print("Could not find qpdf executable on system PATH.", + file=sys.stderr) + raise MissingDependencyError() from e qpdf_version = re.match(r'qpdf version (.+)', versions).group(1) return qpdf_version @@ -53,6 +55,14 @@ def check(input_file, log): return True +def _probably_encrypted(e): + """qpdf can report a false positive "file is encrypted" message for damaged + files - suppress this""" + return e.returncode == 2 and \ + 'invalid password' in e.output and \ + 'file is damaged' not in e.output + + def repair(input_file, output_file, log): args_qpdf = [ get_program('qpdf'), input_file, output_file @@ -65,20 +75,20 @@ def repair(input_file, output_file, log): log.debug(e.output) return - if e.returncode == 2 and e.output.find("invalid password"): + if _probably_encrypted(e): log.error("{0}: this PDF is password-protected - password must " "be removed for OCR".format(input_file)) - sys.exit(ExitCode.input_file) + raise EncryptedPdfError() from e elif e.returncode == 2: log.error("{0}: not a valid PDF, and could not repair it.".format( input_file)) log.error("Details: " + e.output) - sys.exit(ExitCode.input_file) + raise InputFileError() from e else: log.error("{0}: unknown error".format( input_file)) log.error(e.output) - sys.exit(ExitCode.unknown) + raise SubprocessOutputError() from e def get_npages(input_file, log): @@ -89,7 +99,7 @@ def get_npages(input_file, log): except CalledProcessError as e: if e.returncode == 2 and e.output.find('No such file'): log.error(e.output) - sys.exit(ExitCode.input_file) + raise InputFileError() from e return int(pages) diff --git a/ocrmypdf/exe/tesseract.py b/ocrmypdf/exe/tesseract.py index 1e9387d2..40dd4d54 100644 --- a/ocrmypdf/exe/tesseract.py +++ b/ocrmypdf/exe/tesseract.py @@ -6,9 +6,11 @@ import os import re import shutil from functools import lru_cache -from .. import ExitCode, page_number +from ..exceptions import MissingDependencyError +from .. import page_number from . import get_program from collections import namedtuple +from textwrap import dedent from subprocess import Popen, PIPE, CalledProcessError, \ TimeoutExpired, check_output, STDOUT, DEVNULL @@ -51,9 +53,10 @@ def version(): versions = check_output( args_tess, close_fds=True, universal_newlines=True, stderr=STDOUT) - except CalledProcessError: - print("Could not find Tesseract executable on system PATH.") - sys.exit(ExitCode.missing_dependency) + except CalledProcessError as e: + print("Could not find Tesseract executable on system PATH.", + file=sys.stderr) + raise MissingDependencyError from e tesseract_version = re.match(r'tesseract\s(.+)', versions).group(1) return tesseract_version @@ -70,11 +73,13 @@ def languages(): args_tess, close_fds=True, universal_newlines=True, stderr=STDOUT) except CalledProcessError as e: - print("Tesseract failed to report available languages.") - print("Output from Tesseract:") - print("-" * 40) - print(e.output) - sys.exit(ExitCode.missing_dependency) + msg = dedent("""Tesseract failed to report available languages. + Output from Tesseract: + ----------- + """) + msg += e.output + print(msg, file=sys.stderr) + raise MissingDependencyError from e return set(lang.strip() for lang in langs.splitlines()[1:]) diff --git a/ocrmypdf/exe/unpaper.py b/ocrmypdf/exe/unpaper.py index 4c589251..d7db30a9 100644 --- a/ocrmypdf/exe/unpaper.py +++ b/ocrmypdf/exe/unpaper.py @@ -8,7 +8,7 @@ from tempfile import NamedTemporaryFile import sys import os from functools import lru_cache -from .. import ExitCode +from ..exceptions import MissingDependencyError from . import get_program @@ -48,17 +48,17 @@ def run(input_file, output_file, dpi, log, mode_args): im = im.convert(mode='1') else: im = im.convert(mode='RGB') - except IOError: + except IOError as e: log.error( "Could not convert image with type " + im.mode) - sys.exit(ExitCode.missing_dependency) + raise MissingDependencyError() from e try: suffix = SUFFIXES[im.mode] except KeyError: log.error( "Failed to convert image to a supported format.") - sys.exit(ExitCode.missing_dependency) + raise MissingDependencyError() from e with NamedTemporaryFile(suffix=suffix) as input_pnm, \ NamedTemporaryFile(suffix=suffix, mode="r+b") as output_pnm: diff --git a/ocrmypdf/pipeline.py b/ocrmypdf/pipeline.py index 2c4f365b..a77df60a 100644 --- a/ocrmypdf/pipeline.py +++ b/ocrmypdf/pipeline.py @@ -28,8 +28,9 @@ from .helpers import re_symlink, is_iterable_notstr from .exe import ghostscript from .exe import tesseract from .exe import qpdf +from .exceptions import * from . import leptonica -from . import ExitCode, page_number, PROGRAM_NAME, VERSION +from . import page_number, PROGRAM_NAME, VERSION VECTOR_PAGE_DPI = 400 @@ -107,8 +108,7 @@ def triage_image_file(input_file, output_file, log, options): realpath = '' msg = msg.replace(input_file, realpath) log.error(msg) - sys.exit(ExitCode.input_file) - return + raise UnsupportedImageFormatError() from e else: log.info("Input file is an image") @@ -120,21 +120,21 @@ def triage_image_file(input_file, output_file, log, options): "Input file is an image, but the resolution (DPI) is " "not credible. Estimate the resolution at which the " "image was scanned and specify it using --image-dpi.") - sys.exit(ExitCode.input_file) + raise DpiError() elif not options.image_dpi: log.info("Image size: (%d, %d)" % im.size) log.error( "Input file is an image, but has no resolution (DPI) " "in its metadata. Estimate the resolution at which " "image was scanned and specify it using --image-dpi.") - sys.exit(ExitCode.input_file) + raise DpiError() if 'iccprofile' not in im.info: if im.mode == 'RGB': log.info('Input image has no ICC profile, assuming sRGB') elif im.mode == 'CMYK': log.info('Input CMYK image has no ICC profile, not usable') - sys.exit(ExitCode.input_file) + raise UnsupportedImageFormatError() im.close() try: @@ -152,7 +152,7 @@ def triage_image_file(input_file, output_file, log, options): log.info("Successfully converted to PDF, processing...") except img2pdf.ImageOpenError as e: log.error(e) - sys.exit(ExitCode.input_file) + raise UnsupportedImageFormatError() from e def triage( @@ -169,7 +169,7 @@ def triage( return except EnvironmentError as e: log.error(e) - sys.exit(ExitCode.input_file) + raise InputFileError() from e options = context.get_options() triage_image_file(input_file, output_file, log, options) @@ -239,7 +239,7 @@ def is_ocr_required(pageinfo, log, options): if not options.force_ocr and not options.skip_text: log.error(msg.format(page, "aborting (use --force-ocr to force OCR)")) - sys.exit(ExitCode.already_done_ocr) + raise PriorOcrFoundError() elif options.force_ocr: log.info(msg.format(page, "rasterizing text and running OCR anyway")) @@ -282,7 +282,7 @@ def split_pages( if not input_file: log.error("{0}: file not found or invalid argument".format( options.input_file)) - sys.exit(ExitCode.input_file) + raise InputFileError() npages = qpdf.get_npages(input_file, log) qpdf.split_pages(input_file, work_folder, npages) @@ -582,10 +582,6 @@ def render_hocr_debug_page( showBoundingboxes=True, invisibleText=False) -class PdfMergeFailedError(Exception): - pass - - def add_text_layer( infiles, output_file, diff --git a/tests/test_main.py b/tests/test_main.py index fd1bfc5a..00c8f3a8 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -10,7 +10,7 @@ import sys import pytest from ocrmypdf.pageinfo import pdf_get_all_pageinfo import PyPDF2 as pypdf -from ocrmypdf import ExitCode +from ocrmypdf.exceptions import ExitCode from ocrmypdf import leptonica from ocrmypdf.pdfa import file_claims_pdfa import platform @@ -61,11 +61,6 @@ def check_ocrmypdf(input_basename, output_basename, *args, env=None): p, out, err = run_ocrmypdf(input_basename, output_basename, *args, env=env) print(err) # ensure py.test collects the output, use -s to view - if p.returncode != 0: - print('stdout\n======') - print(out) - print('stderr\n======') - print(err) assert p.returncode == 0 assert os.path.exists(output_file), "Output file not created" assert os.stat(output_file).st_size > 100, "PDF too small or empty" @@ -88,6 +83,8 @@ def run_ocrmypdf(input_basename, output_basename, *args, env=None): p_args, close_fds=True, stdout=PIPE, stderr=PIPE, universal_newlines=True, env=env) out, err = p.communicate() + print(err) + return p, out, err @@ -548,15 +545,16 @@ def test_qpdf_repair_fails(): env['OCRMYPDF_QPDF'] = os.path.abspath('./spoof/qpdf_dummy_return2.py') p, out, err = run_ocrmypdf( '-v', '1', - 'c02-22.pdf', 'wont_be_created.pdf', env=env) + 'c02-22.pdf', 'wont_be_created_repair_fail.pdf', env=env) print(out) print(err) assert p.returncode == ExitCode.input_file def test_encrypted(): - p, out, err = run_ocrmypdf('skew-encrypted.pdf', 'wont_be_created.pdf') - assert p.returncode == ExitCode.input_file + p, out, err = run_ocrmypdf( + 'skew-encrypted.pdf', 'wont_be_created_test_enc.pdf') + assert p.returncode == ExitCode.encrypted_pdf assert out.find('password')