From 406a86da2f677131865172e44b6ed3acfb9b5d7b Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 28 Aug 2022 23:34:47 +0100 Subject: [PATCH] Improved errors parsing code Refactored errors parsing code into a common utility Added testing code for errors parsing Bumped revision number --- premake5.lua | 33 +++++-- source/common/common.cpp | 106 ++++++++++++++++++++++ source/common/common.hpp | 58 ++++++++++++ source/{ => server}/server.cpp | 72 ++++----------- source/{ => server}/server.hpp | 0 source/{ => shared}/main.cpp | 8 +- source/{ => shared}/shared.cpp | 73 ++++------------ source/{ => shared}/shared.hpp | 5 -- source/testing/main.cpp | 155 +++++++++++++++++++++++++++++++++ 9 files changed, 381 insertions(+), 129 deletions(-) create mode 100644 source/common/common.cpp create mode 100644 source/common/common.hpp rename source/{ => server}/server.cpp (65%) rename source/{ => server}/server.hpp (100%) rename source/{ => shared}/main.cpp (83%) rename source/{ => shared}/shared.cpp (85%) rename source/{ => shared}/shared.hpp (62%) create mode 100644 source/testing/main.cpp diff --git a/premake5.lua b/premake5.lua index a6af10e..167cf94 100644 --- a/premake5.lua +++ b/premake5.lua @@ -20,11 +20,13 @@ CreateWorkspace({name = "luaerror", abi_compatible = true}) IncludeDetouring() files({ - "source/main.cpp", - "source/server.cpp", - "source/server.hpp", - "source/shared.cpp", - "source/shared.hpp" + "source/shared/main.cpp", + "source/server/server.cpp", + "source/server/server.hpp", + "source/shared/shared.cpp", + "source/shared/shared.hpp", + "source/common/common.cpp", + "source/common/common.hpp" }) CreateProject({serverside = false, manual_files = true}) @@ -36,7 +38,22 @@ CreateWorkspace({name = "luaerror", abi_compatible = true}) IncludeScanning() files({ - "source/main.cpp", - "source/shared.cpp", - "source/shared.hpp" + "source/shared/main.cpp", + "source/shared/shared.cpp", + "source/shared/shared.hpp", + "source/common/common.cpp", + "source/common/common.hpp" + }) + + project("testing") + kind("ConsoleApp") + includedirs("source/common") + files({ + "source/common/common.hpp", + "source/common/common.cpp", + "source/testing/main.cpp" + }) + vpaths({ + ["Header files/*"] = "source/**.hpp", + ["Source files/*"] = "source/**.cpp" }) diff --git a/source/common/common.cpp b/source/common/common.cpp new file mode 100644 index 0000000..72262af --- /dev/null +++ b/source/common/common.cpp @@ -0,0 +1,106 @@ +#include "common.hpp" + +#include +#include +#include +#include +#include + +namespace common +{ + +inline int32_t StringToInteger( const std::string &strint ) +{ + try + { + return std::stoi( strint ); + } + catch( const std::exception & ) + { + return 0; + } +} + +inline std::string Trim( const std::string &s ) +{ + std::string c = s; + auto not_isspace = std::not_fn( isspace ); + // remote trailing "spaces" + c.erase( std::find_if( c.rbegin( ), c.rend( ), not_isspace ).base( ), c.end( ) ); + // remote initial "spaces" + c.erase( c.begin( ), std::find_if( c.begin( ), c.end( ), not_isspace ) ); + return c; +} + +bool ParseError( const std::string &error, ParsedError &parsed_error ) +{ + static const std::regex error_parts_regex( + "^(.+):(\\d+): (.+)$", + std::regex::ECMAScript | std::regex::optimize + ); + + std::smatch matches; + if( !std::regex_search( error, matches, error_parts_regex ) ) + return false; + + parsed_error.source_file = matches[1]; + parsed_error.source_line = StringToInteger( matches[2] ); + parsed_error.error_string = matches[3]; + return true; +} + +bool ParseErrorWithStackTrace( const std::string &error, ParsedErrorWithStackTrace &parsed_error ) +{ + std::istringstream error_stream( Trim( error ) ); + + std::string error_first_line; + if( !std::getline( error_stream, error_first_line ) ) + return false; + + ParsedErrorWithStackTrace temp_parsed_error; + + { + static const std::regex client_error_addon_matcher( + "^\\[(.+)\\] ", + std::regex::ECMAScript | std::regex::optimize + ); + + std::smatch matches; + if( std::regex_search( error_first_line, matches, client_error_addon_matcher ) ) + { + temp_parsed_error.addon_name = matches[1]; + error_first_line.erase( 0, 1 + temp_parsed_error.addon_name.size( ) + 1 + 1 ); // [addon]:space: + } + } + + if( !ParseError( error_first_line, temp_parsed_error ) ) + temp_parsed_error.error_string = error_first_line; + + while( error_stream ) + { + static const std::regex frame_parts_regex( + "^\\s+(\\d+)\\. (.+) \\- (.+):(\\-?\\d+)$", + std::regex::ECMAScript | std::regex::optimize + ); + + std::string frame_line; + if( !std::getline( error_stream, frame_line ) ) + break; + + std::smatch matches; + if( !std::regex_search( frame_line, matches, frame_parts_regex ) ) + return false; + + temp_parsed_error.stack_trace.emplace_back( ParsedErrorWithStackTrace::StackFrame { + StringToInteger( matches[1] ), + matches[2], + matches[3], + StringToInteger( matches[4] ) + } ); + } + + parsed_error = std::move( temp_parsed_error ); + return true; +} + +} diff --git a/source/common/common.hpp b/source/common/common.hpp new file mode 100644 index 0000000..6ae5467 --- /dev/null +++ b/source/common/common.hpp @@ -0,0 +1,58 @@ +#pragma once + +#include +#include +#include + +namespace common +{ + +struct ParsedError +{ + std::string source_file; + int32_t source_line = -1; + std::string error_string; + + inline bool operator==( const ParsedError &rhs ) const + { + return source_file == rhs.source_file && + source_line == rhs.source_line && + error_string == rhs.error_string; + } +}; + +struct ParsedErrorWithStackTrace : public ParsedError +{ + struct StackFrame + { + int32_t level = 0; + std::string name; + std::string source; + int32_t currentline = -1; + + inline bool operator==( const StackFrame &rhs ) const + { + return level == rhs.level && + name == rhs.name && + source == rhs.source && + currentline == rhs.currentline; + } + }; + + std::string addon_name; + std::vector stack_trace; + + inline bool operator==( const ParsedErrorWithStackTrace &rhs ) const + { + return source_file == rhs.source_file && + source_line == rhs.source_line && + error_string == rhs.error_string && + addon_name == rhs.addon_name && + stack_trace == rhs.stack_trace; + } +}; + +bool ParseError( const std::string &error, ParsedError &parsed_error ); +bool ParseErrorWithStackTrace( const std::string &error, ParsedErrorWithStackTrace &parsed_error ); + +} diff --git a/source/server.cpp b/source/server/server.cpp similarity index 65% rename from source/server.cpp rename to source/server/server.cpp index 85e185b..4ab7da6 100644 --- a/source/server.cpp +++ b/source/server/server.cpp @@ -1,5 +1,5 @@ #include "server.hpp" -#include "shared.hpp" +#include "common/common.hpp" #include #include @@ -28,25 +28,16 @@ namespace server static GarrysMod::Lua::ILuaInterface *lua = nullptr; -static std::regex client_error_addon_matcher( "^\\[(.+)\\] ", std::regex_constants::optimize ); - typedef void ( *HandleClientLuaError_t )( CBasePlayer *player, const char *error ); static Detouring::Hook HandleClientLuaError_detour; -inline std::string Trim( const std::string &s ) -{ - std::string c = s; - auto not_isspace = std::not_fn( isspace ); - // remote trailing "spaces" - c.erase( std::find_if( c.rbegin( ), c.rend( ), not_isspace ).base( ), c.end( ) ); - // remote initial "spaces" - c.erase( c.begin( ), std::find_if( c.begin( ), c.end( ), not_isspace ) ); - return c; -} - static void HandleClientLuaError_d( CBasePlayer *player, const char *error ) { + common::ParsedErrorWithStackTrace parsed_error; + if( !common::ParseErrorWithStackTrace( error, parsed_error ) ) + return HandleClientLuaError_detour.GetTrampoline( )( player, error ); + const int32_t funcs = LuaHelpers::PushHookRun( lua, "ClientLuaError" ); if( funcs == 0 ) return HandleClientLuaError_detour.GetTrampoline( )( player, error ); @@ -54,70 +45,43 @@ static void HandleClientLuaError_d( CBasePlayer *player, const char *error ) lua->GetField( GarrysMod::Lua::INDEX_GLOBAL, "Entity" ); if( !lua->IsType( -1, GarrysMod::Lua::Type::FUNCTION ) ) { - lua->Pop( funcs + 2 ); + lua->Pop( funcs + 1 ); lua->ErrorNoHalt( "[ClientLuaError] Global Entity is not a function!\n" ); return HandleClientLuaError_detour.GetTrampoline( )( player, error ); } lua->PushNumber( player->entindex( ) ); lua->Call( 1, 1 ); - std::string cleanerror = Trim( error ); - std::string addon; - std::smatch matches; - if( std::regex_search( cleanerror, matches, client_error_addon_matcher ) ) - { - addon = matches[1]; - cleanerror.erase( 0, 1 + addon.size( ) + 1 + 1 ); // [addon]:space: - } - - lua->PushString( cleanerror.c_str( ) ); + lua->PushString( error ); - std::istringstream errstream( cleanerror ); - const int32_t errorPropsCount = shared::PushErrorProperties( lua, errstream ); + lua->PushString( parsed_error.source_file.c_str( ) ); + lua->PushNumber( parsed_error.source_line ); + lua->PushString( parsed_error.error_string.c_str( ) ); lua->CreateTable( ); - while( errstream ) + for( const auto &stack_frame : parsed_error.stack_trace ) { - int32_t level = 0; - errstream >> level; - - errstream.ignore( 2 ); // ignore ". " - - std::string name; - errstream >> name; - - errstream.ignore( 3 ); // ignore " - " - - std::string source; - std::getline( errstream, source, ':' ); - - int32_t currentline = -1; - errstream >> currentline; - - if( !errstream ) // it shouldn't have reached eof by now - break; - - lua->PushNumber( level ); + lua->PushNumber( stack_frame.level ); lua->CreateTable( ); - lua->PushString( name.c_str( ) ); + lua->PushString( stack_frame.name.c_str( ) ); lua->SetField( -2, "name" ); - lua->PushNumber( currentline ); + lua->PushNumber( stack_frame.currentline ); lua->SetField( -2, "currentline" ); - lua->PushString( source.c_str( ) ); + lua->PushString( stack_frame.source.c_str( ) ); lua->SetField( -2, "source" ); lua->SetTable( -3 ); } - if( addon.empty( ) ) + if( parsed_error.addon_name.empty( ) ) lua->PushNil( ); else - lua->PushString( addon.c_str( ) ); + lua->PushString( parsed_error.addon_name.c_str( ) ); - if( !LuaHelpers::CallHookRun( lua, 4 + errorPropsCount, 1 ) ) + if( !LuaHelpers::CallHookRun( lua, 7, 1 ) ) return HandleClientLuaError_detour.GetTrampoline( )( player, error ); const bool proceed = !lua->IsType( -1, GarrysMod::Lua::Type::BOOL ) || !lua->GetBool( -1 ); diff --git a/source/server.hpp b/source/server/server.hpp similarity index 100% rename from source/server.hpp rename to source/server/server.hpp diff --git a/source/main.cpp b/source/shared/main.cpp similarity index 83% rename from source/main.cpp rename to source/shared/main.cpp index 347749c..5ca9f2f 100644 --- a/source/main.cpp +++ b/source/shared/main.cpp @@ -1,10 +1,10 @@ -#include "shared.hpp" +#include "shared/shared.hpp" #include #if defined LUAERROR_SERVER -#include "server.hpp" +#include "server/server.hpp" #endif @@ -12,11 +12,11 @@ GMOD_MODULE_OPEN( ) { LUA->CreateTable( ); - LUA->PushString( "luaerror 1.5.7" ); + LUA->PushString( "luaerror 1.5.8" ); LUA->SetField( -2, "Version" ); // version num follows LuaJIT style, xxyyzz - LUA->PushNumber( 10507 ); + LUA->PushNumber( 10508 ); LUA->SetField( -2, "VersionNum" ); #if defined LUAERROR_SERVER diff --git a/source/shared.cpp b/source/shared/shared.cpp similarity index 85% rename from source/shared.cpp rename to source/shared/shared.cpp index 9804f7c..24958db 100644 --- a/source/shared.cpp +++ b/source/shared/shared.cpp @@ -1,4 +1,5 @@ #include "shared.hpp" +#include "common/common.hpp" #include #include @@ -8,8 +9,10 @@ #include #include +#include #include #include +#include #include @@ -24,50 +27,6 @@ static bool runtime_detoured = false; static bool compiletime_detoured = false; static GarrysMod::Lua::CFunc AdvancedLuaErrorReporter = nullptr; -struct ErrorProperties -{ - std::string source_file; - int32_t source_line = -1; - std::string error_string; -}; - -static int32_t PushErrorProperties( GarrysMod::Lua::ILuaInterface *lua, std::istringstream &error, ErrorProperties &props ) -{ - std::string source_file; - std::getline( error, source_file, ':' ); - - int32_t source_line = -1; - error >> source_line; - - error.ignore( 2 ); // ignore ": " - - std::string error_string; - std::getline( error, error_string ); - - if( !error ) // our stream is still valid - { - lua->PushNil( ); - lua->PushNil( ); - lua->PushNil( ); - return 3; - } - - props.source_file = source_file; - props.source_line = source_line; - props.error_string = error_string; - - lua->PushString( props.source_file.c_str( ) ); - lua->PushNumber( props.source_line ); - lua->PushString( props.error_string.c_str( ) ); - return 3; -} - -int32_t PushErrorProperties( GarrysMod::Lua::ILuaInterface *lua, std::istringstream &error ) -{ - ErrorProperties props; - return PushErrorProperties( lua, error, props ); -} - inline bool GetUpvalues( GarrysMod::Lua::ILuaInterface *lua, int32_t funcidx ) { if( funcidx < 0 ) @@ -137,7 +96,7 @@ inline bool GetLocals( GarrysMod::Lua::ILuaInterface *lua, lua_Debug &dbg ) return true; } -static int32_t PushStackTable( GarrysMod::Lua::ILuaInterface *lua ) +static void PushStackTable( GarrysMod::Lua::ILuaInterface *lua ) { lua->CreateTable( ); @@ -195,8 +154,6 @@ static int32_t PushStackTable( GarrysMod::Lua::ILuaInterface *lua ) // Pop activelines and func lua->Pop( 2 ); } - - return 1; } inline const IAddonSystem::Information *FindWorkshopAddonFromFile( const std::string &source ) @@ -270,34 +227,34 @@ class CLuaGameCallback : public GarrysMod::Lua::ILuaGameCallback void LuaError( const CLuaError *error ) { - if( entered_hook ) + const std::string &error_str = runtime ? runtime_error : error->message; + + common::ParsedError parsed_error; + if( entered_hook || !common::ParseError( error_str, parsed_error ) ) return callback->LuaError( error ); const int32_t funcs = LuaHelpers::PushHookRun( lua, "LuaError" ); if( funcs == 0 ) return callback->LuaError( error ); - const std::string &errstr = runtime ? runtime_error : error->message; - lua->PushBool( runtime ); - lua->PushString( errstr.c_str( ) ); + lua->PushString( error_str.c_str( ) ); - std::istringstream errstream( errstr ); - ErrorProperties props; - int32_t args = PushErrorProperties( lua, errstream, props ); + lua->PushString( parsed_error.source_file.c_str( ) ); + lua->PushNumber( parsed_error.source_line ); + lua->PushString( parsed_error.error_string.c_str( ) ); if( runtime ) { - args += 1; runtime_stack.Push( ); runtime_stack.Free( ); } else - args += PushStackTable( lua ); + PushStackTable( lua ); runtime = false; - const auto source_addon = FindWorkshopAddonFromFile( props.source_file ); + const auto source_addon = FindWorkshopAddonFromFile( parsed_error.source_file ); if( source_addon == nullptr ) { lua->PushNil( ); @@ -310,7 +267,7 @@ class CLuaGameCallback : public GarrysMod::Lua::ILuaGameCallback } entered_hook = true; - const bool call_success = LuaHelpers::CallHookRun( lua, 4 + args, 1 ); + const bool call_success = LuaHelpers::CallHookRun( lua, 8, 1 ); entered_hook = false; if( !call_success ) return callback->LuaError( error ); diff --git a/source/shared.hpp b/source/shared/shared.hpp similarity index 62% rename from source/shared.hpp rename to source/shared/shared.hpp index 2771810..2ec86d9 100644 --- a/source/shared.hpp +++ b/source/shared/shared.hpp @@ -1,8 +1,5 @@ #pragma once -#include -#include - namespace GarrysMod { namespace Lua @@ -15,8 +12,6 @@ namespace GarrysMod namespace shared { -int32_t PushErrorProperties( GarrysMod::Lua::ILuaInterface *lua, std::istringstream &error ); - void Initialize( GarrysMod::Lua::ILuaBase *LUA ); void Deinitialize( GarrysMod::Lua::ILuaBase *LUA ); diff --git a/source/testing/main.cpp b/source/testing/main.cpp new file mode 100644 index 0000000..da64e54 --- /dev/null +++ b/source/testing/main.cpp @@ -0,0 +1,155 @@ +#include + +#include + +static bool test_parsed_error( const std::string &error, const common::ParsedError &control_parsed_error ) +{ + common::ParsedError parsed_error; + if( !common::ParseError( error, parsed_error ) ) + return false; + + return parsed_error == control_parsed_error; +} + +static bool test_parsed_error_with_stacktrace( const std::string &error, const common::ParsedErrorWithStackTrace &control_parsed_error ) +{ + common::ParsedErrorWithStackTrace parsed_error; + if( !common::ParseErrorWithStackTrace( error, parsed_error ) ) + return false; + + return parsed_error == control_parsed_error; +} + +int main( const int, const char *[] ) +{ + const std::string error1 = "lua_run:1: '=' expected near ''"; + const common::ParsedError control_parsed_error1 = + { + "lua_run", + 1, + "'=' expected near ''" + }; + if( !test_parsed_error( error1, control_parsed_error1 ) ) + { + printf( "Failed on test case 1!\n" ); + return 5; + } + + const std::string error2 = + "\n" + "[gcad] bad argument #3 to 'Add' (function expected, got nil)\n" + " 1. Add - lua/includes/modules/hook.lua:31\n" + " 2. unknown - addons/gcad/lua/gcad/ui/contextmenu/contextmenueventhandler.lua:150\n" + " 3. dtor - addons/glib/lua/glib/oop/oop.lua:292\n" + " 4. unknown - addons/gcad/lua/gcad/ui/contextmenu/contextmenueventhandler.lua:130\n" + " 5. xpcall - [C]:-1\n" + " 6. DispatchEvent - addons/glib/lua/glib/events/eventprovider.lua:86\n" + " 7. UnloadSystem - addons/glib/lua/glib/stage1.lua:380\n" + " 8. RunPackFile - addons/glib/lua/glib/loader/loader.lua:200\n" + " 9. runNextPackFile - addons/glib/lua/glib/loader/loader.lua:494\n" + " 10. callback - addons/glib/lua/glib/loader/loader.lua:497\n" + " 11. RunPackFile - addons/glib/lua/glib/loader/loader.lua:296\n" + " 12. runNextPackFile - addons/glib/lua/glib/loader/loader.lua:494\n" + " 13. callback - addons/glib/lua/glib/loader/loader.lua:497\n" + " 14. RunPackFile - addons/glib/lua/glib/loader/loader.lua:296\n" + " 15. runNextPackFile - addons/glib/lua/glib/loader/loader.lua:494\n" + " 16. callback - addons/glib/lua/glib/loader/loader.lua:497\n" + "\n"; + const common::ParsedErrorWithStackTrace control_parsed_error2 = + { + "", + -1, + "bad argument #3 to 'Add' (function expected, got nil)", + "gcad", + { + { 1, "Add", "lua/includes/modules/hook.lua", 31 }, + { 2, "unknown", "addons/gcad/lua/gcad/ui/contextmenu/contextmenueventhandler.lua", 150 }, + { 3, "dtor", "addons/glib/lua/glib/oop/oop.lua", 292 }, + { 4, "unknown", "addons/gcad/lua/gcad/ui/contextmenu/contextmenueventhandler.lua", 130 }, + { 5, "xpcall", "[C]", -1 }, + { 6, "DispatchEvent", "addons/glib/lua/glib/events/eventprovider.lua", 86 }, + { 7, "UnloadSystem", "addons/glib/lua/glib/stage1.lua", 380 }, + { 8, "RunPackFile", "addons/glib/lua/glib/loader/loader.lua", 200 }, + { 9, "runNextPackFile", "addons/glib/lua/glib/loader/loader.lua", 494 }, + { 10, "callback", "addons/glib/lua/glib/loader/loader.lua", 497 }, + { 11, "RunPackFile", "addons/glib/lua/glib/loader/loader.lua", 296 }, + { 12, "runNextPackFile", "addons/glib/lua/glib/loader/loader.lua", 494 }, + { 13, "callback", "addons/glib/lua/glib/loader/loader.lua", 497 }, + { 14, "RunPackFile", "addons/glib/lua/glib/loader/loader.lua", 296 }, + { 15, "runNextPackFile", "addons/glib/lua/glib/loader/loader.lua", 494 }, + { 16, "callback", "addons/glib/lua/glib/loader/loader.lua", 497 }, + } + }; + if( !test_parsed_error_with_stacktrace( error2, control_parsed_error2 ) ) + { + printf( "Failed on test case 2!\n" ); + return 5; + } + + const std::string error3 = + "\n" + "[ERROR] CompileString:1: '=' expected near ''\n" + " 1. unknown - lua_run:1\n" + "\n"; + const common::ParsedErrorWithStackTrace control_parsed_error3 = + { + "CompileString", + 1, + "'=' expected near ''", + "ERROR", + { + { 1, "unknown", "lua_run", 1 } + } + }; + if( !test_parsed_error_with_stacktrace( error3, control_parsed_error3 ) ) + { + printf( "Failed on test case 3!\n" ); + return 5; + } + + const std::string error4 = + "\n" + "[ERROR] lua_run:1: yes\n" + " 1. error - [C]:-1\n" + " 2. err - lua_run:1\n" + " 3. unknown - lua_run:1\n" + "\n"; + const common::ParsedErrorWithStackTrace control_parsed_error4 = + { + "lua_run", + 1, + "yes", + "ERROR", + { + { 1, "error", "[C]", -1 }, + { 2, "err", "lua_run", 1 }, + { 3, "unknown", "lua_run", 1 } + } + }; + if( !test_parsed_error_with_stacktrace( error4, control_parsed_error4 ) ) + { + printf( "Failed on test case 4!\n" ); + return 5; + } + + const std::string error5 = + "\n" + "[ERROR] lua_run:1: '=' expected near ''\n" + "\n"; + const common::ParsedErrorWithStackTrace control_parsed_error5 = + { + "lua_run", + 1, + "'=' expected near ''", + "ERROR", + { } + }; + if( !test_parsed_error_with_stacktrace( error5, control_parsed_error5 ) ) + { + printf( "Failed on test case 5!\n" ); + return 5; + } + + printf( "Successfully ran all test cases!\n" ); + return 0; +}