Skip to content

[18.0][FIX] base_exception: allow detecting exceptions without rollback - #3741

Open
gal-adhoc wants to merge 1 commit into
OCA:18.0from
adhoc-dev:18.0-t-75454-gal
Open

gal-adhoc wants to merge 1 commit into
OCA:18.0from
adhoc-dev:18.0-t-75454-gal

Conversation

@gal-adhoc

@gal-adhoc gal-adhoc commented Sep 24, 2026 •

Copy link
Copy Markdown

Follow-up of #3590.

Problem

detect_exceptions() stores the exceptions through a second cursor, so the ongoing transaction can be rolled back. When the ongoing transaction already wrote on the main records, the second cursor waits on the row lock that the same thread holds: the request never ends (until limit_time_real kills the worker).

Real case (Odoo 18, sale_exception): signing a quotation in the customer portal. portal_quote_accept writes the signature and flushes, then _validate_order() → action_confirm() → detect_exceptions(). pg_stat_activity shows the main connection idle in transaction and the second one waiting on Lock / transactionid for UPDATE sale_order SET main_exception_id ....

Change

_exceptions_rollback(), true by default. It is false for the main records listed in the context base_exception_no_rollback={model: ids}, and then detect_exceptions() stores the exceptions with the current cursor and does not raise BaseExceptionError. The caller decides with _must_raise_exception_after_detection().

The flag is scoped by model and ids on purpose: other exception models reached from the same call (e.g. purchase or stock exceptions triggered by the sale confirmation) keep the default behavior.

Nothing changes when the context key is not set.

The consumer is the sale-workflow PR for sale_exception, which will reference this one.

Relation with #3739

The two PRs solve different halves of the problem. #3739 fixes the hang itself, for any caller that wrote earlier in the transaction, and still raises BaseExceptionError so the backend popup keeps working. This PR does not fix that hang: with it applied, confirming an order from the backend after an earlier write on the same row still waits on the row lock forever. What it adds is a way for the caller to ask for no rollback, which #3739 does not cover, and which the portal needs to keep the signature it already wrote.

They compose: with this context set, new_env is self.env, which #3739 writes with directly. Both touch the same lines of detect_exceptions(), so whichever lands second needs a rebase. Happy to rebase this one on top of #3739.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @sebastienbeau, @hparfr,
some modules you are maintaining are being modified, check this out!

@grindtildeath grindtildeath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for the PR, I've seen it's WIP but here's a small comment.

However, did you check #3739 yet?

):
raise_exception = True
if raise_exception:
if raise_exception and not no_rollback:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpicking a little bit, but couldn't you name the function _exceptions_rollback and default to True instead of using a negative and having a double negation here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point, done: _exceptions_rollback() is true by default and the double negation is gone.

The context key keeps the negative name (base_exception_no_rollback), so that not setting it means the current behavior.

Add _exceptions_rollback(), true by default: it is false for the main
records listed in the base_exception_no_rollback context ({model: ids}),
and then the exceptions are stored in the ongoing transaction and
detect_exceptions() does not raise.

Callers that already wrote on those records in the transaction cannot
use the second cursor: it waits forever on the row lock held by the
ongoing transaction.
@gal-adhoc gal-adhoc changed the title [WIP] [18.0][FIX] base_exception: allow detecting exceptions without rollback [18.0][FIX] base_exception: allow detecting exceptions without rollback Sep 29, 2026
@gal-adhoc

Copy link
Copy Markdown
Author

Thanks for the review, and for pointing at #3739 — I had missed it, it was opened a few hours before this one.

The two PRs solve different halves of the problem and do not overlap:

  • [18.0][FIX] base_exception: don't hang when the ongoing transaction locks the rows #3739 fixes the hang itself, for any caller that wrote earlier in the transaction, and still raises BaseExceptionError so the backend popup keeps working. This PR does not fix that: with it applied, confirming an order from the backend after an earlier write on the same row still hangs (main connection idle in transaction, independent one waiting on Lock / transactionid for UPDATE sale_order SET main_exception_id ...). I checked it on an Odoo 18 database before writing this.
  • This PR lets the caller ask for no rollback, which [18.0][FIX] base_exception: don't hang when the ongoing transaction locks the rows #3739 does not cover. In the customer portal the controller writes the signature and flushes it before confirming, so a rollback loses it, and the error reaches a client with no handler for it (the handler is declared in web.assets_backend only, so the portal shows the raw payload). The free order flow of website_sale goes through the same method.

So #3739 alone would turn the portal hang into a lost signature after 2 seconds, and this PR alone leaves the backend hang unfixed. Together they cover both.

They compose cleanly: with the context of this PR set, new_env is self.env, which #3739 writes with directly. They do touch the same lines of detect_exceptions(), so whichever lands second needs a rebase — happy to rebase this one on top of #3739.

The consumer is OCA/sale-workflow#4600, where _validate_order() leaves the blocked orders unconfirmed, keeps the signature and posts an internal note with the exceptions.

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