[18.0][IMP] report_py3o: improve error observability in LO conversion chain by rrebollo · Pull Request #1166 · OCA/reporting-engine

rrebollo · 2026-06-02T19:30:13Z

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:

LibreOffice runs --headless --convert-to → exit code 0 → but silently produces no output file (e.g. because the rendered ODT contains HTML content LO cannot handle)
_convert_single_report has zero error handling — subprocess.check_output succeeds, code computes the expected output path, never verifies the file exists
create_report tries open(result_path, "r+b") → FileNotFoundError propagates
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 FileNotFoundError → UserError
try/except CalledProcessError → logger.error(details) + UserError
Post-conversion: os.path.exists(result_path) → logger.error(stderr) + UserError if missing
_convert_single_report_cmd: RuntimeError → UserError

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

fsmw

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 RuntimeError → UserError 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 ✅

github-actions · 2026-10-04T16:31:52Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

rrebollo · 2026-10-07T14:43:14Z

Review comments addressed — thanks @vvrossem for the detailed pass. Resolved the threads where the suggestion was applied as-is; left replies on the three where we took a small liberty (self.env._ for the new translated string, both logs kept, and from None kept with a preceding logger.error).

Two extra notes: the module-level import json was moved to the stdlib block (isort), and TestReportPy3oController now warms the routing map before patching report_routes — Odoo builds the map lazily on the first request and introspects the (mocked) route's __name__, which made the request 500 before reaching the handler. All CI checks are green.

vvrossem · 2026-10-09T08:51:05Z

The message of the second commit mentions an indentation fix in test_report_template_configs that's not in the diff anymore, can you drop that line?

rrebollo · 2026-10-09T19:07:11Z

@vvrossem both points addressed: the FileNotFoundError message now says the conversion command could not be executed (branch kept — a broken wrapper shebang reaches it — and from None → from exc so the cause is preserved); the string in _convert_single_report_cmd uses self.env._ too; and the rest of py3o_report.py / ir_actions_report.py is a separate [REF] commit that also drops the now-unused imports. CI is green again — could you take another look when you get a chance? (I cannot formally request a review from the fork.)

LibreOffice can exit with a zero status while producing no output file (e.g. when the rendered ODT contains content it cannot handle). The previous code blindly opened the expected output path, so the failure surfaced as an unhandled FileNotFoundError far from its root cause. Convert subprocess.check_output failures into a UserError with an actionable message, capture LO stdout+stderr for the logs, and verify the output file exists before using it. Log the error before raising when the conversion command itself cannot be executed. Controller: add logger.exception() before serializing the error to JSON, so the real traceback appears in the Odoo logs instead of being silently swallowed by the except Exception clause. Log the request URL rather than reportname, which may be unbound if the URL cannot be parsed. Tests: cover the zero-exit-no-output branch, the failing conversion command and the controller logging path; update the availability test to expect UserError. Also switch the strings touched here to the Odoo 18 self.env._() API and drop the now-unused module-level `_` imports. Backward compatibility note: _convert_single_report_cmd now raises UserError instead of RuntimeError when the LibreOffice runtime is not available, which is a behavior change on a stable branch. Overrides catching RuntimeError must be updated. Follows OCA patterns from report_csv, report_xlsx, report_xml controllers for consistent error logging across the reporting-engine ecosystem.

rrebollo · 2026-10-16T19:31:49Z

@vvrossem, your suggestions have been addressed. Would you be so kind as to review this one again?

OCA-git-bot · 2026-10-16T21:00:02Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

rrebollo · 2026-10-17T12:07:48Z

@legalsylvain could you merge this one?

jelenapoblet · 2026-10-23T13:56:43Z

@legalsylvain if you don't mind, I'm going to merge this.

rrebollo · 2026-10-26T14:42:56Z

@jelenapoblet would you finally merge this one please?

jelenapoblet

This PR fixes error handling and logging, it doesn’t add a new feature and it has neccesary reviews. I'll proceed to merge

jelenapoblet · 2026-10-26T14:50:37Z

/ocabot merge patch

OCA-git-bot · 2026-10-26T14:50:52Z

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-1166-by-jelenapoblet-bump-patch, awaiting test results.

OCA-git-bot · 2026-10-26T15:05:22Z

Congratulations, your PR was merged at b5f21a8. Thanks a lot for contributing to OCA. ❤️

OCA-git-bot added series:18.0 mod:report_py3o Module report_py3o labels Jun 2, 2026

rrebollo marked this pull request as draft June 2, 2026 19:30

rrebollo marked this pull request as ready for review June 2, 2026 19:36

rrebollo mentioned this pull request Jun 2, 2026

[18.0][FIX] report_py3o: use py3o report in invoice Send flow #1165

Open

fsmw approved these changes Jun 3, 2026

View reviewed changes

github-actions Bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 4, 2026

vvrossem reviewed Oct 6, 2026

View reviewed changes

rrebollo force-pushed the 18.0-imp-report_py3o-lo-error-handling branch 3 times, most recently from 0a0142a to 94c3cab Compare October 7, 2026 14:36

vvrossem reviewed Oct 9, 2026

View reviewed changes

Comment thread report_py3o/models/py3o_report.py Outdated

rrebollo force-pushed the 18.0-imp-report_py3o-lo-error-handling branch from 94c3cab to 10c01f4 Compare October 9, 2026 19:00

rrebollo requested a review from vvrossem October 9, 2026 19:10

github-actions Bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 11, 2026

rrebollo force-pushed the 18.0-imp-report_py3o-lo-error-handling branch from 10c01f4 to 3cf9958 Compare October 16, 2026 19:20

vvrossem approved these changes Oct 16, 2026

View reviewed changes

OCA-git-bot added the approved label Oct 16, 2026

OCA-git-bot added the ready to merge label Oct 16, 2026

jelenapoblet approved these changes Oct 26, 2026

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Oct 26, 2026

OCA-git-bot merged commit 202072c into OCA:18.0 Oct 26, 2026
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Oct 26, 2026

Conversation

rrebollo commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Testing

Uh oh!

fsmw left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rrebollo commented Oct 7, 2026

Uh oh!

vvrossem commented Oct 9, 2026

Uh oh!

Uh oh!

rrebollo commented Oct 9, 2026

Uh oh!

rrebollo commented Oct 16, 2026

Uh oh!

OCA-git-bot commented Oct 16, 2026

Uh oh!

rrebollo commented Oct 17, 2026

Uh oh!

jelenapoblet commented Oct 23, 2026

Uh oh!

rrebollo commented Oct 26, 2026

Uh oh!

jelenapoblet left a comment

Choose a reason for hiding this comment

Uh oh!

jelenapoblet commented Oct 26, 2026

Uh oh!

OCA-git-bot commented Oct 26, 2026

Uh oh!

Uh oh!

OCA-git-bot commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rrebollo commented Jun 2, 2026 •

edited

Loading