Unhandled exceptions occurring inside function passed to `executor.map` are swallowed about dangerzone HOT 7 OPEN

Got it. So we practically need a try: ... except Exception: log.exception at the top level, that would log any stray exceptions from the exception blocks you pointed out.

from dangerzone.

Indeed. IIUC this wrapping handler should also mark the document as failed upon exception (if it was not done already) to correctly report it at the end.

from dangerzone.

Cool! Hopefully this issue provides enough context. In a nutshell, we want to:

1. Catch and report all stray conversion exceptions from IsolationProvider.convert.
2. Mark the related document as failed.

Happy to answer any question you may have 🙂

from dangerzone.

Setting up the environment might take a while because I'm busy right now

from dangerzone.

Thanks a lot for bringing this issue to our attention, and for writing a test case.

I agree that the generator result should be consumed in principle, but we catch and log all exceptions (as you've already pointed out) here. I have to dig deeper, but my bet would be that there's a different underlying cause :-/.

from dangerzone.

but we catch and log all exceptions (as you've already pointed out) here

FWIW, I've ran into this in CLI tests when dummy conversion is used. The Dummy isolation provider overrides the convert method and does not have any exception handling.

I do not have a test case when using e.g. the container isolation provider (which IIUC uses the convert method defined on the base isolation provider), but the two sections (here and here (come to think of it, it also applies to the except errors.ConverterProcException and except errors.ConversionException blocks as well)) I've mentioned could hide exceptions if an exception were to occur inside them, since those sections are not inside a try/except (this part is inside the except Exception block, but any potential exception coming from inside the except block is not handled (there is no exception handling in convert_doc) and would be swallowed by exception.map).

To simulate a swallowed exception when using the container provider, we would have to raise it explicitly. E.g. try adding 1 / 0 here (before the try block). Running dev_scripts/dangerzone-cli /tmp/sample.pdf then results in:

╭──────────────────────────╮
│           ▄██▄           │
│          ██████          │
│         ███▀▀▀██         │
│        ███   ████        │
│       ███   ██████       │
│      ███   ▀▀▀▀████      │
│     ███████  ▄██████     │
│    ███████ ▄█████████    │
│   ████████████████████   │
│    ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀    │
│                          │
│    Dangerzone v0.6.0     │
│ https://dangerzone.rocks │
╰──────────────────────────╯
[INFO ] Assigning ID '0sHZEe' to doc '/tmp/sample.pdf'

Converting document to safe PDF
[DEBUG] Marking doc 0sHZEe as 'converting'

from dangerzone.

I'm interested in contributing to this

from dangerzone.