[SERVER-34628] Remove appendCommandStatus() in favor of throwing exceptions Created: 23/Apr/18 Updated: 29/Oct/23 Resolved: 09/May/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 4.0.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Siyuan Zhou | 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: | Minor Change | ||||||||
| Sprint: | Platforms 2018-05-07, Platforms 2018-05-21 | ||||||||
| Participants: | |||||||||
| Description |
|
Now we have two ways to return an error to clients: appendCommandStatus() and throwing exceptions, e.g. uassertStatusOK(). It'll be great if throwing exceptions is the only way to do so.
|
| Comments |
| Comment by Mathias Stearn [ 09/May/18 ] |
|
While our tests showed that there was no change in the error replies we look at, it is possible that that this change causes some commands that accidentally left information in the result builder to no longer leak that (presumably incorrect) information any more. |
| Comment by Githook User [ 08/May/18 ] |
|
Author: {'email': 'mathias@10gen.com', 'name': 'Mathias Stearn', 'username': 'RedBeard0531'}Message: All remaining callers are transitioned to some form of usassert. This was done |
| Comment by Githook User [ 08/May/18 ] |
|
Author: {'email': 'mathias@10gen.com', 'name': 'Mathias Stearn', 'username': 'RedBeard0531'}Message: |
| Comment by Githook User [ 08/May/18 ] |
|
Author: {'email': 'mathias@10gen.com', 'name': 'Mathias Stearn', 'username': 'RedBeard0531'}Message:
A following commit will completely remove appendCommandStatus. It is split out |
| Comment by Spencer Brody (Inactive) [ 24/Apr/18 ] |
|
The key thing here is to have a single error-handling path where code that needs to run when the command failed can run, without having to remember to duplicate that code in two places - once in the exception handling block and once by manually inspecting the responseBuilder. |
| Comment by Kaloian Manassiev [ 24/Apr/18 ] |
|
From what I understand, appendCommandStatus just appends the error to an already populated response buffer, whereas uassert will reset it and only populate it with the assertion error. Thus removing appendCommandStatus removes the ability to do the former, but perhaps with the new means to specify serialization format for an exception's data, if there is a need to attach side information to an exception we can use that mechanism. CC redbeard0531 |