[CLIENT-3994] Improve error actionability by adding subcode field to AerospikeError

aerospike-stubs/exception.pyi

@@ -3,6 +3,7 @@ from typing import Union
 class AerospikeError(Exception):
     # When attributes are first assigned to exception class, they have an initial value of None
     code: Union[int, None]
+    subcode: Union[int, None]
     msg: Union[str, None]
     file: Union[str, None]
     line: Union[int, None]

doc/client.rst

@@ -1751,6 +1751,17 @@ Base Policies
 
             Default: :py:obj:`None`
 
+        * **error_detail_verbosity** (:class:`int`)
+
+            Request server error detail fields in responses.
+
+            - ``0``: disabled (no error details returned). Default.
+            - ``1``: return subcode only.
+            - ``2``: return subcode and human-readable message.
+
+            When enabled and the server returns error details, :py:attr:`aerospike.exception.AerospikeError.subcode` will contain the
+            numeric subcode and :py:attr:`aerospike.exception.AerospikeError.msg` will contain the server-authored message.
+
 .. _aerospike_write_policies:
 
 Write Policies

doc/exception.rst

@@ -56,6 +56,13 @@ Base Class
 
         ``True`` if it is possible that the command succeeded. See :ref:`indoubt`.
 
+    .. py:attribute:: subcode
+
+        Server error detail subcode. When ``error_detail_verbosity`` is greater than or equal to ``1`` on the command's
+        base policy and the server returns structured error details, this field contains the numeric subcode.
+
+        Set to ``0`` when no subcode was returned.
+
     In addition to accessing these attributes by their names, \
     they can also be checked by calling ``exc.args[i]``, where ``exc`` is the exception object and \
     ``i`` is the index of the attribute in the order they appear above. \

doc/spelling_wordlist.txt

@@ -103,4 +103,5 @@ lowercases
 lowercased
 msgpack
 precomposed
+subcode
 DBCS

src/include/conversions.h

@@ -127,7 +127,7 @@ as_status metadata_to_pyobject(as_error *err, const as_record *rec,
 as_status bins_to_pyobject(AerospikeClient *self, as_error *err,
                            const as_record *rec, PyObject **obj);
 
-void error_to_pyobject(const as_error *err, PyObject **obj);
+void create_py_tuple_from_as_error(const as_error *err, PyObject **obj);
 
 as_status as_privilege_to_pyobject(as_error *err, as_privilege privileges[],
                                    PyObject *py_as_privilege,

src/main/aerospike.c

@@ -703,7 +703,8 @@ DEFINE_SET_OF_VALID_KEYS(client_config_tls, "enable", "cafile", "capath",
 
 #define BASE_POLICY_KEYS                                                       \
     "total_timeout", "socket_timeout", "max_retries", "sleep_between_retries", \
-        "compress", "txn", "expressions", "connect_timeout", "timeout_delay"
+        "compress", "txn", "expressions", "connect_timeout", "timeout_delay",  \
+        "error_detail_verbosity"
 
 DEFINE_SET_OF_VALID_KEYS(apply_policy, BASE_POLICY_KEYS, "key", "replica",
                          "commit_level", "durable_delete", "ttl",

src/main/conversions.c

@@ -54,11 +54,14 @@
 #define PY_KEYT_KEY 2
 #define PY_KEYT_DIGEST 3
 
-#define PY_EXCEPTION_CODE 0
-#define PY_EXCEPTION_MSG 1
-#define PY_EXCEPTION_FILE 2
-#define PY_EXCEPTION_LINE 3
-#define AS_PY_EXCEPTION_IN_DOUBT 4
+enum {
+    PY_EXCEPTION_CODE = 0,
+    PY_EXCEPTION_MSG,
+    PY_EXCEPTION_FILE,
+    PY_EXCEPTION_LINE,
+    AS_PY_EXCEPTION_IN_DOUBT,
+    EXCEPTION_TUPLE_MEMBER_COUNT
+};
 
 #define CTX_KEY "ctx"
 #define CDT_CTX_ORDER_KEY "order_key"
@@ -2260,7 +2263,7 @@ as_status metadata_to_pyobject(as_error *err, const as_record *rec,
     return err->code;
 }
 
-void error_to_pyobject(const as_error *err, PyObject **obj)
+void create_py_tuple_from_as_error(const as_error *err, PyObject **obj)
 {
     PyObject *py_file = NULL;
     if (err->file) {
@@ -2285,7 +2288,7 @@ void error_to_pyobject(const as_error *err, PyObject **obj)
     PyObject *py_in_doubt = err->in_doubt ? Py_True : Py_False;
     Py_INCREF(py_in_doubt);
 
-    PyObject *py_err = PyTuple_New(5);
+    PyObject *py_err = PyTuple_New(EXCEPTION_TUPLE_MEMBER_COUNT);
     PyTuple_SetItem(py_err, PY_EXCEPTION_CODE, py_code);
     PyTuple_SetItem(py_err, PY_EXCEPTION_MSG, py_message);
     PyTuple_SetItem(py_err, PY_EXCEPTION_FILE, py_file);

src/main/exception.c

@@ -68,8 +68,8 @@ struct exception_def {
 #define NO_ERROR_CODE 0
 
 // Same order as the tuple of args passed into the exception
-const char *const aerospike_err_attrs[] = {"code", "msg",      "file",
-                                           "line", "in_doubt", NULL};
+const char *const aerospike_err_attrs[] = {
+    "code", "msg", "file", "line", "in_doubt", "subcode", NULL};
 const char *const record_err_attrs[] = {"key", "bin", NULL};
 const char *const index_err_attrs[] = {"name", NULL};
 const char *const udf_err_attrs[] = {"module", "func", NULL};
@@ -465,14 +465,17 @@ void raise_exception_base(as_error *err, PyObject *py_as_key, PyObject *py_bin,
 
     // Convert C error to Python exception
     PyObject *py_err_tuple = NULL;
-    error_to_pyobject(err, &py_err_tuple);
+    create_py_tuple_from_as_error(err, &py_err_tuple);
     if (!py_err_tuple) {
         goto CHAIN_PREV_EXC_AND_RETURN;
     }
 
-    for (unsigned long i = 0;
-         i < sizeof(aerospike_err_attrs) / sizeof(aerospike_err_attrs[0]) - 1;
-         i++) {
+    Py_ssize_t tuple_size = PyTuple_Size(py_err_tuple);
+    if (tuple_size == -1) {
+        goto CHAIN_PREV_EXC_AND_RETURN;
+    }
+
+    for (Py_ssize_t i = 0; i < tuple_size; i++) {
         // Here, we are assuming the number of attrs is the same as the number of tuple members
         PyObject *py_arg = PyTuple_GetItem(py_err_tuple, i);
         if (py_arg == NULL) {
@@ -485,6 +488,18 @@ void raise_exception_base(as_error *err, PyObject *py_as_key, PyObject *py_bin,
         }
     }
 
+    PyObject *py_subcode = PyLong_FromUInt32(err->subcode);
+    if (!py_subcode) {
+        goto CHAIN_PREV_EXC_AND_RETURN;
+    }
+
+    // Subcode is not included as last element in tuple
+    int retval = PyObject_SetAttrString(
+        py_exc_class, aerospike_err_attrs[tuple_size], py_subcode);
+    if (retval == -1) {
+        goto CHAIN_PREV_EXC_AND_RETURN;
+    }
+
     // Raise exception
     PyErr_SetObject(py_exc_class, py_err_tuple);
     Py_DECREF(py_err_tuple);

src/main/policy.c

@@ -322,6 +322,7 @@ static inline as_status pyobject_to_policy_base(AerospikeClient *self,
     POLICY_SET_FIELD(sleep_between_retries, uint32_t);
     POLICY_SET_FIELD(compress, bool);
     POLICY_SET_FIELD(connect_timeout, uint32_t);
+    POLICY_SET_FIELD(error_detail_verbosity, uint8_t);
 
     // Setting txn field to a non-NULL value in a query or scan policy is a no-op,
     // so this is safe to call for a scan/query policy's base policy

src/main/policy_config.c

@@ -18,6 +18,7 @@
 #include "policy_config.h"
 #include "types.h"
 #include "policy.h"
+#include "conversions.h"
 
 as_status set_optional_key(as_policy_key *target_ptr, PyObject *py_policy,
                            const char *name);
@@ -900,6 +901,27 @@ as_status set_batch_remove_policy(as_error *err,
     return AEROSPIKE_OK;
 }
 
+as_status set_optional_uint8_property(uint8_t *target_ptr, PyObject *py_policy,
+                                      const char *name)
+{
+    // Assume py_policy is a Python dictionary
+    PyObject *py_policy_val = PyDict_GetItemString(py_policy, name);
+    if (!py_policy_val) {
+        // Key doesn't exist in policy
+        return AEROSPIKE_OK;
+    }
+
+    uint8_t int_value = convert_pyobject_to_uint8_t(py_policy_val);
+
+    if (PyErr_Occurred()) {
+        PyErr_Clear();
+        return AEROSPIKE_ERR_PARAM;
+    }
+
+    *target_ptr = int_value;
+    return AEROSPIKE_OK;
+}
+
 as_status set_base_policy(as_policy_base *base_policy, PyObject *py_policy)
 {
 
@@ -956,6 +978,11 @@ as_status set_base_policy(as_policy_base *base_policy, PyObject *py_policy)
         return status;
     }
 
+    status = set_optional_uint8_property(&base_policy->error_detail_verbosity,
+                                         py_policy, "error_detail_verbosity");
+    if (status != AEROSPIKE_OK) {
+        return status;
+    }
     return AEROSPIKE_OK;
 }
 

src/main/query/where.c

@@ -281,7 +281,7 @@ static int AerospikeQuery_Where_Add(AerospikeQuery *self, PyObject *py_ctx,
         // If it ain't supported, raise and error
         as_error_update(&err, AEROSPIKE_ERR_PARAM, "unknown predicate type");
         PyObject *py_err = NULL;
-        error_to_pyobject(&err, &py_err);
+        create_py_tuple_from_as_error(&err, &py_err);
         PyErr_SetObject(PyExc_Exception, py_err);
         goto CLEANUP_VALUES_ON_ERROR;
     }

test/new_tests/as_errors.py

@@ -214,6 +214,8 @@
 
 AEROSPIKE_ERR_FAIL_ELEMENT_EXISTS = 24
 
+AEROSPIKE_ERR_OP_NOT_APPLICABLE = 26
+
 AEROSPIKE_FILTERED_OUT = 27
 
 #

test/new_tests/test_base_class.py

@@ -178,9 +178,9 @@ def get_new_connection(add_config=None):
                 if res is not None:
                     break
             res = res.split(".")
-            TestBaseClass.major_ver = res[0]
-            TestBaseClass.minor_ver = res[1]
-            TestBaseClass.minor_ver = res[2]
+            TestBaseClass.major_ver = int(res[0])
+            TestBaseClass.minor_ver = int(res[1])
+            TestBaseClass.minor_ver = int(res[2])
             # print("major_ver:", TestBaseClass.major_ver, "minor_ver:", TestBaseClass.minor_ver)
 
         return client

test/new_tests/test_exception_subcode.py

@@ -0,0 +1,86 @@
+import pytest
+from .conftest import KEYS, BIN_NAME
+import aerospike
+from aerospike import exception as e
+from aerospike_helpers.operations import list_operations as list_ops
+from .test_base_class import TestBaseClass
+from . import as_errors
+
+
+KEY = KEYS[0]
+OPS = [
+    list_ops.list_get_by_index(BIN_NAME, index=99, return_type=aerospike.LIST_RETURN_VALUE)
+]
+ERROR_DETAIL_VERBOSITY_SETTING = "error_detail_verbosity"
+
+
+class TestExceptionSubcode:
+    # TODO: need to reuse fixture in conftest.py using indirect params to set num of records
+    @pytest.fixture(autouse=True)
+    def setup(self, as_connection):
+        self.as_connection.put(KEY, bins={BIN_NAME: []})
+        yield
+        self.as_connection.remove(KEY)
+
+    @pytest.mark.parametrize(
+        "policy_w_verbosity_setting",
+        [
+            {},
+            {ERROR_DETAIL_VERBOSITY_SETTING: 0},
+            {ERROR_DETAIL_VERBOSITY_SETTING: 1},
+            {ERROR_DETAIL_VERBOSITY_SETTING: 2},
+        ]
+    )
+    @pytest.mark.parametrize(
+        "set_in_client_config",
+        [False, True]
+    )
+    def test_error_verbosity_levels(self, policy_w_verbosity_setting: dict, set_in_client_config: bool):
+        if set_in_client_config:
+            config = {
+                "policies": {
+                    "operate": policy_w_verbosity_setting
+                }
+            }
+            self.as_connection = TestBaseClass.get_new_connection(config)
+
+        with pytest.raises(e.OpNotApplicable) as excinfo:
+            cmd_policy = {}
+            if not set_in_client_config:
+                cmd_policy |= policy_w_verbosity_setting
+
+            self.as_connection.operate(KEYS[0], OPS, policy=cmd_policy)
+
+        # Make sure there's no regression with the parent error code
+        assert excinfo.value.code == as_errors.AEROSPIKE_ERR_OP_NOT_APPLICABLE
+
+        subcode_should_be_zero = (
+            ERROR_DETAIL_VERBOSITY_SETTING not in policy_w_verbosity_setting
+            or
+            policy_w_verbosity_setting[ERROR_DETAIL_VERBOSITY_SETTING] == 0
+            or
+            # If running against a unsupported version, we expect subcode to always return 0
+            # (and no undefined behavior)
+            (TestBaseClass.major_ver, TestBaseClass.minor_ver, TestBaseClass.patch_ver) < (8, 1, 3)
+        )
+        if subcode_should_be_zero:
+            assert excinfo.value.subcode == 0
+        else:
+            assert excinfo.value.subcode > 0
+
+        EXPECTED_SUBCODE_IN_MESSAGE = "subcode="
+        if excinfo.value.subcode == 0:
+            assert EXPECTED_SUBCODE_IN_MESSAGE not in excinfo.value.msg
+        elif policy_w_verbosity_setting[ERROR_DETAIL_VERBOSITY_SETTING] == 1:
+            assert EXPECTED_SUBCODE_IN_MESSAGE in excinfo.value.msg
+        else:
+            # There should be a message before the subcode
+            SUBCODE_IN_QUOTES = "({}".format(EXPECTED_SUBCODE_IN_MESSAGE)
+            assert SUBCODE_IN_QUOTES in excinfo.value.msg
+
+    def test_invalid_verbosity(self):
+        policy = {
+            ERROR_DETAIL_VERBOSITY_SETTING: 3
+        }
+        with pytest.raises(e.ServerError):
+            self.as_connection.operate(KEYS[0], OPS, policy=policy)