Skip to content

Loading…

DBAL-813: Original PDOException is dropped from previous property in wrapper #2046

Closed
doctrinebot opened this Issue · 9 comments

2 participants

@doctrinebot

Jira issue originally created by user hosiplan:

262deeb#diff-276fe718ac0aa7fc20162d42ae4dc7b0R52

Is there any case when PDOException has it's own previous? I don't know of any. Even if it had one, it doesn't make sense because you're skipping one important!! piece in chain of exceptions that cause each other

Do you agree that the ->getPrevious() is wrong and should be removed? Should I send a pullrequest?

@doctrinebot

Comment created by @deeky666:

After having added the missing information from the wrapped \PDOException to Doctrine\DBAL\Driver\PDOException in commit:
https://github.com/doctrine/dbal/blob/b362492900bc59800441e0d9922736fc55bf8c41/lib/Doctrine/DBAL/Driver/PDOException.php#L54-L55
all information from \PDOException should be retrievable from the Doctrine\DBAL\Driver\PDOException wrapper now. Therefore there should be no change in behaviour in existing applications.
[~hosiplan] Are you okay with the solution? Can this ticket be closed?

@doctrinebot

Comment created by hosiplan:

I disagree, first of all you're dropping a stack trace of that exception and replacing it with other from higher level, it may not be all that important, but all best practises say that you should always pass the exception to $previous.

There is zero disadvantages when keeping the exception in the chain. The few kilobytes of memory saved means nothing in comparision to backwards compatibility (which was broken without any real benefit).

@doctrinebot

Comment created by @ocramius:

I agree that the original PDOException should be passed in as $previous, since PDOExceptions themselves may or may not have a non-null getPrevious() result.

[~hosiplan] can you provide a pull request for that?

@doctrinebot

Comment created by hosiplan:

Yes I can! There you go #534 let me know if it needs any adjustments.

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-534] was closed:
#534

@doctrinebot

Comment created by @ocramius:

merged: 646edac

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot

Comment created by hosiplan:

Thank you very much!

@doctrinebot doctrinebot added the Bug label
@deeky666 deeky666 was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.5 milestone
@doctrinebot doctrinebot closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.