Skip to content

Commit

Permalink
fix: crash if env not set (#189)
Browse files Browse the repository at this point in the history
Co-authored-by: Hu Yueh-Wei <wei.hu.tw@gmail.com>
  • Loading branch information
leoadonia and halajohn authored Oct 23, 2024
1 parent 53e2b48 commit 52b3ffe
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
18 changes: 15 additions & 3 deletions core/src/ten_runtime/binding/python/native/common/error.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <stdbool.h>

#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) {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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|}"
}
}
]
}
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,51 @@ 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")

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"'
assert testValue2 == '"testValue2"'

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
Expand All @@ -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()

Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down

0 comments on commit 52b3ffe

Please sign in to comment.