[18.0][FIX] base_exception: Rollback transaction if we detect exceptions by grindtildeath · Pull Request #3590 · OCA/server-tools

grindtildeath · 2026-04-01T18:48:30Z

This aims to solve issues when modules not depending on whatever implementation of base_exception, override the same function that triggers the detection of exceptions.

Before this commit, any changes done in such overrides could end up being committed to the database if the MRO did execute such function before the function implementing base_exception that avoids to call super in case an exception is detected. (eg sale.order. action_confirm in sale_exception)

With this commit, in case there is any newly detected exception, or a record with exception that is not ignored, or a blocking exception linked to a record, exception changes will be committed in DB while a specific Exception type will be raised to rollback any changes done in the ongoing transaction. Such an exception will be handled in the UI to refresh the exception_ids field so that the user knows why the action was not completed.

Replaces and fixes:

Dependent PRs:

OCA-git-bot · 2026-04-01T18:48:38Z

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

ivantodorovich · 2026-04-01T19:09:46Z

base_exception/models/base_exception_method.py

+        raise_exception = False
+        # Write exceptions in a new transaction to be committed so that we can
+        #  rollback the ongoing one while keeping the exceptions stored
+        with Registry(self.env.cr.dbname).cursor() as new_cr:


Suggested change

with Registry(self.env.cr.dbname).cursor() as new_cr:

with self.env.registry.cursor() as new_cr:

no need to initialize a new registry

ivantodorovich · 2026-04-01T19:11:25Z

base_exception/models/base_exception_method.py

+        # Write exceptions in a new transaction to be committed so that we can
+        #  rollback the ongoing one while keeping the exceptions stored
+        with Registry(self.env.cr.dbname).cursor() as new_cr:
+            new_env = Environment(new_cr, self.env.uid, self.env.context)


Suggested change

new_env = Environment(new_cr, self.env.uid, self.env.context)

records_env = records.with_env(self.env(cr=new_cr))

to avoid all the with_env everytime you read/write, initialize a records_env variable.

also you can use self.env(cr=) like this

Actually we have many sets of records:

self

values of rules_to_add

values of rules_to_add

For each one we need to use the new cursor, so I guess it makes sense to create the new_env this way. AFAIU self.env(cr=new_cr) would just do the same, but we need to reuse it many times anyway

ivantodorovich · 2026-04-01T19:26:22Z

base_exception/models/base_exception_method.py

+            # In case we have new exception, or exceptions that were not ignored yet, or
+            #  blocking exceptions, we need to raise an exception to rollback the ongoing
+            #  transaction
+            if (
+                rules_to_add
+                or not all(
+                    rec.ignore_exception
+                    for rec in self.with_env(new_env)
+                    if rec.exception_ids
+                )
+                or any(
+                    rule.is_blocking
+                    for rule in self.with_env(new_env).mapped("exception_ids")
+                )
+            ):
+                raise_exception = True


Maybe extract this to a method records_env._should_raise_exception ?

hparfr · 2026-04-02T07:04:59Z

ping @florian-dacosta

This aims to solve issues when modules not depending on whatever implementation of base_exception, override the same function that triggers the detection of exceptions. Before this commit, any changes done in such overrides could end up being committed to the database if the MRO did execute such function before the function implementing base_exception that avoids to call super in case an exception is detected. (eg sale.order. action_confirm in sale_exception) With this commit, in case there is any newly detected exception, or a record with exception that is not ignored, or a blocking exception linked to a record, exception changes will be committed in DB while a specific Exception type will be raised to rollback any changes done in the ongoing transaction. Such an exception will be handled in the UI to refresh the exception_ids field so that the user knows why the action was not completed.

With the previous change raising an exception to rollback a transaction, the function `_popup_exceptions` could not be called anymore while the error was raised. Instead, we provide now a hook `_must_popup_exception` that can be redefined by model and will be called when `action_popup_exception` is called by the webclient, to smoothly refresh the page and display the popup exception wizard.

With OCA/server-tools#3590 changing the way exceptions are detected, the error raising doesn't allow to call _popup_exception as we used to, but we can use the new hook to have the popup displayed as it used to.

With OCA/server-tools#3590 changing the way exceptions are detected, the error raising doesn't allow to call `_popup_exception` as we used to, but we can use the new hook to have the popup displayed smoothly.

With OCA/server-tools#3590 changing the way exceptions are detected, the error raising doesn't allow to call _popup_exception as we used to, but we can use the new hook to have the popup displayed smoothly.

…ollback With OCA/server-tools#3590 changing the way exceptions are detected, the error raising doesn't allow to call _popup_exception as we used to, but we can use the new hook to have the popup displayed smoothly.

grindtildeath · 2026-04-06T16:21:58Z

ping @jok-adhoc @lav-adhoc @lef-adhoc @maq-adhoc @rousseldenis @florian-dacosta

After spending hours on this issue, IMO this solves the core issue we have with non dependent module having their changes done in action_confirm committed after an exception was detected since the transaction was not fully rolled back.

I then changed the way the exception popup can be displayed and it seems to work smoothly with no side effects and no need of glue modules.

Could you guys please have a look, eventually test this (with their dependent PRs) and let me know what do you think? 🙏

rousseldenis

LGTM.

Great! @grindtildeath Thanks for this, this seems indeed cleaner to solve the historical issue we have with this.

ivantodorovich approved these changes Apr 1, 2026

View reviewed changes

sebastienbeau requested review from hparfr and sebastienbeau April 1, 2026 23:09

sebastienbeau assigned sebastienbeau and unassigned sebastienbeau Apr 1, 2026

grindtildeath force-pushed the 18.0-fix-base_exception_rollback_transaction_fix_ui branch from 9e62007 to 4ea380d Compare April 2, 2026 13:22

grindtildeath force-pushed the 18.0-fix-base_exception_rollback_transaction_fix_ui branch from 4ea380d to 61eed8a Compare April 2, 2026 15:10

Fix tests

9a1c94c

ivantodorovich approved these changes Apr 6, 2026

View reviewed changes

grindtildeath mentioned this pull request Apr 6, 2026

[18.0][IMP] sale_exception: Use hook to popup exception after rollback OCA/sale-workflow#4255

Open

1 task

grindtildeath mentioned this pull request Apr 6, 2026

[18.0][IMP] sale_exception: Use hook to popup exception after rollback OCA/purchase-workflow#3012

Open

1 task

rousseldenis approved these changes Apr 7, 2026

View reviewed changes

OCA-git-bot added the approved label Apr 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[18.0][FIX] base_exception: Rollback transaction if we detect exceptions#3590

[18.0][FIX] base_exception: Rollback transaction if we detect exceptions#3590
grindtildeath wants to merge 3 commits intoOCA:18.0from
camptocamp:18.0-fix-base_exception_rollback_transaction_fix_ui

grindtildeath commented Apr 1, 2026 •

edited

Loading

Uh oh!

OCA-git-bot commented Apr 1, 2026

Uh oh!

ivantodorovich Apr 1, 2026

Uh oh!

ivantodorovich Apr 1, 2026

Uh oh!

grindtildeath Apr 2, 2026

Uh oh!

ivantodorovich Apr 1, 2026

Uh oh!

hparfr commented Apr 2, 2026

Uh oh!

grindtildeath commented Apr 6, 2026

Uh oh!

rousseldenis left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	with Registry(self.env.cr.dbname).cursor() as new_cr:
	with self.env.registry.cursor() as new_cr:

	new_env = Environment(new_cr, self.env.uid, self.env.context)
	records_env = records.with_env(self.env(cr=new_cr))

Uh oh!

Conversation

grindtildeath commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OCA-git-bot commented Apr 1, 2026

Uh oh!

ivantodorovich Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ivantodorovich Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

grindtildeath Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

ivantodorovich Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

hparfr commented Apr 2, 2026

Uh oh!

grindtildeath commented Apr 6, 2026

Uh oh!

rousseldenis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

grindtildeath commented Apr 1, 2026 •

edited

Loading