Skip to content
Permalink
Browse files
Improve Script CPP API diagnostics
  • Loading branch information
kwolekr committed Aug 6, 2015
1 parent 3183d5a commit bcf47bc67cf3e1c2f410a81e26ceab1bdab06b4a
Show file tree
Hide file tree
Showing 16 changed files with 129 additions and 107 deletions.
@@ -69,11 +69,51 @@ int script_exception_wrapper(lua_State *L, lua_CFunction f)
return lua_error(L); // Rethrow as a Lua error.
}

void script_error(lua_State *L)
/*
* Note that we can't get tracebacks for LUA_ERRMEM or LUA_ERRERR (without
* hacking Lua internals). For LUA_ERRMEM, this is because memory errors will
* not execute the the error handler, and by the time lua_pcall returns the
* execution stack will have already been unwound. For LUA_ERRERR, there was
* another error while trying to generate a backtrace from a LUA_ERRRUN. It is
* presumed there is an error with the internal Lua state and thus not possible
* to gather a coherent backtrace. Realistically, the best we can do here is
* print which C function performed the failing pcall.
*/
void script_error(lua_State *L, int pcall_result, const char *fxn)
{
const char *s = lua_tostring(L, -1);
std::string str(s ? s : "");
throw LuaError(str);
if (pcall_result == 0)
return;

const char *err_type;
switch (pcall_result) {
case LUA_ERRRUN:
err_type = "Runtime";
break;
case LUA_ERRMEM:
err_type = "OOM";

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Aug 24, 2015

Member

Perhaps spelling this out would be helpfull for those that aren't familiar with the acronym.

This comment has been minimized.

Copy link
@est31

est31 Aug 24, 2015

Contributor

It won't help much either if spelling out, because often there is enough memory available, but due to some bug it still gives "OOM" as a reason. So its better to have an opaque set of letters than even more non-experienced users think its hw fault.

break;
case LUA_ERRERR:
err_type = "Double fault";
break;
default:
err_type = "Unknown";
}

const char *err_descr = lua_tostring(L, -1);
if (!err_descr)
err_descr = "<no description>";

std::string err_msg(err_type);
if (fxn) {
err_msg += " error in ";
err_msg += fxn;
err_msg += "(): ";
} else {
err_msg += " error: ";
}
err_msg += err_descr;

throw LuaError(err_msg);
}

// Push the list of callbacks (a lua table).
@@ -82,7 +122,8 @@ void script_error(lua_State *L)
// - runs the callbacks
// - replaces the table and arguments with the return value,
// computed depending on mode
void script_run_callbacks(lua_State *L, int nargs, RunCallbacksMode mode)
void script_run_callbacks_f(lua_State *L, int nargs,
RunCallbacksMode mode, const char *fxn)
{
FATAL_ERROR_IF(lua_gettop(L) < nargs + 1, "Not enough arguments");

@@ -104,14 +145,12 @@ void script_run_callbacks(lua_State *L, int nargs, RunCallbacksMode mode)
// Stack now looks like this:
// ... <error handler> <run_callbacks> <table> <mode> <arg#1> <arg#2> ... <arg#n>

if (lua_pcall(L, nargs + 2, 1, errorhandler)) {
script_error(L);
}
script_error(L, lua_pcall(L, nargs + 2, 1, errorhandler), fxn);

lua_remove(L, -2); // Remove error handler
}

void log_deprecated(lua_State *L, std::string message)
void log_deprecated(lua_State *L, const std::string &message)
{
static bool configured = false;
static bool dolog = false;
@@ -131,9 +170,10 @@ void log_deprecated(lua_State *L, std::string message)

if (doerror) {
if (L != NULL) {
script_error(L);
script_error(L, LUA_ERRRUN, NULL);
} else {
FATAL_ERROR("Can't do a scripterror for this deprecated message, so exit completely!");
FATAL_ERROR("Can't do a scripterror for this deprecated message, "
"so exit completely!");
}
}

@@ -34,6 +34,16 @@ extern "C" {

#include "common/c_types.h"

#define PCALL_RESL(L, RES) do { \
int result_ = (RES); \
if (result_ != 0) { \
script_error((L), result_, __FUNCTION__); \
} \
} while (0)

#define script_run_callbacks(L, nargs, mode) \
script_run_callbacks_f((L), (nargs), (mode), __FUNCTION__)

// What script_run_callbacks does with the return values of callbacks.
// Regardless of the mode, if only one callback is defined,
// its return value is the total return value.
@@ -67,8 +77,9 @@ enum RunCallbacksMode
std::string script_get_backtrace(lua_State *L);
int script_error_handler(lua_State *L);
int script_exception_wrapper(lua_State *L, lua_CFunction f);
void script_error(lua_State *L);
void script_run_callbacks(lua_State *L, int nargs, RunCallbacksMode mode);
void log_deprecated(lua_State *L, std::string message);
void script_error(lua_State *L, int pcall_result, const char *fxn);
void script_run_callbacks_f(lua_State *L, int nargs,
RunCallbacksMode mode, const char *fxn);
void log_deprecated(lua_State *L, const std::string &message);

#endif /* C_INTERNAL_H_ */
@@ -164,9 +164,7 @@ void AsyncEngine::step(lua_State *L, int errorhandler)
lua_pushlstring(L, jobDone.serializedResult.data(),
jobDone.serializedResult.size());

if (lua_pcall(L, 2, 0, errorhandler)) {
script_error(L);
}
PCALL_RESL(L, lua_pcall(L, 2, 0, errorhandler));
}
resultQueueMutex.Unlock();
lua_pop(L, 1); // Pop core
@@ -293,8 +291,9 @@ void* AsyncWorkerThread::Thread()
toProcess.serializedParams.data(),
toProcess.serializedParams.size());

if (lua_pcall(L, 2, 1, m_errorhandler)) {
scriptError();
int result = lua_pcall(L, 2, 1, m_errorhandler);
if (result) {
PCALL_RES(result);
toProcess.serializedResult = "";
} else {
// Fetch result
@@ -165,9 +165,9 @@ void ScriptApiBase::realityCheck()
}
}

void ScriptApiBase::scriptError()
void ScriptApiBase::scriptError(int result, const char *fxn)
{
throw LuaError(lua_tostring(m_luastack, -1));
script_error(getStack(), result, fxn);
}

void ScriptApiBase::stackDump(std::ostream &o)
@@ -40,6 +40,12 @@ extern "C" {
// use that name to bypass security!
#define BUILTIN_MOD_NAME "*builtin*"

#define PCALL_RES(RES) do { \
int result_ = (RES); \
if (result_ != 0) { \
scriptError(result_, __FUNCTION__); \
} \
} while (0)

class Server;
class Environment;
@@ -74,7 +80,7 @@ class ScriptApiBase {
{ return m_luastack; }

void realityCheck();
void scriptError();
void scriptError(int result, const char *fxn);
void stackDump(std::ostream &o);

void setServer(Server* server) { m_server = server; }
@@ -92,8 +92,7 @@ void ScriptApiEntity::luaentity_Activate(u16 id,
lua_pushlstring(L, staticdata.c_str(), staticdata.size());
lua_pushinteger(L, dtime_s);
// Call with 3 arguments, 0 results
if (lua_pcall(L, 3, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 3, 0, m_errorhandler));
} else {
lua_pop(L, 1);
}
@@ -140,8 +139,7 @@ std::string ScriptApiEntity::luaentity_GetStaticdata(u16 id)
luaL_checktype(L, -1, LUA_TFUNCTION);
lua_pushvalue(L, object); // self
// Call with 1 arguments, 1 results
if (lua_pcall(L, 1, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 1, 1, m_errorhandler));
lua_remove(L, object); // Remove object

size_t len = 0;
@@ -210,8 +208,7 @@ void ScriptApiEntity::luaentity_Step(u16 id, float dtime)
lua_pushvalue(L, object); // self
lua_pushnumber(L, dtime); // dtime
// Call with 2 arguments, 0 results
if (lua_pcall(L, 2, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 2, 0, m_errorhandler));
lua_pop(L, 1); // Pop object
}

@@ -242,8 +239,7 @@ void ScriptApiEntity::luaentity_Punch(u16 id,
push_tool_capabilities(L, *toolcap);
push_v3f(L, dir);
// Call with 5 arguments, 0 results
if (lua_pcall(L, 5, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 5, 0, m_errorhandler));
lua_pop(L, 1); // Pop object
}

@@ -269,8 +265,7 @@ void ScriptApiEntity::luaentity_Rightclick(u16 id,
lua_pushvalue(L, object); // self
objectrefGetOrCreate(L, clicker); // Clicker reference
// Call with 2 arguments, 0 results
if (lua_pcall(L, 2, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 2, 0, m_errorhandler));
lua_pop(L, 1); // Pop object
}

@@ -48,8 +48,7 @@ int ScriptApiDetached::detached_inventory_AllowMove(
lua_pushinteger(L, to_index + 1); // to_index
lua_pushinteger(L, count); // count
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 7, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 7, 1, m_errorhandler));
if(!lua_isnumber(L, -1))
throw LuaError("allow_move should return a number. name=" + name);
int ret = luaL_checkinteger(L, -1);
@@ -77,8 +76,7 @@ int ScriptApiDetached::detached_inventory_AllowPut(
lua_pushinteger(L, index + 1); // index
LuaItemStack::create(L, stack); // stack
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 5, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 5, 1, m_errorhandler));
if (!lua_isnumber(L, -1))
throw LuaError("allow_put should return a number. name=" + name);
int ret = luaL_checkinteger(L, -1);
@@ -106,8 +104,7 @@ int ScriptApiDetached::detached_inventory_AllowTake(
lua_pushinteger(L, index + 1); // index
LuaItemStack::create(L, stack); // stack
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 5, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 5, 1, m_errorhandler));
if (!lua_isnumber(L, -1))
throw LuaError("allow_take should return a number. name=" + name);
int ret = luaL_checkinteger(L, -1);
@@ -139,8 +136,7 @@ void ScriptApiDetached::detached_inventory_OnMove(
lua_pushinteger(L, to_index + 1); // to_index
lua_pushinteger(L, count); // count
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 7, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 7, 0, m_errorhandler));
}

// Report put items
@@ -164,8 +160,7 @@ void ScriptApiDetached::detached_inventory_OnPut(
lua_pushinteger(L, index + 1); // index
LuaItemStack::create(L, stack); // stack
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 5, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 5, 0, m_errorhandler));
}

// Report taken items
@@ -189,8 +184,7 @@ void ScriptApiDetached::detached_inventory_OnTake(
lua_pushinteger(L, index + 1); // index
LuaItemStack::create(L, stack); // stack
objectrefGetOrCreate(L, player); // player
if (lua_pcall(L, 5, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 5, 0, m_errorhandler));
}

// Retrieves core.detached_inventories[name][callbackname]
@@ -42,8 +42,7 @@ bool ScriptApiItem::item_OnDrop(ItemStack &item,
LuaItemStack::create(L, item);
objectrefGetOrCreate(L, dropper);
pushFloatPos(L, pos);
if (lua_pcall(L, 3, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 3, 1, m_errorhandler));
if (!lua_isnil(L, -1)) {
try {
item = read_item(L,-1, getServer());
@@ -68,8 +67,7 @@ bool ScriptApiItem::item_OnPlace(ItemStack &item,
LuaItemStack::create(L, item);
objectrefGetOrCreate(L, placer);
pushPointedThing(pointed);
if (lua_pcall(L, 3, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 3, 1, m_errorhandler));
if (!lua_isnil(L, -1)) {
try {
item = read_item(L,-1, getServer());
@@ -94,8 +92,7 @@ bool ScriptApiItem::item_OnUse(ItemStack &item,
LuaItemStack::create(L, item);
objectrefGetOrCreate(L, user);
pushPointedThing(pointed);
if (lua_pcall(L, 3, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 3, 1, m_errorhandler));
if(!lua_isnil(L, -1)) {
try {
item = read_item(L,-1, getServer());
@@ -116,7 +113,7 @@ bool ScriptApiItem::item_OnCraft(ItemStack &item, ServerActiveObject *user,
lua_getfield(L, -1, "on_craft");
LuaItemStack::create(L, item);
objectrefGetOrCreate(L, user);

// Push inventory list
std::vector<ItemStack> items;
for (u32 i = 0; i < old_craft_grid->getSize(); i++) {
@@ -125,8 +122,7 @@ bool ScriptApiItem::item_OnCraft(ItemStack &item, ServerActiveObject *user,
push_items(L, items);

InvRef::create(L, craft_inv);
if (lua_pcall(L, 4, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 4, 1, m_errorhandler));
if (!lua_isnil(L, -1)) {
try {
item = read_item(L,-1, getServer());
@@ -156,8 +152,7 @@ bool ScriptApiItem::item_CraftPredict(ItemStack &item, ServerActiveObject *user,
push_items(L, items);

InvRef::create(L, craft_inv);
if (lua_pcall(L, 4, 1, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 4, 1, m_errorhandler));
if (!lua_isnil(L, -1)) {
try {
item = read_item(L,-1, getServer());
@@ -55,8 +55,7 @@ void ScriptApiMainMenu::handleMainMenuEvent(std::string text)

// Call it
lua_pushstring(L, text.c_str());
if (lua_pcall(L, 1, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 1, 0, m_errorhandler));
}

void ScriptApiMainMenu::handleMainMenuButtons(const StringMap &fields)
@@ -85,7 +84,6 @@ void ScriptApiMainMenu::handleMainMenuButtons(const StringMap &fields)
}

// Call it
if (lua_pcall(L, 1, 0, m_errorhandler))
scriptError();
PCALL_RES(lua_pcall(L, 1, 0, m_errorhandler));
}

0 comments on commit bcf47bc

Please sign in to comment.