From 7eeb087ac4b1700ff6cfe84719a40bdaaa245f16 Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 07:51:35 +0300 Subject: [PATCH 1/6] Fix error stack handling --- lua/gluatest/runner/helpers.lua | 64 +++++++++++++++++++++------------ lua/gluatest/runner/runner.lua | 4 +-- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index 9b0c832..32d9edf 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -128,12 +128,12 @@ local function makeTestEnv() return env, cleanup end -local function getLocals( level ) +local function getLocals( thread, level ) local locals = {} local i = 1 while true do - local name, value = debug.getlocal( level, i ) + local name, value = debug.getlocal( thread, level, i ) if name == nil then break end if name ~= "(*temporary)" then table.insert( locals, { name, value == nil and "nil" or value } ) @@ -144,28 +144,35 @@ local function getLocals( level ) return locals end --- FIXME: There has to be a better way to do this -local function findStackInfo() +-- OLD: FIXME: There has to be a better way to do this +-- NEW: Fixed by srlion :) +local function findStackInfo( thread ) -- Step up through the stacks to find the error we care about - - for stack = 1, 12 do - local info = debug.getinfo( stack, "lnS" ) - if not info then break end - - local emptyName = #info.namewhat == 0 - local notGluatest = not string.match( info.short_src, "/lua/gluatest/" ) - - if emptyName and notGluatest then - return stack, info + local stack, lastInfo + for i = 1, 20 do + local info = debug.traceback( thread, "", i ) + info, line = string.match( info, "(.+):(.+): in function" ) + if line then + stack = i + lastInfo = { + short_src = info:sub( 20 ), -- Removes "stack traceback:" from the start + currentline = tonumber( line ) or line + } + else + break end end -- This should never happen!! - ErrorNoHaltWithStack( "Could not find stack info! This should never happen - please report this!" ) - return 2, debug.getinfo( 2, "lnS" ) + if not stack then + ErrorNoHaltWithStack( "Could not find stack info! This should never happen - please report this!" ) + return 2, debug.getinfo( 2, "lnS" ) + end + + return stack, lastInfo end -function Helpers.FailCallback( reason ) +function Helpers.FailCallback( thread, reason ) if reason == "" then ErrorNoHaltWithStack( "Received empty error reason in failCallback- ignoring " ) return @@ -183,14 +190,15 @@ function Helpers.FailCallback( reason ) local cleanReason = table.concat( reasonSpl, ": ", 2, #reasonSpl ) - local level, info = findStackInfo() - local locals = getLocals( level ) + local level, info = findStackInfo( thread ) + local locals = getLocals( thread, level ) return { reason = cleanReason, sourceFile = info.short_src, lineNumber = info.currentline, - locals = locals + locals = locals, + thread = thread } end @@ -218,7 +226,7 @@ function Helpers.MakeAsyncEnv( done, fail, onFailedExpectation ) built.to.expected = function( ... ) if recordedFailure then return end - local _, errInfo = xpcall( expected, Helpers.FailCallback, ... ) + local _, errInfo = Helpers.SafeRunFunction( expected, ... ) onFailedExpectation( errInfo ) recordedFailure = true @@ -259,7 +267,7 @@ function Helpers.SafeRunWithEnv( defaultEnv, before, func, state ) setfenv( before, defaultEnv ) setfenv( func, testEnv ) - local success, errInfo = xpcall( func, Helpers.FailCallback, state ) + local success, errInfo = Helpers.SafeRunFunction( func ) setfenv( func, defaultEnv ) cleanup() @@ -272,6 +280,18 @@ function Helpers.SafeRunWithEnv( defaultEnv, before, func, state ) return success, errInfo end +function Helpers.SafeRunFunction( func, ... ) + local co = coroutine.create( func ) + local success, err = coroutine.resume( co, ... ) + + local errInfo + if not success then + errInfo = Helpers.FailCallback( co, err ) + end + + return success, errInfo +end + function Helpers.CreateCaseState( testGroupState ) return setmetatable( {}, { __index = function( self, idx ) diff --git a/lua/gluatest/runner/runner.lua b/lua/gluatest/runner/runner.lua index fec39a9..2cedcce 100644 --- a/lua/gluatest/runner/runner.lua +++ b/lua/gluatest/runner/runner.lua @@ -1,5 +1,5 @@ local Helpers = include( "gluatest/runner/helpers.lua" ) -local FailCallback = Helpers.FailCallback +local SafeRunFunction = Helpers.SafeRunFunction local MakeAsyncEnv = Helpers.MakeAsyncEnv local SafeRunWithEnv = Helpers.SafeRunWithEnv local CreateCaseState = Helpers.CreateCaseState @@ -244,7 +244,7 @@ return function( allTestGroups ) setfenv( testGroup.beforeEach, defaultEnv ) setfenv( case.func, asyncEnv ) - local success, errInfo = xpcall( case.func, FailCallback, case.state ) + local success, errInfo = SafeRunFunction( case.func, case.state ) -- If the test failed while calling it -- (Async expectation failures handled in asyncEnv.expect) From 1eb4683d82236ed6ee23a0355cc6c637eddb1afb Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 11:26:58 +0300 Subject: [PATCH 2/6] Better error handling when there is no stack at all --- lua/gluatest/runner/helpers.lua | 40 ++++++++++++++++----------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index 32d9edf..c9cba89 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -146,33 +146,31 @@ end -- OLD: FIXME: There has to be a better way to do this -- NEW: Fixed by srlion :) -local function findStackInfo( thread ) - -- Step up through the stacks to find the error we care about - local stack, lastInfo - for i = 1, 20 do - local info = debug.traceback( thread, "", i ) - info, line = string.match( info, "(.+):(.+): in function" ) - if line then - stack = i - lastInfo = { - short_src = info:sub( 20 ), -- Removes "stack traceback:" from the start - currentline = tonumber( line ) or line - } - else +local function findStackInfo( thread, caseFunc ) + -- Step through the stack to find the first non-C function call. If no stack is found for the called function, it will point to + -- the wrapper function instead. This is because we wrap the function inside another one to ensure there is always at least one Lua stack trace. + local lastInfoLevel, lastInfo + for level = 0, 20 do + local info = debug.getinfo( thread, level, "nSl" ) + if info and info.short_src ~= "[C]" then + lastInfoLevel, lastInfo = level, info break end end - -- This should never happen!! - if not stack then - ErrorNoHaltWithStack( "Could not find stack info! This should never happen - please report this!" ) - return 2, debug.getinfo( 2, "lnS" ) + if not lastInfoLevel then + ErrorNoHalt( + "Failed to get a stack, probably returning a function that errored! " .. + "For example, 'return error('!')'\n" + ) + lastInfoLevel, lastInfo = 1, debug.getinfo( caseFunc, "nSl" ) + lastInfo.currentline = lastInfo.linedefined -- currentline will be -1, so we will point it to the line where the function was defined end - return stack, lastInfo + return lastInfoLevel, lastInfo end -function Helpers.FailCallback( thread, reason ) +function Helpers.FailCallback( thread, caseFunc, reason ) if reason == "" then ErrorNoHaltWithStack( "Received empty error reason in failCallback- ignoring " ) return @@ -190,7 +188,7 @@ function Helpers.FailCallback( thread, reason ) local cleanReason = table.concat( reasonSpl, ": ", 2, #reasonSpl ) - local level, info = findStackInfo( thread ) + local level, info = findStackInfo( thread, caseFunc ) local locals = getLocals( thread, level ) return { @@ -286,7 +284,7 @@ function Helpers.SafeRunFunction( func, ... ) local errInfo if not success then - errInfo = Helpers.FailCallback( co, err ) + errInfo = Helpers.FailCallback( co, func, err ) end return success, errInfo From ca3d13b260f4f9543632e3b7fc12845ddd2d42e0 Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 11:38:16 +0300 Subject: [PATCH 3/6] Handle getLocals inside findStackInfo --- lua/gluatest/runner/helpers.lua | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index c9cba89..cc63f46 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -147,8 +147,8 @@ end -- OLD: FIXME: There has to be a better way to do this -- NEW: Fixed by srlion :) local function findStackInfo( thread, caseFunc ) - -- Step through the stack to find the first non-C function call. If no stack is found for the called function, it will point to - -- the wrapper function instead. This is because we wrap the function inside another one to ensure there is always at least one Lua stack trace. + -- Step through the stack to find the first non-C function call. If no stack is found for the called function, it will point to case function. This case will only happen + -- when the function is tail called, and the error is thrown from the tail called function. local lastInfoLevel, lastInfo for level = 0, 20 do local info = debug.getinfo( thread, level, "nSl" ) @@ -158,16 +158,21 @@ local function findStackInfo( thread, caseFunc ) end end + local locals if not lastInfoLevel then ErrorNoHalt( "Failed to get a stack, probably returning a function that errored! " .. "For example, 'return error('!')'\n" ) - lastInfoLevel, lastInfo = 1, debug.getinfo( caseFunc, "nSl" ) + lastInfo = debug.getinfo( caseFunc, "nSl" ) lastInfo.currentline = lastInfo.linedefined -- currentline will be -1, so we will point it to the line where the function was defined + + locals = {} -- We can't get locals from a function that has tail call returns + else + locals = getLocals( thread, lastInfoLevel ) end - return lastInfoLevel, lastInfo + return lastInfo, locals end function Helpers.FailCallback( thread, caseFunc, reason ) @@ -188,8 +193,7 @@ function Helpers.FailCallback( thread, caseFunc, reason ) local cleanReason = table.concat( reasonSpl, ": ", 2, #reasonSpl ) - local level, info = findStackInfo( thread, caseFunc ) - local locals = getLocals( thread, level ) + local info, locals = findStackInfo( thread, caseFunc ) return { reason = cleanReason, From 27a1815b40f0a6dac68fe6ec658070d8537ea340 Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 12:13:32 +0300 Subject: [PATCH 4/6] Handle a case when calling a nil value, there won't be there a currentline in from debug.getinfo --- lua/gluatest/runner/helpers.lua | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index cc63f46..5a2955e 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -146,7 +146,7 @@ end -- OLD: FIXME: There has to be a better way to do this -- NEW: Fixed by srlion :) -local function findStackInfo( thread, caseFunc ) +local function findStackInfo( thread, caseFunc, reason ) -- Step through the stack to find the first non-C function call. If no stack is found for the called function, it will point to case function. This case will only happen -- when the function is tail called, and the error is thrown from the tail called function. local lastInfoLevel, lastInfo @@ -169,6 +169,15 @@ local function findStackInfo( thread, caseFunc ) locals = {} -- We can't get locals from a function that has tail call returns else + -- We got info about the error, but if the error was thrown from calling a nil value 'thisdoesntexist()', we can't get the currentline (executing line) as it was a nil value! + -- Thankfully, the error message will contain the line number, so we can extract it from there. + if lastInfo.currentline == -1 then + local line = string.match( reason, ":(%d+):" ) + if line then + lastInfo.currentline = tonumber( line ) + end + end + locals = getLocals( thread, lastInfoLevel ) end @@ -193,7 +202,7 @@ function Helpers.FailCallback( thread, caseFunc, reason ) local cleanReason = table.concat( reasonSpl, ": ", 2, #reasonSpl ) - local info, locals = findStackInfo( thread, caseFunc ) + local info, locals = findStackInfo( thread, caseFunc, reason ) return { reason = cleanReason, From 51d0575e84ea7753e3a126c7c8db793ef5fccf77 Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 12:30:37 +0300 Subject: [PATCH 5/6] Forgot to supply state to SafeRunFunction --- lua/gluatest/runner/helpers.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index 5a2955e..57dbaad 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -278,7 +278,7 @@ function Helpers.SafeRunWithEnv( defaultEnv, before, func, state ) setfenv( before, defaultEnv ) setfenv( func, testEnv ) - local success, errInfo = Helpers.SafeRunFunction( func ) + local success, errInfo = Helpers.SafeRunFunction( func, state ) setfenv( func, defaultEnv ) cleanup() From d91eb40df213b49bb168d21f5ffdfe9d8d879054 Mon Sep 17 00:00:00 2001 From: Srlion Date: Tue, 8 Oct 2024 14:04:42 +0300 Subject: [PATCH 6/6] Add check to make sure that the error is not from gluatest, woopsie --- lua/gluatest/runner/helpers.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/gluatest/runner/helpers.lua b/lua/gluatest/runner/helpers.lua index 57dbaad..d29acae 100644 --- a/lua/gluatest/runner/helpers.lua +++ b/lua/gluatest/runner/helpers.lua @@ -152,7 +152,7 @@ local function findStackInfo( thread, caseFunc, reason ) local lastInfoLevel, lastInfo for level = 0, 20 do local info = debug.getinfo( thread, level, "nSl" ) - if info and info.short_src ~= "[C]" then + if info and info.short_src ~= "[C]" and not string.match( info.short_src, "/lua/gluatest/" ) then lastInfoLevel, lastInfo = level, info break end