
BugGrabber and New Blizzard handler interface in Retail (error forwarding feature request)
2072 opened this issue ยท 8 comments
Describe the problem
This is for BugGrabber (I couldn't find its repository)
In the retail version of wow there is a new internal interface to manage Lua errors with logging and stuff.
See below (code from 11.1.059538 in _retail_\BlizzardInterfaceCode\Interface\AddOns\Blizzard_ScriptError\Blizzard_ScriptError.lua
)
local function GetErrorData()
-- Example of how debug stack level is calculated
-- Current stack: [1, 2, 3, 4, 5] (current function is at 1, total current height is 5)
-- Stack at time of error: [1, 2] (these are currently now index 4 and 5, but at the time of error the stack height is 2)
-- To calcuate the level to debug (4): curentStackHeight - (errorStackHeight - 1) = 5 - (2 - 1) = 4
local currentStackHeight = GetCallstackHeight();
local errorCallStackHeight = GetErrorCallstackHeight();
local errorStackOffset = errorCallStackHeight and (errorCallStackHeight - 1);
local debugStackLevel = currentStackHeight - (errorStackOffset or 0);
local skipFunctionsAndUserdata = true;
local stack = debugstack(debugStackLevel);
local locals = debuglocals(debugStackLevel, skipFunctionsAndUserdata);
locals = string.gsub(locals, "|([kK])", "%1");
return stack, locals;
end
local warningHandlers = {};
local errorHandlers = {};
function AddLuaWarningHandler(handler)
assert(issecure());
table.insert(warningHandlers, handler);
end
function AddLuaErrorHandler(handler)
assert(issecure());
table.insert(errorHandlers, handler);
end
function HandleLuaWarning(warnType, warningMessage)
local stack, locals = GetErrorData();
C_Log.LogMessage(Enum.LogPriority.Warning, string.format("Lua Warning: message=%s", warningMessage));
for index, handler in ipairs(warningHandlers) do
pcall(handler, warnType, warningMessage, stack, locals);
end
end
local function HandleLuaError(errorMessage)
local stack, locals = GetErrorData();
local formattedMessage = string.format("Lua Error: %s\n%s", errorMessage, stack);
addframetext(formattedMessage);
C_Log.LogMessage(Enum.LogPriority.Error, formattedMessage);
for index, handler in ipairs(errorHandlers) do
pcall(handler, errorMessage, stack, locals);
end
if ProcessExceptionClient then
ProcessExceptionClient(string.format("%sLocals: %s", errorMessage or "", locals or ""), errorMessage);
end
end
seterrorhandler(HandleLuaError);
As you can see there is internal logging involved and stuff and they made their handler local so it's not possible to call it manually.
In Decursive which is using the embedable version of Buggrabber I used to forward non-Decursive errors to the original handler, so that users can see them if they don't have BugSack but now having Decursive without BugSack means that no other error can be seen by the user or Blizzard since they now have internal logging it seems...
It would be wise if Buggrabber could forward to the original handler instead of keeping it all for itself.
At the very least could you add an API to get the original error handler back? So I can use it to forward them from Decursive?
What steps will reproduce the problem?
N/A
Consider attaching a screenshot below to help describe your issue (Attach directly, do not link to other websites)
N/A
What version of the addon are you using? (Stating 'latest' is not useful)
v11.0.2 BugGrabber and v11.0.3 BugSack
Do you have an error log of what happened?
N/A
Any additional information? (example: WoW language if not English)
N/A
If you want a reference to the original error handler, you can do this.
BugGrabber.lua (around line 255 or so.) call geterrorhandler()
to get a reference to the current error handler (the local HandleLuaError
).
-- Error handler
local grabError
local originalErrorHandler = geterrorhandler()
function addon:GetOriginalErrorHandler() return originalErrorHandler end
function addon:SendToOriginalErrorHandler(errorMessage) originalErrorHandler(errorMessage) end
Then you can call BugGrabber:GetOriginalErrorHandler()("your error text")
to pass an error to the original error handler. Or you can do BugGrabber:SendToOriginalErrorHandler("your error text")
to do the same thing without the extra function call.
This is not a "perfect" solution but it works for what you want.
@ChrisKader in what version of Buggrabber do you see this? There is no such code in the current version.
edit, you mean @funkydude could do this, yes indeed that would be a better than nothing solution.
Thanks for the suggestion! I just save the original error handler before loading the embeded version of Buggrabber. I'll need to remember to disable that once it's fixed here.
I'm not seeing what needs fixed here, or I don't understand the issue you have. I don't think the original intention for allowing embedding was ever for addons to try to forward errors to the Blizzard handler.
If this was the original intention, we'd be hard disabling the embedded version if the user is using the Blizz one (just like we do if the user has BugGrabber proper installed), and we don't.
Realistically I don't think anyone is actually using the Blizz one as I cannot think of anything more intrusive than error windows spamming you during a boss fight.
Ok let me try to explain this in more details then.
It seems that Blizzard is now using a more advanced error handling mechanism with advanced logging that they probably use internally to find out about their own issues and maybe do some QA.
The code I posted above also shows that they do support multiple error handlers now with AddLuaErrorHandler
and AddLuaWarningHandler
.
The Blizzard Lua error popup has been disabled by default for several years now (the only way to turn it back on is to change the ScriptErrors
cvar)
So my idea is that instead of depriving Blizzard from this feedback whenever Buggrabber or an add-on embedding it is installed, we could just forward (given that the popup is disabled it won't annoy the user and give the feedback they want to Blizzard).
I use the embedded version of Buggrabber in Decursive to create debug reports for my users to send me but, if BugSack is not installed I also forward the other errors (not related to Decursive) to the original Blizzard handler so as not to break the original behavior or selfishly hide all other Lua errors (I grab my errors and let the other errors behave like they would have if Decursive had not been installed).
Anyway I've implemented a workaround in Decursive (where I save the original handler before loading Buggrabber) but I thought it would have been better if this was the default behavior. (Since Blizzard seems to care a little more about Lua errors now and that they might be annoyed to lose this feedback whenever certain add-ons are installed).
Note that this is only true for WoW retail, they still use the old simple handler in WoW classic (but the popup is also disabled by default).
given that the popup is disabled it won't annoy the user and give the feedback they want to Blizzard
Since Blizzard seems to care a little more about Lua errors now and that they might be annoyed to lose this feedback whenever certain add-ons are installed
What feedback? Blizzard doesn't care at all about errors outside of their addons.
I use the embedded version of Buggrabber in Decursive to create debug reports for my users to send me but, if BugSack is not installed I also forward the other errors
I understand the situation you are in, but from my perspective no one cares at all about errors. The only reason they are willing to install BugSack is it can collect them in a non-intrusive manner. If it didn't it would be instantly deleted.
On PTR only AddLuaErrorHandler
remains and it is only callable from secure code (AddLuaWarningHandler
is also only usable with secure code but also does not exist on PTR).
I mainly was wanting BugGrabber to not modify seterrorhandler
while also preserving the original return from geterrorhandler
. BugGrabber could hook seterrorhandler
so that captures when new error handlers are trying to be set and set its own again or some other kind of functionality (keeping track of each call to seterrorhandler
and maybe calling each function sent in a chain versus only having one, for example). That way the original return from geterrorhandler
(The error handler created by Blizzard in Blizzard_ScriptError.lua
) can still be called allowing for blizzard addon logging to be handle the same. An addon like BugSack can handle hiding the defaut error frame, if desired.
BugGrabber could also allow addons to register themselves to pass a function that "reviews errors" sent to the display UI to determine if it should be sent or handled differently.