Spin off from #3348590: Add transaction-related events to the Database API.

Problem/Motivation

While implementing a set of events around the database transaction management process in #3348590: Add transaction-related events to the Database API, and later while working on #3584322: Cleanup transactions in try/catch blocks, I noticed that there are inconsistencies on dealing with unhappy paths when a transaction-related operation fails: sometimes an exception is thrown to the calling code, sometimes not, and the state of the manager after failure is unclear.

This issue is splitting up work in #3348590: Add transaction-related events to the Database API so that the inconsistencies are fixed prior than and independently from the implementation of the transaction events.

Proposed resolution

Fix inconsistencies in the TransactionManager behavior and stack, especially for the cases of failure of commit/release/rollback operations.

Add a unit test to make sure we do not have future regression in the TransactionManager behavior.

Remaining tasks

User interface changes

None.

API changes

Data model changes

None.

Release notes snippet

Issue fork drupal-3584465

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake opened merge request !15444

mondrake’s picture

mondrake
Location 🇮🇹
commented
Category: Feature request » Bug report
Status: Active » Needs review

We were completely missing tests for the case when transaction methods fail. Added one.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Parent issue: #3348590: Add transaction-related events to the Database API » #3495728: [meta] Drop implicit autocommit on Transaction destruction
Related issues: +#3348590: Add transaction-related events to the Database API
smustgrave’s picture

smustgrave commented

Any suggestion on how to review this one?

mondrake’s picture

mondrake
Location 🇮🇹
commented

#7 from the tests up: the new test class has individual methods that check how various situations are handled. Review if each method has reasonable assumptions and if the runtime code has been implemented properly.

Unfortunately creating ‘real’ transaction failure scenarios is hard - those are possibly linked to physical failures (no storage space, network failures etc).

The stub connection implementation is done to enable simulating a transaction failure.