From 4090fcab32051415abad735e6e0ec7443d04d5c1 Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Tue, 13 Aug 2024 14:06:46 +0200 Subject: [PATCH] refactor(standard): error handling (#4516) * refactor(standard): error handling * improve annos * comments * improve check * add a todo * nil check --- components/match2/commons/display_util.lua | 23 +- .../match2/commons/match_group_input.lua | 8 +- .../opponent/commons/player_ext_custom.lua | 3 +- standard/error.lua | 91 ++++++-- standard/error_display.lua | 6 +- standard/error_ext.lua | 14 +- standard/feature_flag.lua | 2 +- standard/logic.lua | 16 +- standard/result_or_error.lua | 196 +++++++----------- 9 files changed, 194 insertions(+), 165 deletions(-) diff --git a/components/match2/commons/display_util.lua b/components/match2/commons/display_util.lua index bb3a9b69437..00c9961a910 100644 --- a/components/match2/commons/display_util.lua +++ b/components/match2/commons/display_util.lua @@ -9,7 +9,7 @@ local Array = require('Module:Array') local FeatureFlag = require('Module:FeatureFlag') local FnUtil = require('Module:FnUtil') -local ResultOrError = require('Module:ResultOrError') +local Logic = require('Module:Logic') local TypeUtil = require('Module:TypeUtil') local DisplayUtil = {propTypes = {}, types = {}} @@ -54,18 +54,15 @@ end ---@param props table ---@return Html function DisplayUtil.TryPureComponent(Component, props) - local resultOrError = ResultOrError.try(function() return Component(props) end) - if resultOrError:isResult() then - ---@cast resultOrError Result - return resultOrError:get() - else - ---@cast resultOrError Error - return mw.html.create('div') - :tag('strong'):addClass('error') - :tag('span'):addClass('scribunto-error') - :wikitext(resultOrError:getErrorJson() .. '.') - :allDone() - end + return Logic.try(function() return Component(props) end) + :catch(function(error) + return mw.html.create('div') + :tag('strong'):addClass('error') + :tag('span'):addClass('scribunto-error') + :wikitext(error:getErrorJson() .. '.') + :allDone() + end) + :get() end ---@alias OverflowModes 'ellipsis'|'wrap'|'hidden' diff --git a/components/match2/commons/match_group_input.lua b/components/match2/commons/match_group_input.lua index 256e244ca84..603f9c60b64 100644 --- a/components/match2/commons/match_group_input.lua +++ b/components/match2/commons/match_group_input.lua @@ -186,12 +186,12 @@ function MatchGroupInput.readBracket(bracketId, args, options) local bracketDatasById = Logic.try(function() return MatchGroupInput._fetchBracketDatas(templateId, bracketId) end) - :catch(function(message) - if String.endsWith(message, 'does not exist') then - table.insert(warnings, message .. ' (Maybe [[Template:' .. templateId .. ']] needs to be purged?)') + :catch(function(error) + if String.endsWith(error.message, 'does not exist') then + table.insert(warnings, error.message .. ' (Maybe [[Template:' .. templateId .. ']] needs to be purged?)') return {} else - error(message) + error(error.message) end end) :get() diff --git a/components/opponent/commons/player_ext_custom.lua b/components/opponent/commons/player_ext_custom.lua index fa574224228..ce7f4e27d5a 100644 --- a/components/opponent/commons/player_ext_custom.lua +++ b/components/opponent/commons/player_ext_custom.lua @@ -7,5 +7,6 @@ -- local Lua = require('Module:Lua') +local PlayerExt = Lua.import('Module:Player/Ext') -return Lua.import('Module:Player/Ext') +return PlayerExt diff --git a/standard/error.lua b/standard/error.lua index 3c459c02e6a..5af381bd78d 100644 --- a/standard/error.lua +++ b/standard/error.lua @@ -6,16 +6,18 @@ -- Please see https://github.com/Liquipedia/Lua-Modules to contribute -- ----@class error ----@field childErrors? error[] ----@field header string? ----@field innerError any ----@field message string ----@field originalErrors? error[] ----@field stacks? string[] ----@field is_a? function - +local Array = require('Module:Array') local Class = require('Module:Class') +local Json = require('Module:Json') +local Page = require('Module:Page') +local String = require('Module:StringUtils') +local Table = require('Module:Table') + +local FILTERED_ERROR_STACK_ITEMS = { + '^Module:ResultOrError:%d+: in function $', + '^%[C%]: in function \'xpcall\'$', + '^Module:ResultOrError:%d+: in function \'try\'$', +} --[[ A minimal error class, whose purpose is to allow additional fields to be @@ -47,9 +49,14 @@ preamble-like text here to give some context to the error. error.noStack: Disables the stack trace ]] ----@class Error +---@class Error: BaseClass ---@operator call(string|table|nil|any):Error ----@field message string? +---@field message string +---@field childErrors? Error[] +---@field header string? +---@field innerError any +---@field originalErrors? Error[] +---@field stacks? string[] local Error = Class.new(function(self, any) -- Normalize the various ways an error can be thrown if type(any) == 'string' then @@ -67,7 +74,7 @@ local Error = Class.new(function(self, any) self.message = self.message or 'Unknown error' end) ----@param error error +---@param error Error ---@return boolean function Error.isError(error) return type(error) == 'table' @@ -80,4 +87,64 @@ function Error:__tostring() return self.message end +--- TODO: Move to Error Display +---Builds a JSON string for use by `liquipedia.customLuaErrors` JS module with `error()`. +---@return string +function Error:getErrorJson() + local stackTrace = {} + + local processStackFrame = function(frame, frameIndex) + if frameIndex == 1 and frame == '[C]: ?' then + return + end + + local stackEntry = {content = frame} + local frameSplit = mw.text.split(frame, ':', true) + if (frameSplit[1] == '[C]' or frameSplit[1] == '(tail call)') then + stackEntry.prefix = frameSplit[1] + stackEntry.content = mw.text.trim(table.concat(frameSplit, ':', 2)) + elseif frameSplit[1]:sub(1, 3) == 'mw.' then + stackEntry.prefix = table.concat(frameSplit, ':', 1, 2) + stackEntry.content = table.concat(frameSplit, ':', 3) + elseif frameSplit[1] == 'Module' then + local wiki = not Page.exists(table.concat(frameSplit, ':', 1, 2)) and 'commons' + or mw.text.split(mw.title.getCurrentTitle():canonicalUrl(), '/', true)[4] or 'commons' + stackEntry.link = {wiki = wiki, title = table.concat(frameSplit, ':', 1, 2), ln = frameSplit[3]} + stackEntry.prefix = table.concat(frameSplit, ':', 1, 3) + stackEntry.content = table.concat(frameSplit, ':', 4) + end + + table.insert(stackTrace, stackEntry) + end + + Array.forEach(self.stacks or {}, function(stack) + local stackFrames = mw.text.split(stack, '\n') + stackFrames = Array.filter( + Array.map( + Array.sub(stackFrames, 2, #stackFrames), + function(frame) return String.trim(frame) end + ), + function(frame) return not Table.any(FILTERED_ERROR_STACK_ITEMS, function(_, filter) + return string.find(frame, filter) ~= nil + end) end + ) + Array.forEach(stackFrames, processStackFrame) + end) + + local errorSplit = mw.text.split(self.message, ':', true) + local errorText + if #errorSplit == 4 then + errorText = string.format('Lua error in %s:%s at line %s:%s.', unpack(errorSplit)) + elseif #errorSplit > 4 then + errorText = string.format('Lua error in %s:%s at line %s:%s', unpack(Array.sub(errorSplit, 1, 4))) + errorText = errorText .. ':' .. table.concat(Array.sub(errorSplit, 5), ':') .. '.' + else + errorText = string.format('Lua error: %s.', self.message) + end + return Json.stringify({ + errorShort = errorText, + stackTrace = stackTrace, + }, {asArray = true}) +end + return Error diff --git a/standard/error_display.lua b/standard/error_display.lua index c0f4e6f935d..f74b207116a 100644 --- a/standard/error_display.lua +++ b/standard/error_display.lua @@ -12,7 +12,7 @@ local TypeUtil = require('Module:TypeUtil') local ErrorDisplay = {types = {}, propTypes = {}} ----@param props {limit: integer?, errors: error[]} +---@param props {limit: integer?, errors: Error[]} ---@return Html function ErrorDisplay.ErrorList(props) local defaultLimit = 5 @@ -80,7 +80,7 @@ function ErrorDisplay.Box(props) return div:node(tbl) end ----@param error error +---@param error Error ---@return Html function ErrorDisplay.ErrorBox(error) return ErrorDisplay.Box{ @@ -90,7 +90,7 @@ function ErrorDisplay.ErrorBox(error) end ---Shows the message and stack trace of a lua error. Suitable for use in a popup. ----@param error error +---@param error Error ---@return Html function ErrorDisplay.ErrorDetails(error) local errorDetailsNode = mw.html.create('div'):addClass('error-details') diff --git a/standard/error_ext.lua b/standard/error_ext.lua index 4312c90967a..394c7f06a2d 100644 --- a/standard/error_ext.lua +++ b/standard/error_ext.lua @@ -18,13 +18,13 @@ local _local_errors = {} local ErrorExt = {} ----@param error error +---@param error Error function ErrorExt.logAndStash(error) ErrorExt.Stash.add(error) ErrorExt.log(error) end ----@param error error +---@param error Error function ErrorExt.log(error) mw.log(ErrorExt.makeFullDetails(error)) mw.log() @@ -37,7 +37,7 @@ local function tableOrEmpty(tbl) return type(tbl) == 'table' and tbl or {} end ----@param error error +---@param error Error ---@return string function ErrorExt.makeFullDetails(error) local parts = Array.extend( @@ -51,7 +51,7 @@ end ---Builds a string for fields not covered by the other functions in this module. ---Returns nil if there are no extra fields. ----@param error error +---@param error Error ---@return string? function ErrorExt.printExtraProps(error) local extraProps = Table.copy(error) @@ -70,7 +70,7 @@ function ErrorExt.printExtraProps(error) end end ----@param error error +---@param error Error ---@return string function ErrorExt.makeFullStackTrace(error) local parts = Array.extend( @@ -91,7 +91,7 @@ local Stash = {} ErrorExt.Stash = Stash ---Adds an Error instance to the local store. ----@param error error +---@param error Error ---@param storeAsPageVar boolean? function Stash.add(error, storeAsPageVar) if storeAsPageVar then @@ -104,7 +104,7 @@ function Stash.add(error, storeAsPageVar) end ---Returns all errors (locally and from page variables), and clears the store. ----@return error[] +---@return Error[] function Stash.retrieve() local errors = {} diff --git a/standard/feature_flag.lua b/standard/feature_flag.lua index dcbbb89bce2..90e00512ca3 100644 --- a/standard/feature_flag.lua +++ b/standard/feature_flag.lua @@ -78,7 +78,7 @@ end ---@generic V ---@param flags? {[string]: boolean} ---@param f fun(): V ----@return V|error +---@return V|Error function FeatureFlag.with(flags, f) if Table.isEmpty(flags) then return f() diff --git a/standard/logic.lua b/standard/logic.lua index 8029f5ad556..23cbb30c54f 100644 --- a/standard/logic.lua +++ b/standard/logic.lua @@ -143,22 +143,24 @@ function Logic.tryCatch(try, catch) end ---@param f function ----@return any? +---@return RoEResult|RoEError function Logic.try(f) - return require('Module:ResultOrError').try(f) + local ResultOrError = require('Module:ResultOrError') + return ResultOrError.try(f) end ---Returns the result of a function if successful. Otherwise it returns the result of the second function. ---If the first function fails, its error is logged to the console and stashed away for display. ---@param f fun(): any ----@param other? fun(error: error): any ----@param makeError? fun(error: error): error function that allows customizing Error instance being logged and stashed. +---@param other? fun(error: Error): any +---@param makeError? fun(error: Error): Error function that allows customizing Error instance being logged and stashed. ---@return any? function Logic.tryOrElseLog(f, other, makeError) return Logic.try(f) :catch(function(error) - if type(error) == 'string' then - error = require('Module:Error')(error) + local Error = require('Module:Error') + if not Error.isError(error) then + error = Error(error) end error.header = 'Error occured while calling a function: (caught by Logic.tryOrElseLog)' @@ -178,7 +180,7 @@ end ---Returns the result of a function if successful. Otherwise it returns nil. ---If the first function fails, its error is logged to the console and stashed away for display. ---@param f fun(): any ----@param makeError? fun(error: error): error function that allows customizing Error instance being logged and stashed. +---@param makeError? fun(error: Error): Error function that allows customizing Error instance being logged and stashed. ---@return function function Logic.wrapTryOrLog(f, makeError) return function(...) diff --git a/standard/result_or_error.lua b/standard/result_or_error.lua index 55a8a3d1326..32a492c1565 100644 --- a/standard/result_or_error.lua +++ b/standard/result_or_error.lua @@ -6,26 +6,19 @@ -- Please see https://github.com/Liquipedia/Lua-Modules to contribute -- -local Array = require('Module:Array') local Class = require('Module:Class') -local Json = require('Module:Json') -local Page = require('Module:Page') -local String = require('Module:StringUtils') -local Table = require('Module:Table') - -local FILTERED_ERROR_STACK_ITEMS = { - '^Module:ResultOrError:%d+: in function $', - '^%[C%]: in function \'xpcall\'$', - '^Module:ResultOrError:%d+: in function \'try\'$', -} +local Error = require('Module:Error') --[[ A structurally typed, immutable class that represents either a result or an error. Used for representing the outcome of a function that can throw. +ResultOrError expects functions that return one value. Additional return values +are ignored. + Usage: ``` -local socketOrError = ResultOrError.try(function() +local socketOrError = ReltOrError.try(function() return socketlib.open() end) local parsedText = socketOrError @@ -46,25 +39,37 @@ local ResultOrError = Class.new(function(self) end) -- Applies a function to the result, or handles the error +---@param f? fun(any: any): any +---@param onError? fun(error: Error?): any +---@return ResultOrError function ResultOrError:map(f, onError) error('Abstract method') end -- Returns the result or rethrows the error +---@return any function ResultOrError:get() error('Abstract method') end +---@param onError fun(error: Error?): any +---@return ResultOrError function ResultOrError:catch(onError) return self:map(nil, onError) end +---@param f fun(error: Error?): any +---@return ResultOrError function ResultOrError:finally(f) local ret = self:map(f, f) - ---@cast ret -nil - return ret.error and ret or self + if ret:isError() then + return ret + end + return self end +---@return boolean function ResultOrError:isResult() return self:is_a(ResultOrError.Result) end +---@return boolean function ResultOrError:isError() return self:is_a(ResultOrError.Error) end @@ -72,107 +77,46 @@ end --[[ Result case ]] ----@class Result: ResultOrError +---@class RoEResult: ResultOrError ---@field result any -local Result = Class.new(ResultOrError, function(self, result) +ResultOrError.Result = Class.new(ResultOrError, function(self, result) self.result = result end) -function Result:map(f, _) +---@param f? fun(any: any): any +---@param _ any +---@return RoEResult|RoEError +function ResultOrError.Result:map(f, _) return f and ResultOrError.try(function() return f(self.result) end) or self end -function Result:get() +---@return any +function ResultOrError.Result:get() return self.result end ---[[ -Error case. - -The stacks argument is the stack traces for the error, so that if an error -handler throws, then the stack trace of the error handler error can be a -continuation of the stack trace of the error that was handled (and so on). -This allows error handlers to rethrow the original error without losing the -stack trace, and is needed to implement :finally(). -]] ----@class Error: ResultOrError ----@field error string? ----@field stacks string[]? -local Error = Class.new(ResultOrError, function(self, error, stacks) +---Error case. The error field is an Error instance. +---@class RoEError: ResultOrError +---@field error Error +ResultOrError.Error = Class.new(ResultOrError, function(self, error) self.error = error - self.stacks = stacks end) -function Error:map(_, onError) +---@param _ any +---@param onError? fun(error: Error?): any +---@return RoEResult|RoEError +function ResultOrError.Error:map(_, onError) return onError - and ResultOrError.try(function() return onError(self.error) end, self.stacks) + and ResultOrError.try(function() return onError(self.error) end, self.error) or ResultOrError.Result() end ----Builds a JSON string for use by `liquipedia.customLuaErrors` JS module with `error()`. ----@return string -function Error:getErrorJson() - local stackTrace = {} - - local processStackFrame = function(frame, frameIndex) - if frameIndex == 1 and frame == '[C]: ?' then - return - end - - local stackEntry = {content = frame} - local frameSplit = mw.text.split(frame, ':', true) - if (frameSplit[1] == '[C]' or frameSplit[1] == '(tail call)') then - stackEntry.prefix = frameSplit[1] - stackEntry.content = mw.text.trim(table.concat(frameSplit, ':', 2)) - elseif frameSplit[1]:sub(1, 3) == 'mw.' then - stackEntry.prefix = table.concat(frameSplit, ':', 1, 2) - stackEntry.content = table.concat(frameSplit, ':', 3) - elseif frameSplit[1] == 'Module' then - local wiki = not Page.exists(table.concat(frameSplit, ':', 1, 2)) and 'commons' - or mw.text.split(mw.title.getCurrentTitle():canonicalUrl(), '/', true)[4] or 'commons' - stackEntry.link = {wiki = wiki, title = table.concat(frameSplit, ':', 1, 2), ln = frameSplit[3]} - stackEntry.prefix = table.concat(frameSplit, ':', 1, 3) - stackEntry.content = table.concat(frameSplit, ':', 4) - end - - table.insert(stackTrace, stackEntry) - end - - Array.forEach(self.stacks, function(stack) - local stackFrames = mw.text.split(stack, '\n') - stackFrames = Array.filter( - Array.map( - Array.sub(stackFrames, 2, #stackFrames), - function(frame) return String.trim(frame) end - ), - function(frame) return not Table.any(FILTERED_ERROR_STACK_ITEMS, function(_, filter) - return string.find(frame, filter) ~= nil - end) end - ) - Array.forEach(stackFrames, processStackFrame) - end) - - local errorSplit = mw.text.split(self.error, ':', true) - local errorText - if #errorSplit == 4 then - errorText = string.format('Lua error in %s:%s at line %s:%s.', unpack(errorSplit)) - elseif #errorSplit > 4 then - errorText = string.format('Lua error in %s:%s at line %s:%s', unpack(Array.sub(errorSplit, 1, 4))) - errorText = errorText .. ':' .. table.concat(Array.sub(errorSplit, 5), ':') .. '.' - else - errorText = string.format('Lua error: %s.', self.error) - end - return Json.stringify({ - errorShort = errorText, - stackTrace = stackTrace, - }, {asArray = true}) -end - ---Errors with a JSON string for use by `liquipedia.customLuaErrors` JS module. -function Error:get() - error(self:getErrorJson(), 0) +---@return any +function ResultOrError.Error:get() + error(self.error:getErrorJson(), 0) end --[[ @@ -180,15 +124,14 @@ Invokes a function and places its outcome (result or caught error) in a ResultOrError. If the result is a ResultOrError, then it is flattened, so that a nested ResultOrError is avoided. -Additional stack traces can be attached using the lowerStacks parameter. This -can be used when rethrowing an error to include the stack trace of the existing -error. Errors rethrown in ResultOrError:map() or ResultOrError:catch() will -automatically include both stack traces. +originalError is used when ResultOrError.try is invoking an error handler. It +allows errors thrown in ResultOrError:map() or ResultOrError:catch() to include +stack traces from both the thrown error and the error being handled. ]] ----@param f function ----@param lowerStacks table? ----@return Result|Error -function ResultOrError.try(f, lowerStacks) +---@param f fun(): any +---@param originalError table? +---@return RoEResult|RoEError +function ResultOrError.try(f, originalError) local resultOrError xpcall( function() @@ -198,10 +141,27 @@ function ResultOrError.try(f, lowerStacks) and result:is_a(ResultOrError) resultOrError = isResultOrError and result - or Result(result) + or ResultOrError.Result(result) end, - function(error) - resultOrError = Error(error, Array.extend(debug.traceback(), lowerStacks)) + function(any) + local error = Error.isError(any) and any or Error(any) + + -- Error handler threw a different error than the original error + if originalError and error ~= originalError then + if type(error.originalErrors) ~= 'table' then + error.originalErrors = {} + end + table.insert(error.originalErrors, originalError) + + -- Not an error handler, or error handler rethrow + elseif not error.noStack then + if type(error.stacks) ~= 'table' then + error.stacks = {} + end + table.insert(error.stacks, 1, debug.traceback()) + end + + resultOrError = ResultOrError.Error(error) end ) return resultOrError @@ -214,19 +174,19 @@ that this function swaps the types: it converts an array of ResultOrErrors of values into a ResultOrError of an array of values. ]] ---@param resultOrErrors ResultOrError[] ----@return Result|Error +---@return RoEResult|RoEError function ResultOrError.all(resultOrErrors) local results = {} for _, resultOrError in ipairs(resultOrErrors) do if resultOrError:isResult() then - ---@cast resultOrError Result + ---@cast resultOrError RoEResult table.insert(results, resultOrError.result) else - ---@cast resultOrError Error + ---@cast resultOrError RoEError return resultOrError end end - return Result(results) + return ResultOrError.Result(results) end --[[ @@ -235,22 +195,24 @@ ResultOrError. Otherwise, all input ResultOrErrors are errors, and this aggregates them together and returns a ResultOrError of the aggregate error. ]] ---@param resultOrErrors ResultOrError[] ----@return Result|Error +---@return RoEResult|RoEError function ResultOrError.any(resultOrErrors) local errors = {} for _, resultOrError in ipairs(resultOrErrors) do if resultOrError:isResult() then - ---@cast resultOrError Result + ---@cast resultOrError RoEResult return resultOrError else - ---@cast resultOrError Error + ---@cast resultOrError RoEError table.insert(errors, resultOrError.error) end end - return Error(table.concat(errors, '\n')) + local error = { + childErrors = errors, + message = table.concat(errors, '\n'), + stacks = {debug.traceback()}, + } + return ResultOrError.Error(error) end -ResultOrError.Result = Result -ResultOrError.Error = Error - return ResultOrError