[DRIVERS-2385] Provide access to raw result document when the server returns an error for a command Created: 08/Jul/22  Updated: 02/Jan/24

Status: Implementing
Project: Drivers
Component/s: None
Fix Version/s: None

Type: New Feature Priority: Unknown
Reporter: Kaitlin Mahar Assignee: Jeremy Mikola
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Depends
Issue split
split to CSHARP-4250 Provide access to raw result document... Backlog
split to CXX-2543 Provide access to raw result document... Backlog
split to NODE-4404 Provide access to raw result document... Backlog
split to RUST-1405 Provide access to raw result document... Backlog
split to CDRIVER-4425 Provide access to raw result document... Closed
split to GODRIVER-2487 Provide access to raw result document... Closed
split to JAVA-4676 Provide access to raw result document... Closed
split to MOTOR-992 Provide access to raw result document... Closed
split to RUBY-3049 Provide access to raw result document... Closed
split to PHPLIB-909 Spec tests for expectedError.errorRes... Closed
split to PYTHON-3351 Provide access to raw result document... Closed
Related
related to MONGOSH-1250 [MONGOSH] Incomplete output from coll... Closed
related to DRIVERS-2470 Extra write and write concern error f... Backlog
Driver Changes: Needed
Quarter: FY23Q3
Downstream Changes Summary:

Introduces a new expectedError.errorResponse assertion for matching the full command response attached to an exception. Note that some drivers may need to skip tests for BulkWriteException and WriteException.

Drivers should sync unified tests for Collection Management, CRUD, and Unified Test Runner ("valid") with c04f2ec.

Driver Compliance:
Key Status/Resolution FixVersion
CDRIVER-4425 Fixed 1.24.0
CXX-2543 Backlog
CSHARP-4250 Backlog
GODRIVER-2487 Done 1.11.0
JAVA-4676 Fixed 4.8.0
NODE-4404 Backlog
MOTOR-992 Duplicate
PYTHON-3351 Fixed 4.4
PHPLIB-909 Fixed 1.15.0
RUBY-3049 Fixed 2.19.0
RUST-1405 Backlog
SWIFT-1599 Won't Do

 Description   

Summary

A number of drivers use concrete types to model error responses from the server. As a result, when the server adds a new field to the error response for a command, users cannot actually see the new information.

Rather than having drivers try to exhaustively model every possible field, this ticket proposes that to alleviate issues of missing error information, all drivers must somehow expose the full response document from the server when there is an error. This will future-proof us for cases where more information is added to any error responses in the future. The exact API for this will be up to each driver given the variety of error APIs we have across languages.

There is not a natural place to specify this behavior, but to ensure the requirement is captured somewhere in case e.g. we write a driver in a new language, we also would like to add spec tests for this behavior. This may require an addition to the unified test format to support asserting on the response document.

Motivation

Who is the affected end user?

All driver users.

How does this affect the end user?

The end user is not always able to get access to all of the information the server returns when a command fails.

How likely is it that this problem or use case will occur?

The only known instance of this problem right now occurs with the response to the collMod command, where errors can now have a violations property. This is discussed on DRIVERS-2353. However, the server may add more fields like this to command responses in the future, increasing the likelihood of users running into this.

If the problem does occur, what are the consequences and how severe are they?

Since the user is missing information it is hard/impossible for them to understand the error and take action on it.

Is this issue urgent?

The collMod change above was released in v5.2 of the server so users may already be running into this.

Is this ticket required by a downstream team?

Yes, this prevents mongosh from providing equivalent information in error responses to that the legacy shell provided. See MONGOSH-1250.

Is this ticket only for tests?

No, this ticket includes functional changes. That said, for some drivers such as Python that already expose the error response this will be a test-only change.



 Comments   
Comment by Githook User [ 27/Oct/22 ]

Author:

{'name': 'Jeremy Mikola', 'email': 'jmikola@gmail.com', 'username': 'jmikola'}

Message: DRIVERS-2385: Revert useMultipleMongoses:false in findOneAndUpdate test (#1329)

This was added by mistake in 9fec9df099d132df047650315411bc0a532f0cef
Branch: master
https://github.com/mongodb/specifications/commit/c04f2ec111ec0e5c9cfd46e37a91ea2f3c63f0f5

Comment by Githook User [ 27/Oct/22 ]

Author:

{'name': 'Benjamin Rewis', 'email': '32186188+benjirewis@users.noreply.github.com', 'username': 'benjirewis'}

Message: DRIVERS-2385 useMultipleMongoses false for errorResponse tests (#1327)

Necessary because these tests use configureFailPoint. This was missed in 081578a9d951a81e70e99e165a6c254be88a8959
Branch: master
https://github.com/mongodb/specifications/commit/9fec9df099d132df047650315411bc0a532f0cef

Comment by Githook User [ 14/Oct/22 ]

Author:

{'name': 'Jeremy Mikola', 'email': 'jmikola@gmail.com', 'username': 'jmikola'}

Message: DRIVERS-2385: Skip aggregate-merge-errorResponse test on sharded clusters

This was missed in 081578a9d951a81e70e99e165a6c254be88a8959
Branch: master
https://github.com/mongodb/specifications/commit/5f8d058e5ecbc24f98807713d88527f79581d2e9

Comment by Githook User [ 14/Oct/22 ]

Author:

{'name': 'Jeremy Mikola', 'email': 'jmikola@gmail.com', 'username': 'jmikola'}

Message: DRIVERS-2385: expectedError.errorResponse for asserting server-side error doc (#1316)

Adds basic tests for expectedError.errorResponse under the unified test format, which are derived from the operation-failure tests in valid-fail.

Also adds tests for write operations (i.e. insert, update, delete, and bulkWrite), which may be problematic for some drivers that use special exceptions such as BulkWriteException, which may not provide direct access to a single response. Note that tests should avoid using errorResponse assertions for such operations and permit drivers to skip those tests as needed.
Branch: master
https://github.com/mongodb/specifications/commit/081578a9d951a81e70e99e165a6c254be88a8959

Comment by Jeremy Mikola [ 07/Oct/22 ]

https://github.com/mongodb/specifications/pull/1316

Comment by Jeremy Mikola [ 07/Oct/22 ]

have we considered for future cases standardizing extra error fields server side instead rather than requiring drivers to expose the whole response?

This seems like a very reasonable suggestion for the server product team. rachelle.palmer@mongodb.com: can you escalate this feedback through the proper channels?

Comment by Patrick Freed [ 06/Oct/22 ]

I know the ship has probably already sailed for the collMod case, but have we considered for future cases standardizing extra error fields server side instead rather than requiring drivers to expose the whole response? I'm imagining something like errInfo / details fields in Write/WC errors, except this would be for command errors. Then in drivers, instead of exposing the entire server response, we could add a single new field for uncategorized extra info the server might append.

I think this might be a better API for users, since the relevant error information will be organized in a single location without requiring users to parse (visually or programatically) an entire server response, which generally contain a lot of noisy fields in them (e.g. $clusterTime, operationTime, etc). It would also be more consistent with our existing error API.

Comment by Kaitlin Mahar [ 14/Sep/22 ]

jmikola@mongodb.com Thanks for the reply! That all sounds good to me.

Comment by Jeremy Mikola [ 14/Sep/22 ]

kaitlin.mahar@mongodb.com: runCommand was my thought with that last comment, but reading this thread again I don't understand why I reached that conclusion. Many specs don't define any special exception classes, so there's practically no difference between something like listIndexes or runCommand raising an error. For specs that do define special exception classes (e.g. CRUD, transactions), we might need to make some changes there to ensure server-side errors are available.

When thinking about this issue over the past week, I actually came up with the same idea of a new assertion under expectedError after having completely forgotten that you suggested that in your first comment. That is either a case of inception or an innocent coincidence, but it is nevertheless encouraging that it's likely the best approach.

I was also hard-pressed to think of where we might spec this behavior out; however, adding syntax in the unified test format and then adding tests through all other specs seems like a nice alternative to (a) coming up with a new spec that just talks about errors and (b) littering other specs with copypasta about exposing server-side errors in their own API methods' exceptions. That said, we'll still need some changes to specs with their own exception classes (e.g. CRUD).

Comment by Kaitlin Mahar [ 26/Aug/22 ]

jmikola@mongodb.com, by "generic command execution", do you mean "commands run via runCommand", or something else?

My intent with this ticket was that drivers would do this for all errors returned by the server so all commands are future-proofed against additional fields being added to replies.

Comment by Jeremy Mikola [ 02/Aug/22 ]

From the comments above, I think this ticket only pertains to generic command execution (e.g. collMod).

That said, there may be some relation to DRIVERS-820 where we previously exposed some raw command error info on the bulk write error object (both writeErrors and writeConcernError). If this ticket needs to consider bulk write errors, then we might want to revisit the approach from DRIVERS-820 as well.

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