diff --git a/aerospike-stubs/exception.pyi b/aerospike-stubs/exception.pyi index c3f11713da..48021b69c8 100644 --- a/aerospike-stubs/exception.pyi +++ b/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] diff --git a/doc/client.rst b/doc/client.rst index 2e18adf07f..85afa909d0 100755 --- a/doc/client.rst +++ b/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 diff --git a/doc/exception.rst b/doc/exception.rst index 3ad3f3b2f1..e0055c295b 100644 --- a/doc/exception.rst +++ b/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. \ diff --git a/doc/spelling_wordlist.txt b/doc/spelling_wordlist.txt index 99a87b51a4..26440f84cc 100644 --- a/doc/spelling_wordlist.txt +++ b/doc/spelling_wordlist.txt @@ -103,4 +103,5 @@ lowercases lowercased msgpack precomposed +subcode DBCS diff --git a/src/include/conversions.h b/src/include/conversions.h index b891bcffe6..e33f1cdc88 100644 --- a/src/include/conversions.h +++ b/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, diff --git a/src/main/aerospike.c b/src/main/aerospike.c index 9d5a02c932..171a2700ec 100644 --- a/src/main/aerospike.c +++ b/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", diff --git a/src/main/conversions.c b/src/main/conversions.c index fc6bc80df1..6e6e06ca99 100644 --- a/src/main/conversions.c +++ b/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); diff --git a/src/main/exception.c b/src/main/exception.c index 9ec4b89066..16b30dbdab 100644 --- a/src/main/exception.c +++ b/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); diff --git a/src/main/policy.c b/src/main/policy.c index f80cf2e9c0..231cca2890 100644 --- a/src/main/policy.c +++ b/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 diff --git a/src/main/policy_config.c b/src/main/policy_config.c index 6a76b45818..c9cfba056a 100644 --- a/src/main/policy_config.c +++ b/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; } diff --git a/src/main/query/where.c b/src/main/query/where.c index f9eb0e37d5..dcd50de34f 100644 --- a/src/main/query/where.c +++ b/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; } diff --git a/test/new_tests/as_errors.py b/test/new_tests/as_errors.py index c12574049e..5eadffd67e 100644 --- a/test/new_tests/as_errors.py +++ b/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 # diff --git a/test/new_tests/test_base_class.py b/test/new_tests/test_base_class.py index 6256e32dd5..0519ba5705 100644 --- a/test/new_tests/test_base_class.py +++ b/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 diff --git a/test/new_tests/test_exception_subcode.py b/test/new_tests/test_exception_subcode.py new file mode 100644 index 0000000000..132d597f48 --- /dev/null +++ b/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)