Skip to content

[FIX] edi_core_oca: use savepoint to prevent state corruption on errors#284

Merged
OCA-git-bot merged 1 commit into
OCA:18.0from
qoqa:18.0-fix-error-handling-state-corruption
Jul 3, 2026
Merged

[FIX] edi_core_oca: use savepoint to prevent state corruption on errors#284
OCA-git-bot merged 1 commit into
OCA:18.0from
qoqa:18.0-fix-error-handling-state-corruption

Conversation

@guewen

@guewen guewen commented May 21, 2026

Copy link
Copy Markdown
Member

Some errors such as

  ValueError,
  FileNotFoundError,
  exceptions.UserError,
  exceptions.ValidationError,

are caught, never re-raised, and the transaction is finally committed.

Which means a UserError/ValidationError gets committed, leading to corruption of the processed data.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @simahawk, @etobella,
some modules you are maintaining are being modified, check this out!

@guewen guewen force-pushed the 18.0-fix-error-handling-state-corruption branch from 8811e3e to eb1ea7f Compare May 27, 2026 12:37
@guewen guewen requested review from etobella and simahawk May 28, 2026 19:35
@guewen guewen force-pushed the 18.0-fix-error-handling-state-corruption branch from eb1ea7f to bace507 Compare July 2, 2026 10:09
@guewen

guewen commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Hi @etobella @simahawk, can you have a look please? This one can lead to nasty data corruption, for e.g. in our case quants quantities were modified when they should have been rolled back

@simahawk simahawk 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.

Good catch! Thank you!

@simahawk

simahawk commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

CI is red due to

  File "/__w/edi-framework/edi-framework/edi_core_oca/models/edi_exchange_record.py", line 170, in _compute_exchange_filename
    rec.exchange_filename = rec.type_id._make_exchange_filename(rec)
  File "/__w/edi-framework/edi-framework/edi_core_oca/models/edi_exchange_type.py", line 317, in _make_exchange_filename
    return pattern.format(
  File "/opt/odoo/odoo/_monkeypatches/_cpython.py", line 85, in format
    return origin_format(*args, **kwargs)
AttributeError: 'edi.exchange.consumer.test' object has no attribute 'ref'

That's because exchange_filename_pattern is set to use ref but the test model does not have it.
Weird it didn't fail before. Can you check that? (I would simply add a ref field to the test model)

@guewen guewen force-pushed the 18.0-fix-error-handling-state-corruption branch from bace507 to ed2c8a7 Compare July 2, 2026 16:02
@guewen

guewen commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

oops, I did actually fix the CI issue by changing the file pattern some time ago, but overwritten it today when rebasing 🤦 ☕

Some errors such as

  ValueError,
  FileNotFoundError,
  exceptions.UserError,
  exceptions.ValidationError,

are caught, never re-raised, and the transaction is finally committed.

Which means a UserError/ValidationError gets committed, leading to
corruption of the processed data.
@guewen guewen force-pushed the 18.0-fix-error-handling-state-corruption branch from ed2c8a7 to 989ce63 Compare July 2, 2026 16:06
@guewen

guewen commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Pushed your simpler proposition of adding a ref field

@simahawk

simahawk commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

/ocabot merge patch

Would you mind porting to v19? 🙏

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-284-by-simahawk-bump-patch, awaiting test results.

@etobella etobella left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense to me

@OCA-git-bot OCA-git-bot merged commit d967290 into OCA:18.0 Jul 3, 2026
6 of 7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@guewen

guewen commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Would you mind porting to v19? 🙏

Yep, done here: #316

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants