Skip to content

Exceptions don't include the message id they are an exception for #177

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.

Sign up for GitHub

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

Jump to bottom
Closed
gwvandesteeg opened this issue Feb 23, 2018 · 4 comments
Closed

Exceptions don't include the message id they are an exception for #177

gwvandesteeg opened this issue Feb 23, 2018 · 4 comments
Labels
help wanted
Milestone
3.3

Comments

@gwvandesteeg
Copy link

Issue description:
When using the async methods and polling for results using the result(msg=RES_ANY), an exception can get raised since one of the async calls generated caused the error.
The exception raised does not include the message id of the request that caused the exception.
Having 10 outstanding requests and receiving an exception for one (ie. ldap.ALREADY_EXISTS) and not knowing which request this was for since we're not receiving the msgid in the response.

Steps to reproduce:
connect to python
alternate an add and a search query using the same query three times (each) using the async methods (or randomise the order)
poll for results until you've received a response for each outstanding request.
all the searches should complete, one of the adds should complete, and two of the adds should've failed, but which add.. which message ids.

Operating system:
Linux

Python version:
2.7+

python-ldap version:
all
@encukou
Copy link
Member

encukou commented Feb 26, 2018

Thank you for reporting the issue!
Would you be willing to a propose fix, or perhaps a (failing) test case to illustrate how you would like this to be solved?

@tiran
Copy link
Member

tiran commented Feb 26, 2018

This should be easy to solve by changing PyObject* LDAPerror(LDAP*, char* msg); to PyObject* LDAPerror(LDAP*, char* msg, int msgid);. We also need a value to signal that we don't have a value msgid. A msgid is a positive int except for two special cases: LDAP_RES_ANY is defined as -1 and LDAP_RES_UNSOLICITED as 0. Any value small than -1 should do the trick.

@gwvandesteeg
Copy link
Author

That is basically the same idea I had when looking at the code base, initialise msgid to None and then set it if we get one, or a negative value will do just as well for no message id present.

@spaceone
Copy link
Contributor

I would like to see this as well :-)

@mistotebe mistotebe mentioned this issue Apr 17, 2019
Open
mistotebe added a commit to mistotebe/python-ldap that referenced this issue May 2, 2019
@mistotebe
Attach msgid/controls/... to exceptions (Closes python-ldap#177, pyth…
69832b5 
…on-ldap#208)

Closes:
python-ldap#177 (msgid)
python-ldap#208 (controls)
@mistotebe mistotebe mentioned this issue May 2, 2019
Closed
mistotebe added a commit to mistotebe/python-ldap that referenced this issue May 2, 2019
@mistotebe
Attach msgid/controls/... to exceptions (Closes python-ldap#177, pyth…
7ae8ea1 
…on-ldap#208)

Closes:
python-ldap#177 (msgid)
python-ldap#208 (controls)
mistotebe added a commit to mistotebe/python-ldap that referenced this issue May 2, 2019
@mistotebe
Attach msgid/controls/... to exceptions (Closes python-ldap#177, pyth…
d006fd2 
…on-ldap#208)

Closes:
python-ldap#177 (msgid)
python-ldap#208 (controls)
mistotebe added a commit to mistotebe/python-ldap that referenced this issue May 2, 2019
@mistotebe
Attach msgid/controls/... to exceptions (Closes python-ldap#177, pyth…
22f3367 
…on-ldap#208)

Closes:
python-ldap#177 (msgid)
python-ldap#208 (controls)
@mistotebe mistotebe mentioned this issue May 17, 2019
Merged
@krisztian-kovacs krisztian-kovacs mentioned this issue Feb 13, 2020
Merged
mistotebe added a commit to mistotebe/python-ldap that referenced this issue Apr 30, 2020
@mistotebe
Attach msgid/controls/... to exceptions (Closes python-ldap#177, pyth…
dc00796 
…on-ldap#208)

Closes:
python-ldap#177 (msgid)
python-ldap#208 (controls)
@encukou encukou closed this as completed in f754e35 Jun 5, 2020
encukou added a commit that referenced this issue Jun 5, 2020
@encukou
Add additional exception fields
3c0f7ef 
Work to improve how we raise exceptions on result, deals with controls and msgid being missing

Fixes: #177
Fixes: #208

#288
@tiran tiran added this to the 3.3 milestone Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted
Projects
None yet
Milestone
3.3
Development

No branches or pull requests
4 participants
@encukou @tiran @spaceone @gwvandesteeg