[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:
Backports
Depends
is depended on by SERVER-37124 Retry full upsert path when duplicate... Closed
Related
related to SERVER-38175 Invariant on error codes with ErrorEx... Closed
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: SERVER-38038 mongo shell should support all ErrorExtraInfo codes

(cherry picked from commit 33c750267b6481c0d0320d75c94ef755f3e467fe)
Branch: v4.0
https://github.com/mongodb/mongo/commit/470c14ed7543d62a2a76476e6522823316ec36d3

Comment by Githook User [ 23/Jan/19 ]

Author:

{'email': 'mathias@10gen.com', 'name': 'Mathias Stearn', 'username': 'RedBeard0531'}

Message: SERVER-38038 mongo shell should support all ErrorExtraInfo codes
Branch: master
https://github.com/mongodb/mongo/commit/33c750267b6481c0d0320d75c94ef755f3e467fe

Comment by James Wahlin [ 16/Nov/18 ]

I created SERVER-38175 to report another issue blocking ErrorExtraInfo error codes from being first class citizens in the Server. Currently OperationContext will store error codes when marked killed, or for return on timeout. These error codes may be used to return Status objects. If an error code is stored that has ErrorExtraInfo, Status creation will fail. 

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:

/**
 * Fails fatally if any error codes that should have parsers registered don't. Call this during
 * startup of any shipping executable to prevent failures at runtime.
 */
static void invariantHaveAllParsers();

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:

invariant(parsers::CommandOnShardedViewNotSupportedOnMongod);

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 SERVER-37124, to allow for upsert retry in limited cases. The alternative to this would be to replace the storage-engine generated error to an internal-only error code and then to catch and rethow as DuplicateKey. The concern with this approach is that we would the potential to leak the new error code to users who expect DuplicateKey.

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?

Generated at Thu Feb 08 04:47:47 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.