Skip to content

[18.0][IMP] report_py3o: improve error observability in LO conversion chain#1166

Open
rrebollo wants to merge 2 commits into
OCA:18.0from
BinhexTeam:18.0-imp-report_py3o-lo-error-handling
Open

[18.0][IMP] report_py3o: improve error observability in LO conversion chain#1166
rrebollo wants to merge 2 commits into
OCA:18.0from
BinhexTeam:18.0-imp-report_py3o-lo-error-handling

Conversation

@rrebollo
Copy link
Copy Markdown
Contributor

@rrebollo rrebollo commented Jun 2, 2026

Problem

When LibreOffice fails to convert a rendered ODT to the target format, FileNotFoundError propagates unhandled, the controller catches it without logging, and debugging is impossible.

Actual production traceback (anonymized):

  File ".../reporting-engine/report_py3o/controllers/main.py", line 82, in report_download
    response = self.report_routes(
  File ".../odoo/http.py", line 754, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File ".../reporting-engine/report_xml/controllers/report.py", line 24, in report_routes
    return super().report_routes(
  File ".../odoo/http.py", line 754, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File ".../reporting-engine/report_xlsx_helper/controllers/main.py", line 51, in report_routes
    return super().report_routes(reportname, docids, converter, **data)
  File ".../odoo/http.py", line 754, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File ".../reporting-engine/report_xlsx/controllers/main.py", line 51, in report_routes
    return super().report_routes(reportname, docids, converter, **data)
  File ".../odoo/http.py", line 754, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File ".../reporting-engine/report_py3o/controllers/main.py", line 47, in report_routes
    res, filetype = ir_action._render(reportname, docids, data)
  File ".../odoo/addons/base/models/ir_actions_report.py", line 1135, in _render
    return render_func(report_ref, res_ids, data=data)
  File ".../reporting-engine/report_py3o/models/ir_actions_report.py", line 171, in _render_py3o
    .create_report(res_ids, data)
  File ".../reporting-engine/report_py3o/models/py3o_report.py", line 394, in create_report
    with open(result_path, "r+b") as fd:

FileNotFoundError: [Errno 2] No such file or directory:
    '/tmp/p3o.report.tmp.xxxxxxxx.pdf'

The FileNotFoundError is not the root cause — it is the last symptom of a silent failure chain:

  1. LibreOffice runs --headless --convert-toexit code 0 → but silently produces no output file (e.g. because the rendered ODT contains HTML content LO cannot handle)
  2. _convert_single_report has zero error handlingsubprocess.check_output succeeds, code computes the expected output path, never verifies the file exists
  3. create_report tries open(result_path, "r+b")FileNotFoundError propagates
  4. Controller's except Exception does not log — the entire traceback is silently swallowed, returned as JSON to the browser, and lost on page refresh

Fix

Two minimal changes following OCA patterns already used by sibling addons (report_csv, report_xlsx, report_xml):

py3o_report.py — Model-level error handling:

  • Capture LO stderr: subprocess.check_output(..., stderr=subprocess.STDOUT)
  • try/except FileNotFoundErrorUserError
  • try/except CalledProcessErrorlogger.error(details) + UserError
  • Post-conversion: os.path.exists(result_path)logger.error(stderr) + UserError if missing
  • _convert_single_report_cmd: RuntimeErrorUserError

controllers/main.py — Controller-level logging (one line):

  • logger.exception("Error while generating py3o report: %s", reportname) before serialize_exception
  • Matches report_csv, report_xlsx, report_xml controllers exactly

Testing

Two new tests added:

Test Pattern Verifies
test_convert_single_report_called_process_error TransactionCase + mock.patch("subprocess.check_output") _convert_single_report raises UserError on LO failure
TestReportPy3oController.test_report_download_error_logging HttpCase + mock.patch.object(ReportController, "report_routes") + assertLogs Controller logs before serializing error JSON

@BinhexTeam T18747

rrebollo added 2 commits June 2, 2026 14:59
_convert_single_report: add try/except around subprocess.check_output,
capture LO stdout+stderr, verify output file exists after conversion.
Raise UserError instead of RuntimeError for user-facing failures.

Controller: add logger.exception() before serializing error to JSON,
so the real traceback appears in Odoo logs instead of being silently
swallowed by except Exception.

Follows OCA patterns from report_csv, report_xlsx, report_xml
controllers for consistent error logging across the reporting-engine
ecosystem.
test_convert_single_report_called_process_error: verify _convert_single_report
raises UserError when subprocess.check_output raises CalledProcessError.

TestReportPy3oController.test_report_download_error_logging: verify controller
logs the exception before serializing the error, using HttpCase + assertLogs
pattern from report_csv.

Fix existing test_py3o_report_availability: RuntimeError -> UserError
to match the change in _convert_single_report_cmd.

Fix indentation in test_report_template_configs: _render call was incorrectly
de-indented outside the with temporary_copy block, causing TemplateNotFound.
@rrebollo rrebollo marked this pull request as draft June 2, 2026 19:30
@rrebollo rrebollo marked this pull request as ready for review June 2, 2026 19:36
Copy link
Copy Markdown

@fsmw fsmw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rrebollo for the reciprocal review opportunity — happy to return the favor on #1165.

Solid PR. Confirmed the controller pattern matches report_csv (line 107) and report_xlsx (line 101) exactly: logger.exception before serialize_exception. Good catch on the missing os.path.exists check — that's the real root cause of the silent LO exit-0-with-no-output chain (we hit a similar issue once with HTML content in templates).

The RuntimeErrorUserError change is also correct and the existing test_py3o_report_availability already covers that path at line 264.

CI all green (pre-commit, Odoo, OCB, codecov, runboat), tests are appropriately focused (TransactionCase for the model path, HttpCase + assertLogs for the controller), and the localized error messages + stderr context in logger.error are exactly what admins need for postmortem.

Approved ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants