[CXX-832] Friendlier error message when decoding Decimal128 BSONElements Created: 27/Jan/16 Updated: 21/Jun/16 Resolved: 17/Jun/16 |
|
| Status: | Closed |
| Project: | C++ Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | legacy-1.1.2 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Rathi Gnanasekaran | Assignee: | J Rassi |
| Resolution: | Done | Votes: | 0 |
| Labels: | legacy-cxx | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Description |
|
I'm kicking this out to legacy-1.1.2. This is a large and complex change given the structure of the BSON library in the legacy driver. |
| Comments |
| Comment by Githook User [ 17/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'jrassi', u'name': u'J. Rassi', u'email': u'rassi@10gen.com'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by J Rassi [ 16/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 16/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
That sounds reasonable. We can always update it later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by David Golden [ 16/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
By the time Decimal 128 is available in 3.4, we are supposed to have support for it in the C++11 driver. Still... how about "Please look for a newer version that supports it" as a way to put it. Doesn't promise, just says to look into it. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 16/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
To what should we prompt them to upgrade? The C++11 driver doesn't yet have decimal... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by David Golden [ 16/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
The Drivers ticket specifically wants drivers to prompt users to upgrade. I think we should do that with a very targeted patch. So we don't do it again for future types, how about we make the message more like "...does not support BSON type %d. Please upgrade..."? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by J Rassi [ 15/Jun/16 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
The BSONObj constructors are happy to create objects where the raw data contains invalid elements, as these constructors do not inspect the data itself. BSONElement::size(), however, will throw a MsgAssertionException with exception string "BSONElement: bad type 19" when called on a BSONElement that has been initialized from a buffer containing a decimal element. In practice, this means that current version of the legacy C++ driver will happily return BSONObjs to clients from methods like DBClientInterface::findOne() / DBClientCursor::next() / DBClientInterface::findAndModify() / etc. that contain decimal values. However, calling any accessor methods on these objects like getField(), operator[](), toString() will cause the above exception to be thrown. Ditto for using a BSONObjIterator to iterate over these objects. For testing, I have created a modified build of the server where all documents returned by query subsystem are given an extra field with a decimal value. See the patch below, applied on top of server commit 9fd6bc9:
I have run the legacy C++ integration test suite against this build, and observed many tests failing due to this exception being thrown, but no signs of memory corruption or other misbehavior. As a result, I'd recommend either closing this ticket without any code changes, or creating a pull request for patch like the below to make some of the error messages more friendly. Thoughts?
|