[CXX-1089] document::element::operator[] overloads should not throw when given an invalid element Created: 13/Oct/16 Updated: 15/Nov/16 Resolved: 28/Oct/16 |
|
| Status: | Closed |
| Project: | C++ Driver |
| Component/s: | BSON |
| Affects Version/s: | None |
| Fix Version/s: | 3.0.3 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Armin Ball [X] | Assignee: | David Golden |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Major Change | ||||||||
| Description |
|
bsoncxx::document::element::operator[] is documented as non-throwing, however the expression bsoncxx::document::element{}["foo"] throws. A fix for this issue was attempted in Original description:
|
| Comments |
| Comment by Githook User [ 08/Nov/16 ] | |||||||||||||||||
|
Author: {u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}Message: For any valid element of the wrong type, calling operator[] would This commit makes operator[] not throw for the invalid element either | |||||||||||||||||
| Comment by Githook User [ 28/Oct/16 ] | |||||||||||||||||
|
Author: {u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}Message: For any valid element of the wrong type, calling operator[] would This commit makes operator[] not throw for the invalid element either | |||||||||||||||||
| Comment by David Golden [ 26/Oct/16 ] | |||||||||||||||||
| Comment by J Rassi [ 17/Oct/16 ] | |||||||||||||||||
|
david.golden: I agree with your proposal to make document::element::operator[] non-throwing. I've updated the summary/description to reflect this, and changed the type of the ticket to "Bug". Weltenbummler: instead of your original suggestion of improving the error message, we've decided to remove the throwing behavior altogether. In your example, blubb["jolo"][1] will evaluate to an invalid element (which is false in a boolean context), instead of throwing. | |||||||||||||||||
| Comment by David Golden [ 13/Oct/16 ] | |||||||||||||||||
|
I haven't stack traced it, but I think the error is coming the type() call in the operator[] methods:
We could instead first check for an empty element in the operator[] methods:
That returns an empty element (which would evaluate as false). It could also throw a more informative exception ("not an array" instead of "unset element"), but I'd be fine with allowing Armin's original case if ( blubb["jolo"][1] ) to be false without an exception for convenience to avoid if ( blubb["jolo"] && blubb["jolo"][1] ) for deeply nested fields. That's a simple enough change that if we agree to do it that way, we could even pull it up into 3.1-desired. | |||||||||||||||||
| Comment by Armin Ball [X] [ 13/Oct/16 ] | |||||||||||||||||
|
| |||||||||||||||||
| Comment by David Golden [ 13/Oct/16 ] | |||||||||||||||||
|
You raise some good points. We'll try to find a way to clarify that behavior. |