Document common error of replacing the json object

Prior to having query params and aggregation supported, this wasn't much
of an issue, but now that we have these features, we need to code
against doing multiple things in parallel.  We have had cases in the
past prior to these new features, but these were generally localized to
a single handler, so the fixes tended to go under the radar.

This commit documents this common pitfall, and propose a solution.  The
commit below has already done most of the work to make the codebase
consistent in this regard.  Documenting this will hopefully ensure that
regressions don't happen again, or if they do, we have documentation
that we can point to.

1476687d Remove brace initialization of json objects

Tested: Documentation only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id9e761987fd90955218fbb232b277a08b0227339

1 files changed, 29 insertions, 0 deletions

diff --git a/COMMON_ERRORS.md b/COMMON_ERRORS.md
index 6bb5c05155..8bb1288291 100644
--- a/COMMON_ERRORS.md
+++ b/COMMON_ERRORS.md
@@ -279,3 +279,32 @@ especially when relying on user-driven data.
 The above methods tend to be misused to accept user data and parse various
 fields from it.  In practice, there tends to be better, purpose built methods
 for removing just the field you need.
+
+### 13. Complete replacement of the response object
+
+```
+void getMembers(crow::Response& res){
+  res.jsonValue = {{"Value", 2}};
+}
+```
+
+In many cases, bmcweb is doing multiple async actions in parallel, and
+orthogonal keys within the Response object might be filled in from another
+task.  Completely replacing the json object can lead to convoluted situations
+where the output of the response is dependent on the _order_ of the asynchronous
+actions completing, which cannot be guaranteed, and has many time caused bugs.
+
+```
+void getMembers(crow::Response& res){
+  res.jsonValue["Value"] = 2;
+}
+```
+
+As an added benefit, this code is also more efficient, as it avoids the
+intermediate object construction and the move, and as a result, produces smaller
+binaries.
+
+Note, another form of this error involves calling nlohmann::json::reset(), to
+clear an object that's already been filled in.  This has the potential to clear
+correct data that was already filled in from other sources.
+