[Tarantool-patches] [PATCH v2 10/10] iproto: support error stacked diagnostic area
Nikita Pettik
korablev at tarantool.org
Thu Apr 2 01:24:56 MSK 2020
On 01 Apr 16:26, Nikita Pettik wrote:
> @@ -1169,7 +1166,7 @@ xrow_decode_error(struct xrow_header *row)
> } else if (key == IPROTO_ERROR_STACK ) {
> if (mp_check_array(pos, end) != 0)
> goto error;
> - if (iproto_decode_error_stack(&pos, end) != 0)
> + if (iproto_decode_error_stack(&pos) != 0)
> continue;
> } else {
> mp_next(&pos);
>
> > I don't see any tests on that. Are there some? Maybe at least
> > unit tests, in xrow.cc? Although a proper test would try to
> > send an error stack through applier. Because xrow_decode_error()
> > is used only there.
>
> I'm a bit new to replication tests. I've been working on it.
> Could you please review the rest of changes so far?
Well, I've come up with following unit tests (and a bit more
fixes of xrow_decode_error()):
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 596c765b7..f33226dc5 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -1094,8 +1094,7 @@ iproto_decode_error_stack(const char **pos)
* box_error_add() does not replace previous errors.
*/
box_error_clear();
- if (mp_typeof(**pos) != MP_ARRAY)
- return -1;
+ assert(mp_typeof(**pos) == MP_ARRAY);
uint32_t stack_sz = mp_decode_array(pos);
for (uint32_t i = 0; i < stack_sz; i++) {
uint32_t code = 0;
@@ -1163,11 +1162,11 @@ xrow_decode_error(struct xrow_header *row)
const char *str = mp_decode_str(&pos, &len);
snprintf(error, sizeof(error), "%.*s", len, str);
box_error_set(__FILE__, __LINE__, code, error);
- } else if (key == IPROTO_ERROR_STACK ) {
- if (mp_check_array(pos, end) != 0)
+ } else if (key == IPROTO_ERROR_STACK) {
+ if (mp_typeof(*pos) != MP_ARRAY)
goto error;
if (iproto_decode_error_stack(&pos) != 0)
- continue;
+ goto error;
} else {
mp_next(&pos);
continue;
diff --git a/test/unit/xrow.cc b/test/unit/xrow.cc
index 68a334239..954f22f16 100644
--- a/test/unit/xrow.cc
+++ b/test/unit/xrow.cc
@@ -241,6 +241,140 @@ test_xrow_header_encode_decode()
check_plan();
}
+static char *
+error_stack_entry_encode(char *pos, const char *err_str)
+{
+ pos = mp_encode_map(pos, 2);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+ pos = mp_encode_uint(pos, 159);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+ pos = mp_encode_str(pos, err_str, strlen(err_str));
+ return pos;
+}
+
+void
+test_xrow_error_stack_decode()
+{
+ plan(14);
+ char buffer[2048];
+ /*
+ * To start with, let's test the simplest and obsolete
+ * way of setting errors.
+ */
+ char *pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR);
+ pos = mp_encode_str(pos, "e1", strlen("e1"));
+
+ struct xrow_header header;
+ header.bodycnt = 666;
+ header.type = 159 | IPROTO_TYPE_ERROR;
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ xrow_decode_error(&header);
+ struct error *last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, "e1"), 0,
+ "xrow_decode succeed: error is parsed");
+
+ pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+ pos = mp_encode_array(pos, 2);
+ pos = error_stack_entry_encode(pos, "e1");
+ pos = error_stack_entry_encode(pos, "e2");
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ diag_clear(diag_get());
+ xrow_decode_error(&header);
+ last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, "e2"), 0, "xrow_decode error stack suceed: "
+ "e2 at the top of stack");
+ last = last->cause;
+ isnt(last, NULL, "xrow_decode succeed: 'cause' is present in stack")
+ is(strcmp(last->errmsg, "e1"), 0, "xrow_decode succeed: "
+ "stack has been parsed");
+ /*
+ * Let's try decode broken stack. Variations:
+ * 1. Stack is not encoded as an array;
+ * 2. Stack doesn't contain maps;
+ * 3. Stack's map keys are not uints;
+ * 4. Stack's map values have wrong types;
+ * In all these cases diag_last should contain emty err.
+ */
+ /* Stack is encoded as map. */
+ pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+ pos = mp_encode_map(pos, 1);
+ pos = error_stack_entry_encode(pos, "e1");
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ diag_clear(diag_get());
+ xrow_decode_error(&header);
+ last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+ "stack contains map instead of array");
+
+ /* Stack doesn't containt map(s) - array instead. */
+ pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+ pos = mp_encode_array(pos, 2);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ diag_clear(diag_get());
+ xrow_decode_error(&header);
+ last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+ "stack contains array values instead of maps");
+
+ /* Stack's map keys are not uints. */
+ pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+ pos = mp_encode_array(pos, 1);
+ pos = mp_encode_map(pos, 2);
+ pos = mp_encode_str(pos, "string instead of uint",
+ strlen("string instead of uint"));
+ pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+ pos = mp_encode_uint(pos, 159);
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ diag_clear(diag_get());
+ xrow_decode_error(&header);
+ last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+ "stack's map keys are not uints");
+
+ /* Stack's map values have wrong types. */
+ pos = mp_encode_map(buffer, 1);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+ pos = mp_encode_array(pos, 1);
+ pos = mp_encode_map(pos, 2);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+ pos = mp_encode_uint(pos, 159);
+ pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+ pos = mp_encode_uint(pos, 666);
+ header.body[0].iov_base = buffer;
+ header.body[0].iov_len = pos - buffer;
+
+ diag_clear(diag_get());
+ xrow_decode_error(&header);
+ last = diag_last_error(diag_get());
+ isnt(last, NULL, "xrow_decode succeed: diag has been set");
+ is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+ "stack's map wrong value type");
+ check_plan();
+}
+
void
test_request_str()
{
@@ -279,13 +413,14 @@ main(void)
{
memory_init();
fiber_init(fiber_c_invoke);
- plan(3);
+ plan(4);
random_init();
test_iproto_constants();
test_greeting();
test_xrow_header_encode_decode();
+ test_xrow_error_stack_decode();
test_request_str();
random_free();
diff --git a/test/unit/xrow.result b/test/unit/xrow.result
index 5ee92ad7b..1d07915a4 100644
--- a/test/unit/xrow.result
+++ b/test/unit/xrow.result
@@ -1,4 +1,4 @@
-1..3
+1..4
1..40
ok 1 - round trip
ok 2 - roundtrip.version_id
@@ -53,6 +53,22 @@ ok 1 - subtests
ok 9 - decoded sync
ok 10 - decoded bodycnt
ok 2 - subtests
+ 1..14
+ ok 1 - xrow_decode failed: no diag has been set
+ ok 2 - xrow_decode failed: wrong error is set
+ ok 3 - xrow_decode error stack failed: no diag has been set
+ ok 4 - xrow_decode error stack failed: e2 should come first
+ ok 5 - xrow_decode error stack failed: 'cause' is absent
+ ok 6 - xrow_decode error stack failed: wrong error is set
+ ok 7 - xrow_decode error stack failed: no diag has been set
+ ok 8 - xrow_decode corrupted stack: stack contains map instead of array
+ ok 9 - xrow_decode error stack failed: no diag has been set
+ ok 10 - xrow_decode corrupted stack: stack contains array values instead of maps
+ ok 11 - xrow_decode error stack failed: no diag has been set
+ ok 12 - xrow_decode corrupted stack: stack's map keys are not uints
+ ok 13 - xrow_decode error stack failed: no diag has been set
+ ok 14 - xrow_decode corrupted stack: stack's map wrong value type
+ok 3 - subtests
1..1
ok 1 - request_str
-ok 3 - subtests
+ok 4 - subtests
More information about the Tarantool-patches
mailing list