From 52b3ffe24dcd8cadd7cedcaebded672cab26d6a4 Mon Sep 17 00:00:00 2001 From: leo Date: Wed, 23 Oct 2024 22:16:30 +0800 Subject: [PATCH] fix: crash if env not set (#189) Co-authored-by: Hu Yueh-Wei --- .../ten_runtime/binding/python/common/error.h | 6 +-- .../binding/python/native/common/error.c | 18 ++++++-- .../native/ten_env/ten_env_get_property.c | 10 ++++- .../access_property_go_app/property.json | 8 +++- .../extension/extension_a/extension.go | 11 +++++ .../get_set_prop_python_app/property.json | 6 ++- .../default_extension_python/extension.py | 42 ++++++++++++++----- 7 files changed, 81 insertions(+), 20 deletions(-) diff --git a/core/include_internal/ten_runtime/binding/python/common/error.h b/core/include_internal/ten_runtime/binding/python/common/error.h index 260e768ab..307a4289e 100644 --- a/core/include_internal/ten_runtime/binding/python/common/error.h +++ b/core/include_internal/ten_runtime/binding/python/common/error.h @@ -15,7 +15,7 @@ TEN_RUNTIME_PRIVATE_API bool ten_py_check_and_clear_py_error(void); TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_value_error_exception( - const char *msg); + const char *msg, ...); TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_type_error_exception( const char *msg); @@ -26,8 +26,8 @@ TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_memory_error_exception( TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_system_error_exception( const char *msg); -TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_not_implemented_error_exception( - const char *msg); +TEN_RUNTIME_PRIVATE_API PyObject * +ten_py_raise_py_not_implemented_error_exception(const char *msg); TEN_RUNTIME_PRIVATE_API PyObject *ten_py_raise_py_import_error_exception( const char *msg); diff --git a/core/src/ten_runtime/binding/python/native/common/error.c b/core/src/ten_runtime/binding/python/native/common/error.c index 40da25114..01a857db1 100644 --- a/core/src/ten_runtime/binding/python/native/common/error.c +++ b/core/src/ten_runtime/binding/python/native/common/error.c @@ -7,6 +7,7 @@ #include #include "include_internal/ten_runtime/binding/python/common/python_stuff.h" +#include "ten_utils/lib/string.h" #include "ten_utils/log/log.h" static void ten_py_print_py_error(void) { @@ -67,9 +68,20 @@ bool ten_py_check_and_clear_py_error(void) { return err_occurred; } -PyObject *ten_py_raise_py_value_error_exception(const char *msg) { - TEN_LOGD("Raise Python ValueError exception: %s", msg); - PyErr_SetString(PyExc_ValueError, msg); +PyObject *ten_py_raise_py_value_error_exception(const char *msg, ...) { + ten_string_t err_msg; + ten_string_init(&err_msg); + + va_list args; + va_start(args, msg); + ten_string_append_from_va_list(&err_msg, msg, args); + va_end(args); + + TEN_LOGD("Raise Python ValueError exception: %s", + ten_string_get_raw_str(&err_msg)); + PyErr_SetString(PyExc_ValueError, ten_string_get_raw_str(&err_msg)); + + ten_string_deinit(&err_msg); // Returning NULL indicates that an exception has occurred during the // function's execution, and signaling to the Python interpreter to handle the diff --git a/core/src/ten_runtime/binding/python/native/ten_env/ten_env_get_property.c b/core/src/ten_runtime/binding/python/native/ten_env/ten_env_get_property.c index 9257290dd..5faf33951 100644 --- a/core/src/ten_runtime/binding/python/native/ten_env/ten_env_get_property.c +++ b/core/src/ten_runtime/binding/python/native/ten_env/ten_env_get_property.c @@ -205,11 +205,19 @@ PyObject *ten_py_ten_env_get_property_string(PyObject *self, PyObject *args) { ten_value_t *value = ten_py_ten_property_get_and_check_if_exists(py_ten_env, path); if (!value) { - return ten_py_raise_py_value_error_exception("Failed to get property."); + return ten_py_raise_py_value_error_exception("Property [%s] is not found.", + path); } const char *str_value = ten_value_peek_c_str(value); + if (!str_value) { + ten_value_destroy(value); + + return ten_py_raise_py_value_error_exception( + "Property [%s] is not a string.", path); + } + // Note that `PyUnicode_FromString()` can not accept NULL. PyObject *result = PyUnicode_FromString(str_value); ten_value_destroy(value); diff --git a/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/property.json b/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/property.json index 094dab264..29c13fca5 100644 --- a/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/property.json +++ b/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/property.json @@ -12,10 +12,14 @@ "type": "extension", "name": "A", "addon": "extension_a", - "extension_group": "nodetest" + "extension_group": "nodetest", + "property": { + "env_not_set": "${env:ENV_NOT_SET}", + "env_not_set_has_default": "${env:ENV_NOT_SET|}" + } } ] } ] } -} +} \ No newline at end of file diff --git a/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/ten_packages/extension/extension_a/extension.go b/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/ten_packages/extension/extension_a/extension.go index bfbb346b2..ec317a82e 100644 --- a/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/ten_packages/extension/extension_a/extension.go +++ b/tests/ten_runtime/integration/go/access_property_go/access_property_go_app/ten_packages/extension/extension_a/extension.go @@ -19,6 +19,17 @@ type baseExtension struct { func (ext *baseExtension) OnStart(tenEnv ten.TenEnv) { tenEnv.LogDebug("OnStart") + // The environment variable 'ENV_NOT_SET' is not set, but the property + // should exist. + if _, err := tenEnv.GetPropertyString("env_not_set"); err == nil { + panic("The type should be mismatched, as the default value is not set.") + } + + if prop, err := tenEnv.GetPropertyString("env_not_set_has_default"); err != nil && + prop != "" { + panic("The default value should be used.") + } + tenEnv.OnStartDone() } diff --git a/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/property.json b/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/property.json index 6bb79856f..339acac38 100644 --- a/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/property.json +++ b/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/property.json @@ -19,7 +19,11 @@ "type": "extension", "name": "default_extension_python", "addon": "default_extension_python", - "extension_group": "default_extension_group" + "extension_group": "default_extension_group", + "property": { + "env_not_set": "${env:ENV_NOT_SET}", + "env_not_set_has_default": "${env:ENV_NOT_SET|}" + } }, { "type": "extension", diff --git a/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/ten_packages/extension/default_extension_python/extension.py b/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/ten_packages/extension/default_extension_python/extension.py index 60f667bdc..b48cd1dca 100644 --- a/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/ten_packages/extension/default_extension_python/extension.py +++ b/tests/ten_runtime/integration/python/get_set_prop_python/get_set_prop_python_app/ten_packages/extension/default_extension_python/extension.py @@ -31,7 +31,7 @@ def __test_thread_routine(self, ten_env: TenEnv): i += 1 throw_exception = True - assert throw_exception == True + assert throw_exception is True assert i == 10000 print("DefaultExtension __test_thread_routine done") @@ -39,12 +39,34 @@ def __test_thread_routine(self, ten_env: TenEnv): def on_start(self, ten_env: TenEnv) -> None: ten_env.log_debug("on_start") - assert ten_env.is_property_exist("undefined_key") == False + # The environment variable 'ENV_NOT_SET' used in the property is not + # set, but the property should exist. + assert ten_env.is_property_exist("env_not_set") is True + + try: + _ = ten_env.get_property_string("env_not_set") + + # Should not reach here, as the above line should throw an + # exception. + assert False + except Exception as e: + print(e) + + assert ten_env.is_property_exist("env_not_set_has_default") is True + + try: + env_value = ten_env.get_property_string("env_not_set_has_default") + assert env_value == "" + except Exception as e: + print(e) + assert False + + assert ten_env.is_property_exist("undefined_key") is False ten_env.set_property_from_json("testKey2", '"testValue2"') - assert ten_env.is_property_exist("testKey") == True - assert ten_env.is_property_exist("testKey2") == True + assert ten_env.is_property_exist("testKey") is True + assert ten_env.is_property_exist("testKey2") is True testValue = ten_env.get_property_to_json("testKey") testValue2 = ten_env.get_property_to_json("testKey2") assert testValue == '"testValue"' @@ -52,8 +74,8 @@ def on_start(self, ten_env: TenEnv) -> None: ten_env.set_property_bool("testBoolTrue", True) ten_env.set_property_bool("testBoolFalse", False) - assert ten_env.get_property_bool("testBoolTrue") == True - assert ten_env.get_property_bool("testBoolFalse") == False + assert ten_env.get_property_bool("testBoolTrue") is True + assert ten_env.get_property_bool("testBoolFalse") is False ten_env.set_property_int("testInt", 123) assert ten_env.get_property_int("testInt") == 123 @@ -77,7 +99,7 @@ def on_start(self, ten_env: TenEnv) -> None: except Exception: throw_exception = True - assert throw_exception == True + assert throw_exception is True ten_env.on_start_done() @@ -115,7 +137,7 @@ def check_hello(self, ten_env: TenEnv, result: CmdResult, receivedCmd: Cmd): except Exception: throw_exception = True - assert throw_exception == True + assert throw_exception is True respCmd = CmdResult.create(StatusCode.OK) respCmd.set_property_string("detail", detail + " nbnb") @@ -135,8 +157,8 @@ def on_cmd(self, ten_env: TenEnv, cmd: Cmd) -> None: new_cmd.set_property_bool("testBoolTrue", True) new_cmd.set_property_bool("testBoolFalse", False) - assert new_cmd.get_property_bool("testBoolTrue") == True - assert new_cmd.get_property_bool("testBoolFalse") == False + assert new_cmd.get_property_bool("testBoolTrue") is True + assert new_cmd.get_property_bool("testBoolFalse") is False new_cmd.set_property_int("testInt", 123) assert new_cmd.get_property_int("testInt") == 123