[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:
Duplicate
is duplicated by CXX-1091 document::element::operator[] overloa... Closed
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 CXX-862, but the fix was incomplete.

Original description:

bsoncxx::document view blubb;

if( blubb["jolo"] )
=> false

if( blubb["jolo"][1] )
terminate called after throwing an instance of 'bsoncxx::v_noabi::exception'
what(): unset document::element
For this, can the exception say, that I'm out of bound of a array, or that no array is existing?
Most awesome would be if the exception would tell the non existing position.

This behavior is a bit misleading. I would assume for both the upper behavior in just returning a true or false.



 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: CXX-1089 Make operator[] not throw when invalid

For any valid element of the wrong type, calling operator[] would
return an invalid element. Due to a quirk, calling operator[] on
the invalid element would throw.

This commit makes operator[] not throw for the invalid element either
and updates the documentation to clarify its behavior.
Branch: 3.1-dev
https://github.com/mongodb/mongo-cxx-driver/commit/bb5e88eeee853a35bd975016121136913783d32f

Comment by Githook User [ 28/Oct/16 ]

Author:

{u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}

Message: CXX-1089 Make operator[] not throw when invalid

For any valid element of the wrong type, calling operator[] would
return an invalid element. Due to a quirk, calling operator[] on
the invalid element would throw.

This commit makes operator[] not throw for the invalid element either
and updates the documentation to clarify its behavior.
Branch: master
https://github.com/mongodb/mongo-cxx-driver/commit/bb5e88eeee853a35bd975016121136913783d32f

Comment by David Golden [ 26/Oct/16 ]

https://mongodbcr.appspot.com/104180002/

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:

bsoncxx::type element::type() const {
    if (_raw == nullptr) {
        throw bsoncxx::exception{error_code::k_unset_element};
    }
    // ... rest omitted ... 
}
 
element element::operator[](stdx::string_view key) const {
    if (type() != bsoncxx::type::k_document) return element();
    // ... rest omitted ... 
}
 
array::element element::operator[](std::uint32_t i) const {
    if (type() != bsoncxx::type::k_array) return array::element();
    // ... rest omitted ... 
}

We could instead first check for an empty element in the operator[] methods:

if ( _raw==nullptr || type() != bsoncxx::type::k_document ) return element();

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.

Generated at Wed Feb 07 22:01:22 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.