Merge pull request #4465 from telefonicaid/hardening/orionerror-class…

…-refactor

FIX OrionError field names refactor

CHANGES_NEXT_RELEASE

@@ -11,6 +11,7 @@
 - Fix: provide more informative error description in some error responses in update/delete operations
 - Fix: proper use of "PartialUpdate" (instead of "Unprocessed") in responses in the case of partial updates/deletions (#3499)
 - Fix: response 404 Not Found "NotFound" errors instead of 422 Unprocessable Content "Unprocessed" in the case of missing attribute in existing entity in attribute update operations
+- Fix: "Internal Server Error" changed to "InternalServerError" in error responses
 - Add: CLI parameter -dbUri / env var ORION_MONGO_URI (#3794)
 - Fix: improve logs in MongoDB query logic
 - Fix: false positive in log deprecation logic when entity name (or other literal) includes the token "v1" (#4454)

doc/manuals/admin/diagnosis.md

@@ -271,45 +271,20 @@ The symptoms of a database connection problem are the following:
 -   At startup time. The broker doesn't start and the following message
     appears in the log file:
 
-` X@08:04:45 main[313]: MongoDB error`
+```
+... msg=Database Startup Error (cannot connect to mongo - doing 100 retries with a 1000 millisecond interval)
+... msg=Fatal Error (MongoDB error)
+
+```
 
 -   During broker operation. Error message like the following ones
     appear in the responses sent by the broker.
 
 ```
-
-    ...
-    "errorCode": {
-        "code": "500",
-        "reasonPhrase": "Database Error",
-        "details": "collection: ... - exception: Null cursor"
-    }
-    ...
-
-    ...
-    "errorCode": {
-        "code": "500",
-        "reasonPhrase": "Database Error",
-        "details": "collection: ... - exception: socket exception [CONNECT_ERROR] for localhost:27017"
-    }
-    ...
-
-    ...
-    "errorCode": {
-        "code": "500",
-        "reasonPhrase": "Database Error",
-        "details": "collection: ... - exception: socket exception [FAILED_STATE] for localhost:27017"
-    }
-    ...
-
-    ...
-    "errorCode": {
-        "code": "500",
-        "reasonPhrase": "Database Error",
-        "details": "collection: ... - exception: DBClientBase::findN: transport error: localhost:27017 ns: orion.$cmd query: { .. }"
-    }
-    ...
-
+{
+    "error": "InternalServerError",
+    "description": "Database Error ..."
+}
 ```
 
 In both cases, check that the connection to MonogDB is correctly

doc/manuals/orion-api.md

@@ -458,6 +458,7 @@ The `error` reporting is as follows:
   + HTTP 411 Length Required corresponds to `ContentLengthRequired` (`411`)
   + HTTP 413 Request Entity Too Large corresponds to `RequestEntityTooLarge` (`413`)
   + HTTP 415 Unsupported Media Type corresponds to `UnsupportedMediaType` (`415`)
++ Internal errors use `InternalServerError` (`500`)
 
 ## Multi tenancy
 

doc/manuals/user/known_limitations.md

@@ -7,11 +7,8 @@ this limitation into account, you will get messages such the following ones:
 
 ```
 {
-  "errorCode" : {
-    "code" : "413",
-    "reasonPhrase" : "Request Entity Too Large",
-    "details" : "payload size: 1500000, max size supported: 1048576"
-  }
+    "error": "RequestEntityTooLarge"
+    "description": "payload size: 1500000, max size supported: 1048576",
 }
 ```
 

src/lib/jsonParseV2/parseNotification.cpp

@@ -212,7 +212,7 @@ static bool parseNotificationNormalized(ConnectionInfo* ciP, NotifyContextReques
 
       if (parseNotificationData(ciP, iter, ncrP, oeP) == false)
       {
-        alarmMgr.badInput(clientIp, oeP->details);
+        alarmMgr.badInput(clientIp, oeP->description);
         ciP->httpStatusCode = SccBadRequest;
 
         return false;

src/lib/jsonParseV2/parseRegistration.cpp

@@ -58,8 +58,8 @@ static bool dataProvidedParse
   {
     ciP->httpStatusCode = SccBadRequest;
     oeP->code           = SccBadRequest;
-    oeP->reasonPhrase   = ERROR_BAD_REQUEST;
-    oeP->details        = "/dataProvided/ must be a JSON object";
+    oeP->error          = ERROR_BAD_REQUEST;
+    oeP->description    = "/dataProvided/ must be a JSON object";
 
     return false;
   }
@@ -68,20 +68,20 @@ static bool dataProvidedParse
   {
     ciP->httpStatusCode = SccBadRequest;
     oeP->code           = SccBadRequest;
-    oeP->reasonPhrase   = ERROR_BAD_REQUEST;
-    oeP->details        = "/dataProvided/ is empty";
+    oeP->error          = ERROR_BAD_REQUEST;
+    oeP->description    = "/dataProvided/ is empty";
 
     return false;
   }
 
   if (dataProvided.HasMember("entities"))
   {
-    bool b = parseEntitiesVector(ciP, &dataProvidedP->entities, dataProvided["entities"], &oeP->details);
+    bool b = parseEntitiesVector(ciP, &dataProvidedP->entities, dataProvided["entities"], &oeP->description);
     if (b == false)
     {
       ciP->httpStatusCode = SccBadRequest;
       oeP->code           = SccBadRequest;
-      oeP->reasonPhrase   = ERROR_BAD_REQUEST;
+      oeP->error          = ERROR_BAD_REQUEST;
 
       return false;
     }
@@ -99,8 +99,8 @@ static bool dataProvidedParse
     {
       ciP->httpStatusCode = SccNotImplemented;
       oeP->code           = SccNotImplemented;
-      oeP->reasonPhrase   = ERROR_NOTIMPLEMENTED;
-      oeP->details        = ERROR_DESC_IDPATTERN_NOTSUPPORTED;
+      oeP->error          = ERROR_NOTIMPLEMENTED;
+      oeP->description    = ERROR_DESC_IDPATTERN_NOTSUPPORTED;
 
       return false;
     }
@@ -109,20 +109,20 @@ static bool dataProvidedParse
     {
       ciP->httpStatusCode = SccNotImplemented;
       oeP->code           = SccNotImplemented;
-      oeP->reasonPhrase   = ERROR_NOTIMPLEMENTED;
-      oeP->details        = ERROR_DESC_TYPEPATTERN_NOTIMPLEMENTED;
+      oeP->error          = ERROR_NOTIMPLEMENTED;
+      oeP->description    = ERROR_DESC_TYPEPATTERN_NOTIMPLEMENTED;
 
       return false;
     }
   }
 
   if (dataProvided.HasMember("attrs"))
   {
-    if (!parseStringVector(&dataProvidedP->attributes, dataProvided["attrs"], "attrs", true, true, &oeP->details))
+    if (!parseStringVector(&dataProvidedP->attributes, dataProvided["attrs"], "attrs", true, true, &oeP->description))
     {
       ciP->httpStatusCode = SccBadRequest;
       oeP->code           = SccBadRequest;
-      oeP->reasonPhrase   = ERROR_BAD_REQUEST;
+      oeP->error          = ERROR_BAD_REQUEST;
 
       return false;
     }

src/lib/mongoBackend/mongoSubscribeContext.cpp

@@ -70,7 +70,7 @@ HttpStatusCode mongoSubscribeContext
   else
   {
     // Check OrionError
-    responseP->subscribeError.errorCode.fill(oe.code, oe.details);
+    responseP->subscribeError.errorCode.fill(oe.code, oe.description);
   }
 
   // free sub memory associated to subscription

src/lib/mongoBackend/mongoUpdateContext.cpp

@@ -221,8 +221,8 @@ HttpStatusCode mongoUpdateContext
     // Other cases follow the usual response processing flow (whatever it is :)
     if (updateCoverage == UC_PARTIAL)
     {
-      responseP->oe.code         = SccInvalidModification;
-      responseP->oe.reasonPhrase = ERROR_PARTIAL_UPDATE;
+      responseP->oe.code  = SccInvalidModification;
+      responseP->oe.error = ERROR_PARTIAL_UPDATE;
     }
 
     LM_T(LmtNotifier, ("total notifications sent during update: %d", notifSent));

src/lib/mongoBackend/mongoUpdateContextSubscription.cpp

@@ -83,7 +83,7 @@ HttpStatusCode mongoUpdateContextSubscription
     }
     else
     {
-      responseP->subscribeError.errorCode.fill(oe.code, oe.details);
+      responseP->subscribeError.errorCode.fill(oe.code, oe.description);
     }
   }
 

src/lib/rest/OrionError.cpp

@@ -39,10 +39,10 @@
 */
 OrionError::OrionError()
 {
-  code         = SccNone;
-  reasonPhrase = "";
-  details      = "";
-  filled       = false;
+  code        = SccNone;
+  error       = "";
+  description = "";
+  filled      = false;
 }
 
 
@@ -51,12 +51,12 @@ OrionError::OrionError()
 *
 * OrionError::OrionError -
 */
-OrionError::OrionError(HttpStatusCode _code, const std::string& _details, const std::string& _reasonPhrase)
+OrionError::OrionError(HttpStatusCode _code, const std::string& _description, const std::string& _error)
 {
-  code          = _code;
-  reasonPhrase  = _reasonPhrase.empty() ? httpStatusCodeString(code) : _reasonPhrase;
-  details       = _details;
-  filled        = true;
+  code        = _code;
+  error       = _error.empty() ? httpStatusCodeString(code) : _error;
+  description = _description;
+  filled      = true;
 }
 
 
@@ -67,10 +67,10 @@ OrionError::OrionError(HttpStatusCode _code, const std::string& _details, const
 */
 OrionError::OrionError(StatusCode& sc)
 {
-  code          = sc.code;
-  reasonPhrase  = httpStatusCodeString(code);
-  details       = sc.details;
-  filled        = true;
+  code        = sc.code;
+  error       = httpStatusCodeString(code);
+  description = sc.details;
+  filled      = true;
 }
 
 
@@ -79,12 +79,12 @@ OrionError::OrionError(StatusCode& sc)
 *
 * OrionError::fill -
 */
-void OrionError::fill(HttpStatusCode _code, const std::string& _details, const std::string& _reasonPhrase)
+void OrionError::fill(HttpStatusCode _code, const std::string& _description, const std::string& _error)
 {
-  code          = _code;
-  reasonPhrase  = _reasonPhrase.empty()? httpStatusCodeString(code) : _reasonPhrase;
-  details       = _details;
-  filled        = true;
+  code        = _code;
+  error       = _error.empty()? httpStatusCodeString(code) : _error;
+  description = _description;
+  filled      = true;
 }
 
 
@@ -95,10 +95,10 @@ void OrionError::fill(HttpStatusCode _code, const std::string& _details, const s
 */
 void OrionError::fill(const StatusCode& sc)
 {
-  code          = sc.code;
-  reasonPhrase  = (sc.reasonPhrase.empty())? httpStatusCodeString(code) : sc.reasonPhrase;
-  details       = sc.details;
-  filled        = true;
+  code        = sc.code;
+  error       = (sc.reasonPhrase.empty())? httpStatusCodeString(code) : sc.reasonPhrase;
+  description = sc.details;
+  filled      = true;
 }
 
 
@@ -107,19 +107,19 @@ void OrionError::fill(const StatusCode& sc)
 *
 * OrionError::fillOrAppend -
 */
-void OrionError::fillOrAppend(HttpStatusCode _code, const std::string& fullDetails, const std::string& appendDetail, const std::string& _reasonPhrase)
+void OrionError::fillOrAppend(HttpStatusCode _code, const std::string& fullDetails, const std::string& appendDetail, const std::string& _error)
 {
   if (filled)
   {
     // Already filled by a previous operation. This can happen in batch update processing
-    details += appendDetail;
+    description += appendDetail;
   }
   else
   {
-    code          = _code;
-    reasonPhrase  = _reasonPhrase.empty()? httpStatusCodeString(code) : _reasonPhrase;
-    details       = fullDetails;
-    filled        = true;
+    code        = _code;
+    error       = _error.empty()? httpStatusCodeString(code) : _error;
+    description = fullDetails;
+    filled      = true;
   }
 }
 
@@ -137,7 +137,7 @@ std::string OrionError::smartRender(ApiVersion apiVersion)
   }
   else // admin or v2
   {
-    shrinkReasonPhrase();
+    shrinkError();
     return toJson();
   }
 }
@@ -166,8 +166,8 @@ std::string OrionError::setStatusCodeAndSmartRender(ApiVersion apiVersion, HttpS
 */
 std::string OrionError::toJson(void)
 {
-  char*  reasonPhraseEscaped = htmlEscape(reasonPhrase.c_str());
-  char*  detailsEscaped      = htmlEscape(details.c_str());
+  char*  reasonPhraseEscaped = htmlEscape(error.c_str());
+  char*  detailsEscaped      = htmlEscape(description.c_str());
 
   JsonObjectHelper jh;
 
@@ -194,13 +194,13 @@ std::string OrionError::toJsonV1(void)
   //
   // OrionError is NEVER part of any other payload, so the JSON start/end braces must be added here
   //
-  out += startTag("orionError", false);
-  out += valueTag("code",          code,         true);
-  out += valueTag("reasonPhrase",  reasonPhrase, !details.empty());
+  out += startTag("orionError",   false);
+  out += valueTag("code",         code, true);
+  out += valueTag("reasonPhrase", error, !description.empty());
 
-  if (!details.empty())
+  if (!description.empty())
   {
-    out += valueTag("details",       details);
+    out += valueTag("details",    description);
   }
 
   out += endTag();
@@ -214,9 +214,9 @@ std::string OrionError::toJsonV1(void)
 
 /* ****************************************************************************
 *
-* OrionError::shrinkReasonPhrase -
+* OrionError::shrinkError -
 *
-* This method removes any whitespace in the reasonPhrase field, i.e.
+* This method removes any whitespace in the error field, i.e.
 * transforms "Not Found" to "NotFound".
 *
 * It is used by smartRender method, in order to prepare to render in API v2 case
@@ -228,19 +228,19 @@ std::string OrionError::toJsonV1(void)
 *
 *  ...
 *
-*  reasonPhrase.erase(std::remove_if(reasonPhrase.begin(), reasonPhrase.end(), std::isspace), reasonPhrase.end());
+*  reasonPhrase.erase(std::remove_if(error.begin(), error.end(), std::isspace), error.end());
 *
 * However, 'std::isspace' doesn't directly work. We have been able to make it work with
 * 'static_cast<int(*)(int)>(isspace)'. However, that is obscure so until we can find
 * a way of using just 'std::isspace', the current implementation stills.
 *
 */
-void OrionError::shrinkReasonPhrase(void)
+void OrionError::shrinkError(void)
 {
   char buf[80];  // 80 should be enough to hold any reason phrase
 
 #if 0
-  strncpy(buf, reasonPhrase.c_str(), sizeof(buf));
+  strncpy(buf, error.c_str(), sizeof(buf));
 
   // See: http://stackoverflow.com/questions/1726302/removing-spaces-from-a-string-in-c
   if (*j != ' ')
@@ -263,7 +263,7 @@ void OrionError::shrinkReasonPhrase(void)
   *i = 0;
 #endif
 
-  char*         fromP = (char*) reasonPhrase.c_str();
+  char*         fromP = (char*) error.c_str();
   char*         toP   = buf;
   unsigned int  toLen = 0;
 
@@ -281,5 +281,5 @@ void OrionError::shrinkReasonPhrase(void)
   }
   *toP = 0;  // End-of string
 
-  reasonPhrase = std::string(buf);
+  error = std::string(buf);
 }