fix(xpcall): catch errors under direct PCall/DoString + inspect upvalues

.github/workflows/ci.yml

@@ -17,9 +17,6 @@ jobs:
     name: Lint
     runs-on: ubuntu-latest
     timeout-minutes: 15
-    env:
-      GOCACHE: ${{ runner.temp }}/go-build
-      GOLANGCI_LINT_CACHE: ${{ runner.temp }}/golangci-lint
     steps:
       - name: Check out code
         uses: actions/checkout@v6
@@ -32,6 +29,9 @@ jobs:
 
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v9.2.0
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
+          GOLANGCI_LINT_CACHE: ${{ runner.temp }}/golangci-lint
         with:
           version: v2.8.0
           only-new-issues: false
@@ -45,8 +45,6 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, windows-latest]
-    env:
-      GOCACHE: ${{ runner.temp }}/go-build
     steps:
       - name: Check out code
         uses: actions/checkout@v6
@@ -58,14 +56,14 @@ jobs:
           cache: true
 
       - name: Run tests
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
         run: go test -timeout 120s ./...
 
   race:
     name: Race Tests
     runs-on: ubuntu-latest
     timeout-minutes: 40
-    env:
-      GOCACHE: ${{ runner.temp }}/go-build
     steps:
       - name: Check out code
         uses: actions/checkout@v6
@@ -78,15 +76,14 @@ jobs:
 
       - name: Run race tests
         env:
+          GOCACHE: ${{ runner.temp }}/go-build
           GORACE: "halt_on_error=1"
         run: go test -race -timeout 120s ./...
 
   fuzz:
     name: Fuzz
     runs-on: ubuntu-latest
     timeout-minutes: 20
-    env:
-      GOCACHE: ${{ runner.temp }}/go-build
     steps:
       - name: Check out code
         uses: actions/checkout@v6
@@ -98,20 +95,24 @@ jobs:
           cache: true
 
       - name: Fuzz type decode
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
         run: go test -fuzz=FuzzTypeDecodeToValidation -fuzztime=60s -timeout=120s
 
       - name: Fuzz Lua source types
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
         run: go test -fuzz=FuzzLuaTypeValidation -fuzztime=60s -timeout=120s
 
       - name: Fuzz Lua with manifest
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
         run: go test -fuzz=FuzzLuaWithManifestTypes -fuzztime=60s -timeout=120s
 
   bench:
     name: Benchmarks
     runs-on: ubuntu-latest
     timeout-minutes: 15
-    env:
-      GOCACHE: ${{ runner.temp }}/go-build
     steps:
       - name: Check out code
         uses: actions/checkout@v6
@@ -123,6 +124,8 @@ jobs:
           cache: true
 
       - name: Run benchmarks
+        env:
+          GOCACHE: ${{ runner.temp }}/go-build
         run: go test -bench="Benchmark(Validate|Is)" -benchmem -count=1 -timeout=60s | tee bench.txt
 
       - name: Upload benchmark results

baselib.go

@@ -171,20 +171,16 @@ func basePCall(L *LState) int {
 		}()
 
 		if err != nil {
-			L.stack.SetSp(sp)
-			L.currentFrame = L.stack.Last()
+			L.unwindCallFrames(sp)
 			L.reg.SetTop(base)
 			L.currentFrame.Protected = false
-			// Clear continuation info after error
-			if ext := L.getFrameExt(L.currentFrame); ext != nil {
-				ext.Continuation = nil
-				ext.ContinuationCtx = nil
-			}
+			L.clearFrameExt(L.currentFrame)
 			L.Push(LFalse)
 			L.Push(err.(*ApiError).Object)
 			return 2
 		}
 		L.currentFrame.Protected = false
+		L.clearFrameExt(L.currentFrame)
 	} else {
 		// In coroutine - use continuation for yield-transparency
 		// Still need defer/recover for error handling
@@ -211,20 +207,16 @@ func basePCall(L *LState) int {
 		}
 
 		if err != nil {
-			L.stack.SetSp(sp)
-			L.currentFrame = L.stack.Last()
+			L.unwindCallFrames(sp)
 			L.reg.SetTop(base)
 			L.currentFrame.Protected = false
-			// Clear continuation info after error
-			if ext := L.getFrameExt(L.currentFrame); ext != nil {
-				ext.Continuation = nil
-				ext.ContinuationCtx = nil
-			}
+			L.clearFrameExt(L.currentFrame)
 			L.Push(LFalse)
 			L.Push(err.(*ApiError).Object)
 			return 2
 		}
 		L.currentFrame.Protected = false
+		L.clearFrameExt(L.currentFrame)
 	}
 
 	L.Insert(LTrue, 1)
@@ -404,28 +396,89 @@ func xpcallContinuation(L *LState, ctx interface{}, _ ResumeState) int {
 func baseXPCall(L *LState) int {
 	fn := L.CheckFunction(1)
 	errfunc := L.CheckFunction(2)
+	protectedFrame := L.currentFrame
 
-	// Mark the xpcall frame as protected for error handling
-	L.currentFrame.Protected = true
-	L.setFrameExt(L.currentFrame).ErrFunc = errfunc
+	// Mark the xpcall frame as protected. handleProtectedError (in threadRun)
+	// honors this for errors that surface after a yield inside a coroutine.
+	// The inline recover below is the boundary for synchronous errors and is
+	// the only recover layer present under a direct DoString/PCall call, which
+	// is why xpcall previously leaked its error in non-coroutine contexts.
+	protectedFrame.Protected = true
+	L.setFrameExt(protectedFrame).ErrFunc = errfunc
 
 	top := L.GetTop()
+	sp := L.stack.Sp()
 	L.Push(fn)
+	base := L.reg.Top() - 1 // nargs == 0 for xpcall's protected call
 
-	// Use CallK with continuation for yield-transparent xpcall
-	L.CallK(0, MultRet, xpcallContinuation, top)
+	var errValue LValue
+	func() {
+		defer func() {
+			if rcv := recover(); rcv != nil {
+				errValue = errorObjectFromRecover(rcv)
+			}
+		}()
+		L.CallK(0, MultRet, xpcallContinuation, top)
+	}()
 
-	// Check for yield before any cleanup
+	// Check for yield before any cleanup.
 	if L.yieldState != yieldNone {
 		return -1
 	}
 
-	L.currentFrame.Protected = false
-	if ext := L.getFrameExt(L.currentFrame); ext != nil {
-		ext.ErrFunc = nil
+	if errValue != nil {
+		// Invoke the handler BEFORE resetting the stack, while the errored
+		// frames are still on L.stack. This matches standard Lua semantics so
+		// the handler can inspect the throw site (debug.getlocal,
+		// debug.traceback, etc.). PCall's own errfunc path does the same.
+		handled := invokeErrorHandler(L, errfunc, errValue)
+
+		L.unwindCallFrames(sp)
+		L.reg.SetTop(base)
+		protectedFrame.Protected = false
+		L.clearFrameExt(protectedFrame)
+		L.Push(LFalse)
+		L.Push(handled)
+		return 2
 	}
+
+	protectedFrame.Protected = false
+	L.clearFrameExt(protectedFrame)
 	L.Insert(LTrue, top+1)
 	return L.GetTop() - top
 }
 
+// errorObjectFromRecover extracts the Lua value carried by a recovered panic.
+// Lua errors panic with *ApiError; anything else is stringified.
+func errorObjectFromRecover(rcv any) LValue {
+	if aerr, ok := rcv.(*ApiError); ok {
+		return aerr.Object
+	}
+
+	return LString(fmt.Sprint(rcv))
+}
+
+// invokeErrorHandler calls the xpcall error handler with the caught error
+// value while the throw-site frames are still on the stack. If the handler
+// itself raises, its error value is surfaced instead of the original. Stack
+// cleanup belongs to baseXPCall so both handler outcomes use the same unwind.
+func invokeErrorHandler(L *LState, errfunc *LFunction, errValue LValue) LValue {
+	L.Push(errfunc)
+	L.Push(errValue)
+
+	handled := errValue
+	func() {
+		defer func() {
+			if rcv := recover(); rcv != nil {
+				handled = errorObjectFromRecover(rcv)
+			}
+		}()
+
+		L.Call(1, 1)
+		handled = L.Get(-1)
+	}()
+
+	return handled
+}
+
 /* }}} */

compiler/check/synth/phase/extract/function.go

@@ -159,7 +159,7 @@ func (s *Synthesizer) synthFunctionTypeWithCapturePoint(
 
 	// Build CFG once, shared between overlay inference and return inference.
 	var fnGraph *cfg.Graph
-	if fn.Stmts != nil && len(fn.Stmts) > 0 {
+	if len(fn.Stmts) > 0 {
 		fnGraph = s.getOrBuildFunctionGraph(fn)
 	}
 

fixture_harness_test.go

@@ -277,7 +277,7 @@ func matchesExpectation(exp inlineExpectation, d diag.Diagnostic, entryFile stri
 	expFile := exp.File
 	// Match diagnostic file: d.Position.File is set by the checker (e.g. "test.lua" or module name)
 	if !strings.HasSuffix(d.Position.File, strings.TrimSuffix(expFile, ".lua")) &&
-		!(expFile == entryFile && d.Position.File == "test.lua") {
+		(expFile != entryFile || d.Position.File != "test.lua") {
 		return false
 	}
 	if d.Position.Line != exp.Line {
@@ -502,7 +502,10 @@ func verifyGoldenOutput(t *testing.T, s namedSuite, buf *bytes.Buffer) {
 	}
 
 	got := buf.String()
-	want := string(golden)
+	// Git may check text fixtures out with CRLF on Windows, while capturePrint
+	// deliberately emits Lua's canonical newline. Compare the content rather
+	// than the checkout's platform-specific line endings.
+	want := string(bytes.ReplaceAll(golden, []byte("\r\n"), []byte("\n")))
 	if got != want {
 		t.Errorf("output mismatch:\n--- want ---\n%s--- got ---\n%s", want, got)
 	}

inspect/stack.go

@@ -105,9 +105,12 @@ func GetStackFrame(l *lua.LState, level int) (StackFrame, bool) {
 		return StackFrame{}, false
 	}
 
-	// Spawn debug info with function info
-	funcTable := l.NewTable()
-	if _, err := l.GetInfo("nSluf", ar, funcTable); err != nil {
+	// GetInfo with the "f" flag returns the frame's *LFunction as its first
+	// return value (it does not store it in a table). Capture it here so the
+	// Go-function name resolution and the upvalue walk below use the real
+	// function rather than reading a never-populated table key.
+	fnVal, err := l.GetInfo("nSluf", ar, nil)
+	if err != nil {
 		return StackFrame{}, false
 	}
 
@@ -121,18 +124,16 @@ func GetStackFrame(l *lua.LState, level int) (StackFrame, bool) {
 
 	// If no name is provided and this is a Go function, try to determine the name
 	if frame.Name == "" && (frame.FuncType == "Go" || frame.FuncType == "C") {
-		// Spawn the actual function from the debug info table
-		fn := funcTable.RawGet(lua.LString("f"))
-		if luaFn, ok := fn.(*lua.LFunction); ok {
+		if luaFn, ok := fnVal.(*lua.LFunction); ok {
 			// Iterate over globals and see if any key maps to this function
 			globals := l.Get(lua.GlobalsIndex).(*lua.LTable)
 			globals.ForEach(func(key, value lua.LValue) {
 				if globalFn, ok := value.(*lua.LFunction); ok && globalFn == luaFn {
-					// Use the global key as the function name.
 					frame.Name = key.String()
 				}
 			})
 		}
+
 		// Fallback if no match was found.
 		if frame.Name == "" {
 			frame.Name = fmt.Sprintf("<go function at %s:%d>", frame.Source, frame.CurrentLine)
@@ -145,19 +146,19 @@ func GetStackFrame(l *lua.LState, level int) (StackFrame, bool) {
 		if name == "" {
 			break
 		}
+
 		frame.Locals = append(frame.Locals, Local{name, value})
 	}
 
 	// Only get upvalues for Lua functions
-	if fn := funcTable.RawGet(lua.LString("f")); fn != lua.LNil {
-		if luaFn, ok := fn.(*lua.LFunction); ok {
-			for i := 1; ; i++ {
-				name, value := l.GetUpvalue(luaFn, i)
-				if name == "" {
-					break
-				}
-				frame.Upvalues = append(frame.Upvalues, Upvalue{name, value})
+	if luaFn, ok := fnVal.(*lua.LFunction); ok && !luaFn.IsG {
+		for i := 1; ; i++ {
+			name, value := l.GetUpvalue(luaFn, i)
+			if name == "" {
+				break
 			}
+
+			frame.Upvalues = append(frame.Upvalues, Upvalue{name, value})
 		}
 	}
 

inspect/stack_test.go

@@ -250,3 +250,43 @@ func makeInspectFunc(key string) lua.LGFunction {
 		return 0
 	}
 }
+
+// TestGetStackFrameCapturesUpvalues guards against a regression where
+// GetStackFrame discarded the function returned by GetInfo("...f...") and read
+// a never-populated table key instead, so upvalues were never reported.
+func TestGetStackFrameCapturesUpvalues(t *testing.T) {
+	L := lua.NewState()
+	defer L.Close()
+
+	L.SetGlobal("inspect_up", L.NewFunction(func(L *lua.LState) int {
+		trace := GetStackTrace(L)
+		L.SetField(L.Get(lua.RegistryIndex).(*lua.LTable), "up_trace", lua.LString(trace.String()))
+		return 0
+	}))
+
+	if err := L.DoString(`
+		local function make_outer()
+			local captured = "upvalue-payload"
+			local function inner()
+				captured = captured .. "."
+				inspect_up()
+				return captured
+			end
+			inner()
+		end
+		make_outer()
+	`); err != nil {
+		t.Fatalf("Failed to run script: %v", err)
+	}
+
+	registry := L.Get(lua.RegistryIndex).(*lua.LTable)
+	traceStr := registry.RawGetString("up_trace").String()
+
+	if !strings.Contains(traceStr, "Upvalues:") {
+		t.Errorf("trace must include an Upvalues section;\ngot:\n%s", traceStr)
+	}
+
+	if !strings.Contains(traceStr, "captured = upvalue-payload") {
+		t.Errorf("trace must list the captured upvalue;\ngot:\n%s", traceStr)
+	}
+}