Skip to main content
add link to github issue
Source Link
Fabian Schmengler
  • 66.1k
  • 25
  • 188
  • 422

Found the culprit and it has nothing to do with the test framework: Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin which adds commit callbacks for saving and deleting products.

If an exception is thrown in a commit callback, rollback() is called additionally to commit()

private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
{
    try {
        $productResource->beginTransaction();
        $result = $proceed($product);
        $productResource->addCommitCallback(function () use ($product) {
            $this->reindexRow($product->getEntityId());
        });
        $productResource->commit();
    } catch (\Exception $e) {
        $productResource->rollBack();
        throw $e;
    }

    return $result;
}

In Magento\Framework\Model\ResourceModel\AbstractResource, exceptions from the commit callbacks are simply rethrown:

public function commit()
{
    $this->getConnection()->commit();
    /**
     * Process after commit callbacks
     */
    if ($this->getConnection()->getTransactionLevel() === 0) {
        $callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
        try {
            foreach ($callbacks as $callback) {
                call_user_func($callback);
            }
        } catch (\Exception $e) {
            throw $e;
        }
    }
    return $this;
}

The two methods together lead to this nasty bug. To fix it, either exceptions from commit() or exceptions from the callbacks should probably be wrapped into a different exception type that can be handled differently by the core plugin.

I created an issue on GitHub: https://github.com/magento/magento2/issues/6497

Until this is fixed, "just" make sure that commit callbacks never throw exceptions...

Found the culprit and it has nothing to do with the test framework: Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin which adds commit callbacks for saving and deleting products.

If an exception is thrown in a commit callback, rollback() is called additionally to commit()

private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
{
    try {
        $productResource->beginTransaction();
        $result = $proceed($product);
        $productResource->addCommitCallback(function () use ($product) {
            $this->reindexRow($product->getEntityId());
        });
        $productResource->commit();
    } catch (\Exception $e) {
        $productResource->rollBack();
        throw $e;
    }

    return $result;
}

In Magento\Framework\Model\ResourceModel\AbstractResource, exceptions from the commit callbacks are simply rethrown:

public function commit()
{
    $this->getConnection()->commit();
    /**
     * Process after commit callbacks
     */
    if ($this->getConnection()->getTransactionLevel() === 0) {
        $callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
        try {
            foreach ($callbacks as $callback) {
                call_user_func($callback);
            }
        } catch (\Exception $e) {
            throw $e;
        }
    }
    return $this;
}

The two methods together lead to this nasty bug. To fix it, either exceptions from commit() or exceptions from the callbacks should probably be wrapped into a different exception type that can be handled differently by the core plugin.

Until this is fixed, "just" make sure that commit callbacks never throw exceptions...

Found the culprit and it has nothing to do with the test framework: Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin which adds commit callbacks for saving and deleting products.

If an exception is thrown in a commit callback, rollback() is called additionally to commit()

private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
{
    try {
        $productResource->beginTransaction();
        $result = $proceed($product);
        $productResource->addCommitCallback(function () use ($product) {
            $this->reindexRow($product->getEntityId());
        });
        $productResource->commit();
    } catch (\Exception $e) {
        $productResource->rollBack();
        throw $e;
    }

    return $result;
}

In Magento\Framework\Model\ResourceModel\AbstractResource, exceptions from the commit callbacks are simply rethrown:

public function commit()
{
    $this->getConnection()->commit();
    /**
     * Process after commit callbacks
     */
    if ($this->getConnection()->getTransactionLevel() === 0) {
        $callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
        try {
            foreach ($callbacks as $callback) {
                call_user_func($callback);
            }
        } catch (\Exception $e) {
            throw $e;
        }
    }
    return $this;
}

The two methods together lead to this nasty bug. To fix it, either exceptions from commit() or exceptions from the callbacks should probably be wrapped into a different exception type that can be handled differently by the core plugin.

I created an issue on GitHub: https://github.com/magento/magento2/issues/6497

Until this is fixed, "just" make sure that commit callbacks never throw exceptions...

Source Link
Fabian Schmengler
  • 66.1k
  • 25
  • 188
  • 422

Found the culprit and it has nothing to do with the test framework: Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin which adds commit callbacks for saving and deleting products.

If an exception is thrown in a commit callback, rollback() is called additionally to commit()

private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
{
    try {
        $productResource->beginTransaction();
        $result = $proceed($product);
        $productResource->addCommitCallback(function () use ($product) {
            $this->reindexRow($product->getEntityId());
        });
        $productResource->commit();
    } catch (\Exception $e) {
        $productResource->rollBack();
        throw $e;
    }

    return $result;
}

In Magento\Framework\Model\ResourceModel\AbstractResource, exceptions from the commit callbacks are simply rethrown:

public function commit()
{
    $this->getConnection()->commit();
    /**
     * Process after commit callbacks
     */
    if ($this->getConnection()->getTransactionLevel() === 0) {
        $callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
        try {
            foreach ($callbacks as $callback) {
                call_user_func($callback);
            }
        } catch (\Exception $e) {
            throw $e;
        }
    }
    return $this;
}

The two methods together lead to this nasty bug. To fix it, either exceptions from commit() or exceptions from the callbacks should probably be wrapped into a different exception type that can be handled differently by the core plugin.

Until this is fixed, "just" make sure that commit callbacks never throw exceptions...