[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:
Duplicate
duplicates SERVER-30580 Unify implementations of Status and D... Closed
Related
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 SERVER-33037 which is the one we were discussing earlier today. I think the issue this ticket is about was resolved by SERVER-30580, specifically by completely removing the concept of a "location" code.

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.

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