[SERVER-38038] mongo shell should be linked with ErrorExtraInfo derivatives Created: 08/Nov/18 Updated: 08/Jan/24 Resolved: 23/Jan/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Shell |
| Affects Version/s: | 4.0.0, 4.1.5 |
| Fix Version/s: | 4.0.7, 4.1.8 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | James Wahlin | Assignee: | Mathias Stearn |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||
| Backport Requested: |
v4.0
|
||||||||||||||||||||
| Sprint: | Service Arch 2018-12-03, Service Arch 2018-12-17, Service Arch 2018-12-31, Service Arch 2019-01-14, Service Arch 2019-01-28 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Description |
|
Instantiating a Status object for an error with 'extraInfo' defined requires the given ErrorExtraInfo derivative. As the definitions for these errors are not linked into the mongo shell, receipt of one of these errors will cause shell termination. |
| Comments |
| Comment by Githook User [ 07/Mar/19 ] | |||||
|
Author: {'name': 'Mathias Stearn', 'username': 'RedBeard0531', 'email': 'mathias@10gen.com'}Message: (cherry picked from commit 33c750267b6481c0d0320d75c94ef755f3e467fe) | |||||
| Comment by Githook User [ 23/Jan/19 ] | |||||
|
Author: {'email': 'mathias@10gen.com', 'name': 'Mathias Stearn', 'username': 'RedBeard0531'}Message: | |||||
| Comment by James Wahlin [ 16/Nov/18 ] | |||||
|
I created | |||||
| Comment by Mathias Stearn [ 12/Nov/18 ] | |||||
|
The shell was intended to call invariantHaveAllParsers(), but because we don't want every unit test to depend on every ErrorExtraInfo subclass, we don't want to require that in every executable. Additionally, that was supposed to be a non-fatal error in release builds, but one code path still make it fatal (only in the shell, not the servers). This will be fixed and backported soon. | |||||
| Comment by Andrew Morrow (Inactive) [ 09/Nov/18 ] | |||||
|
Can we find another way of doing it that does not require a manual injection into the startup of every binary? Is there an initializer we can hook it up to? The current approach seems very fragile. In particular, it is clear that the only binaries which actually do this are mongod and mongos, but not for instance mongoed or any of the enterprise tools, etc. | |||||
| Comment by Mira Carey [ 09/Nov/18 ] | |||||
|
I believe the missing bit here is ErrorExtraInfo::invariantHaveAllParsers(); From error_extra_info.h:
| |||||
| Comment by Andy Schwerin [ 09/Nov/18 ] | |||||
|
This seems like it should be supported, and I think the implementation of EEI is betraying us by generating a fatal runtime error here. It should either fail to link or have some kind of default behavior that works safely. redbeard0531 and mira.carey@mongodb.com . | |||||
| Comment by James Wahlin [ 09/Nov/18 ] | |||||
|
The shell invariant triggered is in the generated build/opt/mongo/base/error_codes.cpp ErrorExtraInfo::parserFor() method, which expects the parser to exist:
Do we want to strictly prohibit allowing errors with an 'extraInfo' object from propagating to the shell and drivers? I am considering annotating the DuplicateKey error with extraInfo to as part of | |||||
| Comment by Andy Schwerin [ 08/Nov/18 ] | |||||
|
A return from the network shouldn’t lead to shell termination, max.hirschhorn. redbeard0531, what dark magic prevented a link time error for this? | |||||
| Comment by Andy Schwerin [ 08/Nov/18 ] | |||||
|
A return from the network shouldn’t lead to shell termination, max.hirschhorn. redbeard0531, what dark magic prevented a link time error for this? |