Replace most sys.exit() with raising exceptions

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).
This commit is contained in:
James R. Barlow
2016-12-10 15:24:24 -08:00
parent 4ee9658e97
commit d33a50660d
7 changed files with 69 additions and 72 deletions
-17
View File
@@ -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])
+15 -10
View File
@@ -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
+19 -9
View File
@@ -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)
+14 -9
View File
@@ -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:])
+4 -4
View File
@@ -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:
+10 -14
View File
@@ -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 = '<stdin>'
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,
+7 -9
View File
@@ -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')