Skip to content

Commit f23c186

Browse files
committed
[BUGFIX] Compare the status instead of extracting it
IsAcknowledgedStatus is noexcept and reached get<T>() through a type check, so the guard was what kept the promise rather than the code. clang-tidy reads it the way it is written and reports bugprone-exception-escape, which puts the abiv1-preview preset one warning over its limit of 133. Comparing the json value directly holds the same answers without an extraction to guard: 200 and 201 in either integer type are accepted, 2^32 + 200 is not, and a float is still rejected by the type check. The two IsAppliedStatus overloads existed to keep the unsigned case out of a signed comparison, and the comparison does that itself, so they go with it. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 2b5e8ce commit f23c186

1 file changed

Lines changed: 6 additions & 34 deletions

File tree

  • exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail

‎exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -30,44 +30,16 @@ inline bool IsSuccessfulBulkStatus(int status_code) noexcept
3030
}
3131

3232
/**
33-
* What an acknowledged index operation has to carry. Elasticsearch answers one with 201 when it
34-
* created the document and 200 when it replaced an existing one.
33+
* Whether an acknowledged operation status is one that applied the operation. Elasticsearch
34+
* answers 201 when it created the document and 200 when it replaced an existing one.
3535
*
36-
* Two overloads rather than one signed parameter, because an operation status arrives through
37-
* nlohmann::json and is compared in the type it was parsed as: is_number_integer() is true for
38-
* unsigned as well, and 2^32 + 200 narrows back to 200 on a 32 bit int.
39-
*/
40-
inline bool IsAppliedStatus(nlohmann::json::number_unsigned_t value) noexcept
41-
{
42-
return 200U == value || 201U == value;
43-
}
44-
45-
inline bool IsAppliedStatus(nlohmann::json::number_integer_t value) noexcept
46-
{
47-
return 200 == value || 201 == value;
48-
}
49-
50-
/**
51-
* Whether an acknowledged operation status is one that applied the operation.
52-
*
53-
* The type check is here rather than left to the caller, although the one caller does it too.
54-
* This is noexcept, and get() on a value that is not a number throws, so a caller that forgot
55-
* would not get a wrong answer, it would get std::terminate. A float does not throw: it
56-
* truncates, so 200.5 would answer for 200.
36+
* Comparing the json value rather than extracting one keeps this noexcept without relying on the
37+
* type check to make an extraction safe. The check still has to be here: without it a float 200.5
38+
* would answer for 200.
5739
*/
5840
inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
5941
{
60-
if (!status.is_number_integer())
61-
{
62-
return false;
63-
}
64-
65-
if (status.is_number_unsigned())
66-
{
67-
return IsAppliedStatus(status.get<nlohmann::json::number_unsigned_t>());
68-
}
69-
70-
return IsAppliedStatus(status.get<nlohmann::json::number_integer_t>());
42+
return status.is_number_integer() && (status == 200 || status == 201);
7143
}
7244

7345
/**

0 commit comments

Comments
 (0)