Importing a CV: an importer plugin that raises in can_handle breaks the page for every file, Europass included #611

New issue

Closed

opened 2026-10-01 10:48:22 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:48:22 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the import view against the registry's handling of capture sources and trying it against the test settings.

Related to #105.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at plugins/registry.py:344-369 (parse_page, the comparison); resume/views.py:441-461 and plugins/registry.py:274-284 have not moved.

The import page asks each installed importer can_handle() in turn. Third-party importers are asked first and the built-in Europass importer last. Only ImportRefused is caught. Capture sources get a try/except that logs the failing plugin and moves on (registry.parse_page); importers do not, so the project's rule that a plugin that raises is logged and skipped does not hold on this page.

Where: resume/views.py:441-461 (EuropassImportView.post: the generator calling plugin.can_handle(data, upload.name or ""), then importer.read(data), inside try: … except base.ImportRefused); plugins/registry.py:274-284 (plugins(): third-party first); compare plugins/registry.py:362-386 (parse_page).
What happens: an operator installs a third-party importer whose can_handle raises on bytes it did not expect, for example a PDF. Tried on the test settings with a double that raises ValueError: posting a Europass file to resume:europass_import answered 500. From then on every upload on the page fails, because the built-in importer is only asked after the faulty one. An unexpected exception from read() also ends in a 500, where the page could name the importer that failed.
Why it matters: one faulty add-on stops the built-in Europass import for every account, and it is not logged as a plugin failure.
Fix: wrap each importer's can_handle in try/except Exception with logger.exception(...) and continue to the next importer, as parse_page does. Wrap read() the same way, and show the person a sentence naming the importer that failed instead of a server error. Update the importer section of the wiki's Writing a plugin ("Any other exception is a bug in the importer and is reported as one") to say what the person sees.

Done when

A test in tests/test_importers.py registers an importer whose can_handle raises ahead of Europass and shows a Europass file still being read, with the failure logged; a second test, with read() raising, shows a message on the page instead of a 500.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the import view against the registry's handling of capture sources and trying it against the test settings. Related to #105. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `plugins/registry.py:344-369` (`parse_page`, the comparison); `resume/views.py:441-461` and `plugins/registry.py:274-284` have not moved. The import page asks each installed importer `can_handle()` in turn. Third-party importers are asked first and the built-in Europass importer last. Only `ImportRefused` is caught. Capture sources get a `try`/`except` that logs the failing plugin and moves on (`registry.parse_page`); importers do not, so the project's rule that a plugin that raises is logged and skipped does not hold on this page. - **Where:** `resume/views.py:441-461` (`EuropassImportView.post`: the generator calling `plugin.can_handle(data, upload.name or "")`, then `importer.read(data)`, inside `try: … except base.ImportRefused`); `plugins/registry.py:274-284` (`plugins()`: third-party first); compare `plugins/registry.py:362-386` (`parse_page`). - **What happens:** an operator installs a third-party importer whose `can_handle` raises on bytes it did not expect, for example a PDF. Tried on the test settings with a double that raises `ValueError`: posting a Europass file to `resume:europass_import` answered 500. From then on every upload on the page fails, because the built-in importer is only asked after the faulty one. An unexpected exception from `read()` also ends in a 500, where the page could name the importer that failed. - **Why it matters:** one faulty add-on stops the built-in Europass import for every account, and it is not logged as a plugin failure. - **Fix:** wrap each importer's `can_handle` in `try`/`except Exception` with `logger.exception(...)` and continue to the next importer, as `parse_page` does. Wrap `read()` the same way, and show the person a sentence naming the importer that failed instead of a server error. Update the importer section of the wiki's *Writing a plugin* ("Any other exception is a bug in the importer and is reported as one") to say what the person sees. ## Done when - A test in `tests/test_importers.py` registers an importer whose `can_handle` raises ahead of Europass and shows a Europass file still being read, with the failure logged; a second test, with `read()` raising, shows a message on the page instead of a 500.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:48:22 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:48:22 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 05:28:44 +00:00

Importers that raise no longer break the CV import page

tiagoagueda closed this issue

2026-10-04 05:28:44 +00:00