Fix/153 error stacktrace (#154)

* add the error name and message to the stacktrace
* implement Error.captureStackTrace
* set DebugDisableOptimizedBytecode to true for tests
* extract __createError function
* test stacktrace line number in Error
* add tests
* update changelog

.gitignore

@@ -15,9 +15,10 @@ Packages/_Workspace/*
 .DS_Store
 
 # Local IDE settings
-.vscode
+.vscode/*
 .idea/
 sourcemap.json
+!.vscode/launch.sample.json
 
 # Code coverage data files
 **/luacov.*

.vscode/launch.sample.json

@@ -0,0 +1,25 @@
+{
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "type": "roblox-lrdb",
+      "request": "launch",
+      "name": "Luau Tests",
+      "program": "<PATH_TO_ROBLOX_CLI>",
+      "args": [
+        "run",
+        "--load.model",
+        "test-model.project.json",
+        "--debug.on",
+        "--run",
+        "bin/spec.lua",
+        "--fastFlags.allOnLuau",
+        "--fastFlags.overrides",
+        "DebugDisableOptimizedBytecode=true",
+        "--lua.globals=__DEV__=true"
+      ],
+      "cwd": "${workspaceFolder}",
+      "stopOnEntry": true
+    }
+  ]
+}

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Replace custom implementation of `Math.countlz` with the new engine `bit32.countlz` function
 * Fix `indexOf` to be accessible from `String`
 * Refactor structure to a rotriever workspace
+* Fix Error stacktrace to include error name and message
 
 ## 1.0.0
 

bin/ci.sh

@@ -12,6 +12,6 @@ roblox-cli analyze test-model.project.json
 selene modules
 stylua -c modules
 echo "Run tests in DEV"
-roblox-cli run --load.model test-model.project.json --run bin/spec.lua --lua.globals=__DEV__=true
+roblox-cli run --load.model test-model.project.json --run bin/spec.lua --lua.globals=__DEV__=true --fastFlags.overrides DebugDisableOptimizedBytecode=true
 echo "Run tests in release"
-roblox-cli run --load.model model.rbxmx --run bin/spec.lua
+roblox-cli run --load.model model.rbxmx --run bin/spec.lua --fastFlags.overrides DebugDisableOptimizedBytecode=true

foreman.toml

@@ -1,5 +1,5 @@
 [tools]
-rotrieve = { source = "roblox/rotriever", version = "0.5.4" }
+rotrieve = { source = "roblox/rotriever", version = "0.5.5" }
 rojo = { source = "rojo-rbx/rojo", version = "7.2.0" }
 selene = { source = "Kampfkarren/selene", version = "0.20.0" }
 stylua = { source = "JohnnyMorganz/StyLua", version = "=0.14.2" }

modules/luau-polyfill/src/AssertionError/AssertionError.lua

@@ -53,7 +53,9 @@ local process = {
 }
 -- ROBLOX deviation END
 -- ROBLOX TODO START: implement ErrorCaptureStackTrace correctly
-function ErrorCaptureStackTrace(...) end
+function ErrorCaptureStackTrace(err, ...)
+	Error.captureStackTrace(err, ...)
+end
 -- ROBLOX TODO END
 -- ROBLOX TODO START: use real remove colors
 local function removeColors(str)
@@ -560,7 +562,7 @@ function AssertionError.new(options: AssertionErrorOptions): AssertionError
 	self.operator = operator
 	-- end
 	-- ROBLOX deviation END
-	ErrorCaptureStackTrace(self, stackStartFn)
+	ErrorCaptureStackTrace(self, stackStartFn or AssertionError.new)
 	-- Create error message including the error code in the name.
 	--[[
 		ROBLOX deviation: Lua doesn't support 'LuaMemberExpression' as a standalone type

modules/luau-polyfill/src/Error/__tests__/Error.spec.lua

@@ -97,20 +97,208 @@ return function()
 	end)
 
 	it("checks Error stack field", function()
+		local lineNumber = (debug.info(1, "l") :: number) + 1
+		local err = Error("test stack for Error()")
+		local topLineRegExp = RegExp("Error.__tests__\\.Error\\.spec:" .. tostring(lineNumber))
+
+		jestExpect(topLineRegExp:test(err.stack)).toEqual(true)
+
+		local lineNumber2 = (debug.info(1, "l") :: number) + 1
+		local err2 = Error.new("test stack for Error.new()")
+		local topLineRegExp2 = RegExp("Error.__tests__\\.Error\\.spec:" .. tostring(lineNumber2))
+
+		jestExpect(topLineRegExp2:test(err2.stack)).toEqual(true)
+	end)
+
+	it("checks Error stack field contains error message", function()
 		local err = Error("test stack for Error()")
 		local err2 = Error.new("test stack for Error.new()")
 
-		local topLineRegExp = RegExp("^.*Error.__tests__\\.Error\\.spec:\\d+")
+		local topLineRegExp = RegExp("^.*test stack for Error()")
+		local topLineRegExp2 = RegExp("^.*test stack for Error.new()")
 
 		jestExpect(topLineRegExp:test(err.stack)).toEqual(true)
-		jestExpect(topLineRegExp:test(err2.stack)).toEqual(true)
+		jestExpect(topLineRegExp2:test(err2.stack)).toEqual(true)
 	end)
 
+	it("checks Error stack field doesn't contains stack from callable table", function()
+		local err = Error("test stack for Error()")
+
+		local topLineRegExp = RegExp("Error:\\d+ function __call")
+
+		jestExpect(topLineRegExp:test(err.stack)).toEqual(false)
+	end)
+
+	it("checks Error stack field doesn't contains stack from Error.new function", function()
+		local err = Error.new("test stack for Error.new()")
+
+		local topLineRegExp = RegExp("Error:\\d+ function new")
+
+		jestExpect(topLineRegExp:test(err.stack)).toEqual(false)
+	end)
+
+	it("checks Error stack field contains error name at the beginning", function()
+		local err = Error("test stack for Error()")
+		local err2 = Error.new("test stack for Error.new()")
+
+		local topLineRegExp = RegExp("^Error: test stack for Error()")
+		local topLineRegExp2 = RegExp("^Error: test stack for Error.new()")
+
+		jestExpect(topLineRegExp:test(err.stack)).toEqual(true)
+		jestExpect(topLineRegExp2:test(err2.stack)).toEqual(true)
+	end)
+
+	itSKIP(
+		"checks Error stack field contains error name at the beginning if name is modified before accessing stack",
+		function()
+			local err = Error("test stack for Error()")
+			local err2 = Error.new("test stack for Error.new()")
+			err.name = "MyError"
+			err2.name = "MyError"
+
+			local topLineRegExp = RegExp("^MyError: test stack for Error()")
+			local topLineRegExp2 = RegExp("^MyError: test stack for Error.new()")
+
+			jestExpect(topLineRegExp:test(err.stack)).toEqual(true)
+			jestExpect(topLineRegExp2:test(err2.stack)).toEqual(true)
+		end
+	)
+
 	it("checks default Error message field", function()
 		jestExpect(Error().message).toEqual("")
 	end)
 
 	it("prints 'Error' for an empty Error", function()
 		jestExpect(tostring(Error())).toEqual("Error")
 	end)
+
+	describe("Error.captureStackTrace", function()
+		local function createErrorNew()
+			return Error.new("error message new function")
+		end
+
+		local function createErrorCallable()
+			return Error("error message callable table")
+		end
+
+		local function myCaptureStacktrace(err: Error)
+			Error.captureStackTrace(err)
+		end
+
+		local function myCaptureStacktraceNested0(err: Error)
+			local function f1()
+				local function f2()
+					Error.captureStackTrace(err)
+				end
+				f2()
+			end
+			f1()
+		end
+
+		local function myCaptureStacktraceNested1(err: Error)
+			local function f1()
+				local function f2()
+					Error.captureStackTrace(err, f1)
+				end
+				f2()
+			end
+			f1()
+		end
+
+		local function myCaptureStacktraceNested2(err: Error)
+			local function f1()
+				local function f2()
+					Error.captureStackTrace(err, f2)
+				end
+				f2()
+			end
+			f1()
+		end
+
+		it("should capture functions stacktrace - Error.new", function()
+			local err = createErrorNew()
+
+			local stacktraceRegex1 = RegExp("function createErrorNew")
+			local stacktraceRegex2 = RegExp("function createErrorCallable")
+			local stacktraceRegex3 = RegExp("function myCaptureStacktrace")
+
+			jestExpect(stacktraceRegex1:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegex2:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex3:test(err.stack)).toEqual(false)
+
+			myCaptureStacktrace(err)
+
+			jestExpect(stacktraceRegex1:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex2:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex3:test(err.stack)).toEqual(true)
+		end)
+
+		it("should capture functions stacktrace - Error", function()
+			local err = createErrorCallable()
+
+			local stacktraceRegex1 = RegExp("function createErrorNew")
+			local stacktraceRegex2 = RegExp("function createErrorCallable")
+			local stacktraceRegex3 = RegExp("function myCaptureStacktrace")
+
+			jestExpect(stacktraceRegex1:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex2:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegex3:test(err.stack)).toEqual(false)
+
+			myCaptureStacktrace(err)
+
+			jestExpect(stacktraceRegex1:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex2:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegex3:test(err.stack)).toEqual(true)
+		end)
+
+		it("should capture functions stacktrace with option - Error.new", function()
+			local err = createErrorNew()
+			local stacktraceRegex = RegExp("function myCaptureStacktraceNested")
+			local stacktraceRegexF1 = RegExp("function f1")
+			local stacktraceRegexF2 = RegExp("function f2")
+
+			myCaptureStacktraceNested0(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(true)
+
+			myCaptureStacktraceNested1(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(false)
+
+			myCaptureStacktraceNested2(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(false)
+		end)
+
+		it("should capture functions stacktrace with option - Error", function()
+			local err = createErrorCallable()
+			local stacktraceRegex = RegExp("function myCaptureStacktraceNested")
+			local stacktraceRegexF1 = RegExp("function f1")
+			local stacktraceRegexF2 = RegExp("function f2")
+
+			myCaptureStacktraceNested0(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(true)
+
+			myCaptureStacktraceNested1(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(false)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(false)
+
+			myCaptureStacktraceNested2(err)
+
+			jestExpect(stacktraceRegex:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF1:test(err.stack)).toEqual(true)
+			jestExpect(stacktraceRegexF2:test(err.stack)).toEqual(false)
+		end)
+	end)
 end

modules/luau-polyfill/src/Error/init.lua

@@ -13,6 +13,13 @@
 	* limitations under the License.
 ]]
 --!strict
+local LuauPolyfill = script.Parent
+local Packages = LuauPolyfill.Parent
+
+local types = require(Packages.ES7Types)
+
+type Function = types.Function
+
 export type Error = { name: string, message: string, stack: string? }
 
 local Error = {}
@@ -24,21 +31,54 @@ Error.__tostring = function(self)
 	return getmetatable(Error :: any).__tostring(self)
 end
 
+-- ROBLOX NOTE: extracted __createError function so that both Error.new() and Error() can capture the stack trace at the same depth
+local function __createError(message: string?): Error
+	local self = (setmetatable({
+		name = DEFAULT_NAME,
+		message = message or "",
+	}, Error) :: any) :: Error
+	Error.__captureStackTrace(self, 4)
+	return self
+end
+
 function Error.new(message: string?): Error
-	return (
-		setmetatable({
-			name = DEFAULT_NAME,
-			message = message or "",
-			stack = debug.traceback(nil, 2),
-		}, Error) :: any
-	) :: Error
+	return __createError(message)
+end
+
+function Error.captureStackTrace(err: Error, options: Function?)
+	Error.__captureStackTrace(err, 3, options)
+end
+
+function Error.__captureStackTrace(err: Error, level: number, options: Function?)
+	local message = err.message
+	local name = err.name or DEFAULT_NAME
+
+	local errName = name .. (if message ~= nil and message ~= "" then (": " .. message) else "")
+
+	if typeof(options) == "function" then
+		local stack = debug.traceback(nil, level)
+		local functionName: string = debug.info(options, "n")
+		local sourceFilePath: string = debug.info(options, "s")
+
+		local espacedSourceFilePath = string.gsub(sourceFilePath, "([%(%)%.%%%+%-%*%?%[%^%$])", "%%%1")
+		local stacktraceLinePattern = espacedSourceFilePath .. ":%d* function " .. functionName
+		local beg = string.find(stack, stacktraceLinePattern)
+		local end_ = nil
+		if beg ~= nil then
+			beg, end_ = string.find(stack, "\n", beg + 1)
+		end
+		if end_ ~= nil then
+			stack = string.sub(stack, end_ + 1)
+		end
+		err.stack = errName .. "\n" .. stack
+	else
+		err.stack = debug.traceback(errName, level)
+	end
 end
 
 return setmetatable(Error, {
 	__call = function(_, ...)
-		local inst = Error.new(...)
-		inst.stack = debug.traceback(nil, 2)
-		return inst
+		return __createError(...)
 	end,
 	__tostring = function(self)
 		if self.name ~= nil then