[CDRIVER-1110] Add mongoc_cursor_get_error_document () Created: 11/Feb/16 Updated: 30/May/19 Resolved: 03/May/17 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | libmongoc |
| Affects Version/s: | 1.3.3 |
| Fix Version/s: | 1.7.0 |
| Type: | New Feature | Priority: | Major - P3 |
| Reporter: | Jeremy Mikola | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Description |
|
Original title: Allow clients to access failed command response document (e.g. ok:0) – |
| Comments |
| Comment by Githook User [ 07/May/17 ] | ||||||||||||||||||||||||||
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: | ||||||||||||||||||||||||||
| Comment by Githook User [ 03/May/17 ] | ||||||||||||||||||||||||||
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: | ||||||||||||||||||||||||||
| Comment by Githook User [ 03/May/17 ] | ||||||||||||||||||||||||||
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: | ||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
Yes, that's safe to assume, safer actually. Personally I care deeply about the contract, "If something fails due to a network error, it sets a bson_error_t and returns false." That's tested and documented. "If something fails due to a network error, it writes a log message," is not a contract we tend to document and test. | ||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
jesse: That API looks usable. I have one question regarding network errors. Since we currently rely on our log handler (php_phongo_log()) setting EG(exception) after an error/critical log event, we don't bother to check mongoc_cursor_error() for such errors. Is it safe to assume that any runtime error we'd get through a previous log message would be reported mongoc_cursor_error() and the new function? I suppose the only edge case might be invalid arguments, which I'd expect libmongoc to report before attempting to send something across the wire – but we really shouldn't be causing any of those. | ||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
Understood. Something like this?:
You'd use it like:
By the way I notice a "FIXME" in Phongo to deal with a NULL return from mongoc_collection_find_with_opts – that can never happen, mongoc_collection_find_with_opts always returns a cursor. | ||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
There's no rush for having this in 1.7.0. It's just a nice-to-have. | ||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
What release do you need it in? | ||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 27/Apr/17 ] | ||||||||||||||||||||||||||
|
Apologies for losing track of this. The PHP driver uses mongoc_client_command(), which gives us a cursor. We use then use a phongo_advance_cursor_and_check_for_error() helper function to check for errors. If mongoc_cursor_next() returns false, one of the following is the case:
If, after advancing successfully, we find that the first result has a "cursor" document field, we use mongoc_cursor_new_from_command_reply() to convert that result into a command cursor. We then advance and check for an error once more before returning. I don't believe mongoc_cursor_error() would ever trigger for a command cursor, so the second phongo_advance_cursor_and_check_for_error() is just useful for picking up on connection errors (second bullet in the previous list). But for either case (basic cursor or command cursor), we can never access the first result document after advancing. This is certainly the case for picking up an $err field from OP_QUERY, but I expect it also applies to command failures (e.g. find returns ok:0 for some reason). We're requesting an API that could return the full document responsible for the bson_error_t | ||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 20/Apr/17 ] | ||||||||||||||||||||||||||
|
I guess we could add mongoc_cursor_get_error_document () or something. I'm not sure how this would be used in practice though, jmikola how do you envision this working? | ||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 27/Mar/17 ] | ||||||||||||||||||||||||||
|
If you use mongoc_client_command_simple, etc., then the error reply is in the bson_t *reply out-pointer:
Is that helpful? Or do you need the old mongoc_client_command (which returns a mongoc_cursor_t) to also return the error reply? | ||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 27/Mar/17 ] | ||||||||||||||||||||||||||
|
We still have no way to provide what is described in | ||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 27/Mar/17 ] | ||||||||||||||||||||||||||
|
jmikola is this still needed? Do the mongoc_client_command_simple and other command functions not already provide what you need? | ||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 11/Feb/16 ] | ||||||||||||||||||||||||||
This is not required for successful resolution of
|