[SERVER-30279] uassertStatusOK() and appendCommandStatus behave differently Created: 24/Jul/17 Updated: 12/Feb/18 Resolved: 12/Feb/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Charlie Swanson | Assignee: | Mathias Stearn |
| Resolution: | Duplicate | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Operating System: | ALL | ||||||||||||
| Sprint: | Platforms 2018-02-26 | ||||||||||||
| Participants: | |||||||||||||
| Description |
|
uassertStatusOK() expands to calling uassertedWithLocation() with the location code (if one exists), whereas appendCommandStatus() just always uses the error code on the status. I don't think this was intentional, and it is pretty surprising that changing a return of a bad status to be a uassertStatusOK() can result in a different error code being returned to the user. |
| Comments |
| Comment by Mathias Stearn [ 12/Feb/18 ] |
|
billy.donahue Sorry, I confused this ticket with |
| Comment by Billy Donahue [ 12/Feb/18 ] |
|
I don't understand the complaint. "uassertStatusOK() expands to calling uassertedWithLocation() with the location code (if one exists), whereas appendCommandStatus() just always uses the error code on the status. I don't think this was intentional, and it is pretty surprising that changing a return of a bad status to be a uassertStatusOK() can result in a different error code being returned to the user." I agree with Andy. The uassertStatusOK was given a Status object. It feels like the right behavior to throw that Status object as an exception should that assertion fail. If a programmer changes from uassertStatusOK to appendCommandStatus they're signing up for a different meaning for the Status error code, and they've removed a location code from the line of code, leaving us nothing to use as a location code. One way to solve this bug might be to introduce appendCommandStatusWithLocation, but I don't know if we want that. |
| Comment by Andrew Morrow (Inactive) [ 28/Jul/17 ] |
|
Mathias, assigning to you as this seems to be part of PM-864 |
| Comment by Andy Schwerin [ 24/Jul/17 ] |
|
That is interesting. I propose the correct behavior is to always use the error code. |