[Ldap] Add LDAP error codes to exceptions #33339
Conversation
|
I like your work but to me it’s a BC break not about the error code (which could be handled and deprecated first) but someone who is catching an LdapException now got a different one and the try catch doesn’t work as expected anymore... This could be reworked b extending LdapException by the newly introduced exceptions which would be BC. |
I first thought about it. Though if any of the new exceptions is thrown, the using software cannot work correctly nowadays (and I wanted to avoid extra work to fix the class hierarchy in Sf 5). Nevertheless, you are right, to completely keep the same behavior they should extend Though that we have a valid scope is already ensured by the OptionsResolver: |
|
BC is now ensured (apart from |
This PR ensures that an LdapException is only used when an LDAP operations fails. In cases an LdapException was used for non LDAP operations it is replaced by an appropriate exception.
c2f96d8 to
2dede53
Compare
|
Let me know if anything is left to change. |
| if (false === $searchCount) { | ||
| throw new LdapException(sprintf('Error while retrieving entry count: %s.', ldap_error($con))); | ||
| throw LdapException::create('Error while retrieving entry count: [{errorCode}] {errorMsg}.', ldap_errno($con)); |
There was a problem hiding this comment.
We are not doing this in Symfony, can you please revert the ::create() method?
|
I would take care of this PR if we get no feedback by the author |
|
@OskarStark I'm currently busy with other stuff, so feel free to take it up. |
|
Could you please add me as a collaborator to your fork? |
Done |
|
Closing in favor of #35913 |
This PR ensures that an LdapException is only used when an LDAP
operations fails. In cases an LdapException was used for non LDAP
operations it is replaced by an appropriate exception.
LdapExceptionrequires error code now. (I wouldn't expect it to be relevant for any component user.)This PR ensures that an
LdapExceptionis only used when an LDAP operations fails. The constructor ofLdapExceptionenforces that an error code is specified. In cases anLdapExceptionwas used for non LDAP operations it is replaced by an appropriate exception.Additionally silencing is added to ldap functions where missing.