Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doctrine\DBAL\Driver\ExceptionConverterDriver is deprecated #129

Closed
garak opened this issue Sep 21, 2020 · 18 comments
Closed

Doctrine\DBAL\Driver\ExceptionConverterDriver is deprecated #129

garak opened this issue Sep 21, 2020 · 18 comments

Comments

@garak
Copy link
Contributor

garak commented Sep 21, 2020

After upgrading to doctrine/dbal 2.11:

The "DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver" class implements "Doctrine\DBAL\Driver\ExceptionConverterDriver" that is deprecated in PHPUnitExtension::executeBeforeFirstTest

@dmaicher dmaicher changed the title Deprecated Doctrine\DBAL\Driver\ExceptionConverterDriver is deprecated Sep 22, 2020
@dmaicher
Copy link
Owner

I don't see a way to fix this in a forward compatible way 😕 We can fix it here for doctrine/dbal 3 and not implement that interface anymore.

Checked doctrine/dbal#4118 and https://github.com/doctrine/dbal/pull/4129/files

Or do you see a way to get rid of the deprecation already?

@garak
Copy link
Contributor Author

garak commented Sep 23, 2020

Can't we just stop implementing ExceptionConverterDriver?

@dmaicher
Copy link
Owner

Can't we just stop implementing ExceptionConverterDriver?

Well that will probably have side effects? See https://github.com/doctrine/dbal/blob/2.11.x/lib/Doctrine/DBAL/DBALException.php#L181

@garak
Copy link
Contributor Author

garak commented Sep 23, 2020

Do you think? I see that exception is converted with the old interface, otherwise is re-thrown as generic Exception. Is that so different?

@dmaicher
Copy link
Owner

dmaicher commented Sep 23, 2020

I think it would be bad as the test environment would behave differently than dev/prod environments if the underlying driver implements ExceptionConverterDriver.

For example I'm mostly using Doctrine\DBAL\Driver\PDOMySql\Driver which will throw specific exceptions. So in my app I also handle those specific exceptions in some cases. For example UniqueConstraintViolationException.

If now in the tests the StaticDriver of this bundle will not throw any specific exceptions anymore it could certainly break things.

@dmaicher
Copy link
Owner

dmaicher commented Sep 23, 2020

I quickly confirmed it with a functional test.

This works without any changes:

    public function testWillThrowSpecificException(): void
    {
        $this->expectException(TableNotFoundException::class);
        $this->connection->insert('does_not_exist', ['foo' => 'bar']);
    }

and it fails when StaticDriver does not implement ExceptionConverterDriver anymore:

1) Tests\Functional\FunctionalTest::testWillThrowSpecificException
Failed asserting that exception of type "Doctrine\DBAL\Exception" matches expected exception "Doctrine\DBAL\Exception\TableNotFoundException". Message was: "An exception occurred while executing 'INSERT INTO does_not_exist (foo) VALUES (?)' with params ["bar"]:

@garak
Copy link
Contributor Author

garak commented Sep 23, 2020

I see.
I guess we should keep it as is for now.
I would keep this issue open as well, just as a reminder.

@Pierstoval
Copy link

Why not create a different PHPUnitExtension class for latest DBAL versions?

One would use a StaticDriver that implementst the interface, and the new one would implement another StaticDriver without the interface.

Makes sure that using the interface is opt-out, it's backwards-compatible, and when Doctrine DBAL removes the class, migration path is quicker, since the legacy interface might then have a runtime check on the interface.

WDYT?

@dmaicher
Copy link
Owner

@Pierstoval yes for DBAL 3 we will need a StaticDriver that does not implement the interface anymore.

But for dbal 2.11+, < 3 we still need to implement the deprecated interface 😕

@Pierstoval
Copy link

Let's wait until DBAL 3 release then 👍

@franck-grenier
Copy link

Hello,
I'm having the same deprecation error and I cannot have my data reset on each test.

Does it make transactions rollback fail ?

@dmaicher
Copy link
Owner

Does it make transactions rollback fail ?

No. Its just a deprecation. You can safely ignore it.

@dmaicher
Copy link
Owner

Compatibility with dbal 3 is merged & released. As I don't see any way of fixing this for dbal 2.x (unless there is some forward compatibility layer on dbal 2.x for it) I will close here.

@BenMorel
Copy link

@dmaicher Would creating a new release that's only compatible with dbal 3.x be an option? I'll understand if you're not willing to maintain 2 release streams, but this would be the cleanest option IMO.

@dmaicher
Copy link
Owner

@BenMorel are you still seeing this deprecation when using dbal 3?

@BenMorel
Copy link

@dmaicher Sorry, I think I misunderstood the problem. We're using doctrine/dbal 2.13.

I guess this boils down to releasing a version that would be compatible with 2.11+ only? The aim being to have a version that's clear of deprecation warnings.

@dmaicher
Copy link
Owner

I guess this boils down to releasing a version that would be compatible with 2.11+ only? The aim being to have a version that's clear of deprecation warnings.

This is impossible. The driver interface is deprecated on dbal 2.x, but there is no replacement as far as I see. So we need to use it on 2.x as otherwise the functionality is broken.

@BenMorel
Copy link

Oh right, I see now. They've deprecated the ExceptionConverterDriver because the convertException() method has been moved to the Driver interface. It's really a pity to see no way to remove the deprecation warning that clutters the tests...

Anyway, sorry for the noise!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants