diff --git a/internal/core/panic.go b/internal/core/panic.go new file mode 100644 index 00000000000..8a434e69ad5 --- /dev/null +++ b/internal/core/panic.go @@ -0,0 +1,53 @@ +package core + +import ( + "bytes" + "fmt" + "runtime/debug" +) + +// PanicWithStack wraps a recovered panic value along with the original stack trace +// captured at the site of the panic. This allows re-panicking while preserving the +// original stack context, rather than losing it at the re-panic site. +type PanicWithStack struct { + Value any + Stack []byte +} + +func (p *PanicWithStack) String() string { + return fmt.Sprintf("%v\n%s", p.Value, string(p.Stack)) +} + +// NewPanicWithStack creates a PanicWithStack from a recovered panic value. +// It captures the current stack trace via debug.Stack() and strips the +// recovery infrastructure frames (debug.Stack, the deferred recovery function, +// and the panic runtime frame) so that only the actual crash site frames remain. +func NewPanicWithStack(recovered any) *PanicWithStack { + return &PanicWithStack{ + Value: recovered, + Stack: trimPanicRecoveryFrames(debug.Stack()), + } +} + +// trimPanicRecoveryFrames strips recovery infrastructure frames from a stack +// trace captured by debug.Stack() inside a deferred recovery function. +// It removes everything up to and including the "panic(...)" frame and its +// file/line pair, leaving only the frames from the actual crash site onward. +func trimPanicRecoveryFrames(stack []byte) []byte { + // Find the panic() frame. In Go stack traces, it appears as a line + // starting with "panic(" (possibly with leading whitespace stripped). + lines := bytes.Split(stack, []byte("\n")) + for i, line := range lines { + trimmed := bytes.TrimSpace(line) + if bytes.HasPrefix(trimmed, []byte("panic(")) { + // The panic frame is followed by its file/line pair on the next line. + // Skip both to get to the actual crash site. + startIdx := i + 2 + if startIdx < len(lines) { + return bytes.Join(lines[startIdx:], []byte("\n")) + } + } + } + // If we couldn't find the panic frame, return the original stack. + return stack +} diff --git a/internal/core/panic_test.go b/internal/core/panic_test.go new file mode 100644 index 00000000000..1eea0ee544c --- /dev/null +++ b/internal/core/panic_test.go @@ -0,0 +1,44 @@ +package core + +import ( + "testing" +) + +func TestTrimPanicRecoveryFrames(t *testing.T) { + t.Parallel() + + input := []byte(`goroutine 42 [running]: +runtime/debug.Stack() + runtime/debug/stack.go:26 +0x5e +github.com/microsoft/typescript-go/internal/ls.handleCrossProject[...].func1.1() + github.com/microsoft/typescript-go/internal/ls/crossproject.go:88 +0x70 +panic({0xc323a0?, 0x1780b90?}) + runtime/panic.go:783 +0x132 +github.com/microsoft/typescript-go/internal/checker.(*Checker).checkExpression(0xc0045a8000) + github.com/microsoft/typescript-go/internal/checker/checker.go:5000 +0x1a0 +github.com/microsoft/typescript-go/internal/ls.handleCrossProject[...].func1() + github.com/microsoft/typescript-go/internal/ls/crossproject.go:105 +0x150`) + + expected := `github.com/microsoft/typescript-go/internal/checker.(*Checker).checkExpression(0xc0045a8000) + github.com/microsoft/typescript-go/internal/checker/checker.go:5000 +0x1a0 +github.com/microsoft/typescript-go/internal/ls.handleCrossProject[...].func1() + github.com/microsoft/typescript-go/internal/ls/crossproject.go:105 +0x150` + + result := string(trimPanicRecoveryFrames(input)) + if result != expected { + t.Errorf("trimPanicRecoveryFrames result mismatch.\nGot:\n%s\n\nExpected:\n%s", result, expected) + } +} + +func TestTrimPanicRecoveryFramesNoPanicFrame(t *testing.T) { + t.Parallel() + + // If no panic() frame exists, the stack should be returned as-is. + input := []byte(`github.com/microsoft/typescript-go/internal/checker.(*Checker).checkExpression(0xc0045a8000) + github.com/microsoft/typescript-go/internal/checker/checker.go:5000 +0x1a0`) + + result := string(trimPanicRecoveryFrames(input)) + if result != string(input) { + t.Errorf("expected unchanged output when no panic frame, got:\n%s", result) + } +} diff --git a/internal/ls/crossproject.go b/internal/ls/crossproject.go index 27114098f1d..de82052e30d 100644 --- a/internal/ls/crossproject.go +++ b/internal/ls/crossproject.go @@ -2,9 +2,7 @@ package ls import ( "context" - "fmt" "iter" - "runtime/debug" "sync" "github.com/microsoft/typescript-go/internal/collections" @@ -73,7 +71,7 @@ func handleCrossProject[Req lsproto.HasTextDocumentPosition, Resp any]( wg := core.NewWorkGroup(false) var errMu sync.Mutex var enqueueItem func(item projectAndTextDocumentPosition) - var panicsOccured []string + var panicsOccurred []*core.PanicWithStack var panicMu sync.Mutex enqueueItem = func(item projectAndTextDocumentPosition) { var response response[Resp] @@ -86,10 +84,8 @@ func handleCrossProject[Req lsproto.HasTextDocumentPosition, Resp any]( } defer func() { if r := recover(); r != nil { - stack := debug.Stack() - panicOccured := fmt.Sprintf("panic handling request: %v\n%s", r, string(stack)) panicMu.Lock() - panicsOccured = append(panicsOccured, panicOccured) + panicsOccurred = append(panicsOccurred, core.NewPanicWithStack(r)) panicMu.Unlock() } }() @@ -206,8 +202,8 @@ func handleCrossProject[Req lsproto.HasTextDocumentPosition, Resp any]( // Process existing known projects first wg.RunAndWait() // No need to use mu here since we are not in parallel at this point - if panicsOccured != nil { - panic(fmt.Sprintf("Panics occurred during cross-project handling: %v", panicsOccured)) + if panicsOccurred != nil { + panic(panicsOccurred[0]) } if ctx.Err() != nil { return resp, ctx.Err() diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 5c4b93d775d..e8335d5515c 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -965,11 +965,18 @@ func (s *Server) getLanguageServiceAndCrossProjectOrchestrator(ctx context.Conte func (s *Server) recover(req *lsproto.RequestMessage) { if r := recover(); r != nil { stack := debug.Stack() - s.logger.Errorf("panic handling request %s: %v\n%s", req.Method, r, string(stack)) + // If the panic was wrapped with PanicWithStack (e.g., from cross-project handling), + // use the original stack trace to preserve the actual crash context. + panicValue := r + if pws, ok := r.(*core.PanicWithStack); ok { + stack = pws.Stack + panicValue = pws.Value + } + s.logger.Errorf("panic handling request %s: %v\n%s", req.Method, panicValue, string(stack)) if req.ID != nil { - _ = s.sendError(req.ID, fmt.Errorf("%w: panic handling request %s: %v", lsproto.ErrorCodeInternalError, req.Method, r)) + _ = s.sendError(req.ID, fmt.Errorf("%w: panic handling request %s: %v", lsproto.ErrorCodeInternalError, req.Method, panicValue)) } else { - s.logger.Error("unhandled panic in notification", req.Method, r) + s.logger.Error("unhandled panic in notification", req.Method, panicValue) } if s.telemetryEnabled { diff --git a/internal/lsp/stack_sanitizer.go b/internal/lsp/stack_sanitizer.go index 2e3cd886f4b..9a89a9984d3 100644 --- a/internal/lsp/stack_sanitizer.go +++ b/internal/lsp/stack_sanitizer.go @@ -24,10 +24,21 @@ func sanitizeStackTrace(stack string) string { // TODO: should we just look for the first '(' and // just strip everything before the prior newline? startIndex := strings.Index(stack, "runtime/debug.Stack()") - if startIndex < 0 { - return "" + if startIndex >= 0 { + stack = stack[startIndex:] + } else { + // For stacks that have already had recovery frames trimmed + // (e.g., from PanicWithStack), find the first line containing our module. + moduleIndex := strings.Index(stack, "typescript-go/internal") + if moduleIndex < 0 { + return "" + } + // Back up to the beginning of the line containing our module. + lineStart := strings.LastIndex(stack[:moduleIndex], "\n") + if lineStart >= 0 { + stack = stack[lineStart+1:] + } } - stack = stack[startIndex:] result := &strings.Builder{} diff --git a/internal/lsp/stack_sanitizer_test.go b/internal/lsp/stack_sanitizer_test.go index a66fe631f74..944458633fb 100644 --- a/internal/lsp/stack_sanitizer_test.go +++ b/internal/lsp/stack_sanitizer_test.go @@ -83,6 +83,31 @@ created by github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoo }) } +// This test represents a stack trace captured via PanicWithStack from a cross-project +// worker goroutine. The stack originates from the goroutine where the panic occurred +// with recovery infrastructure frames (debug.Stack, deferred func, panic) already +// stripped by NewPanicWithStack, leaving only the actual crash site frames. +func TestSanitizedCrossProjectPanicStackTrace(t *testing.T) { + t.Parallel() + + // This is the stack after NewPanicWithStack trims recovery overhead. + // It starts directly at the crash site without debug.Stack/panic frames. + input := `github.com/microsoft/typescript-go/internal/checker.(*Checker).checkExpression(0xc0045a8000, {0x10f6688, 0xc00c2871d0}, 0xc0001fe008, 0x0) + github.com/microsoft/typescript-go/internal/checker/checker.go:5000 +0x1a0 +github.com/microsoft/typescript-go/internal/ls.(*LanguageService).provideSymbolsAndEntries(0xc008329200, {0x10f6688, 0xc00c2871d0}, {0xc00b472030, 0x28}, {0x2, 0x4}, 0x0, 0x0) + github.com/microsoft/typescript-go/internal/ls/findallreferences.go:100 +0x200 +github.com/microsoft/typescript-go/internal/ls.handleCrossProject[...].func1() + github.com/microsoft/typescript-go/internal/ls/crossproject.go:105 +0x150 +github.com/microsoft/typescript-go/internal/core.(*WorkGroup).worker(0xc000120080) + github.com/microsoft/typescript-go/internal/core/workgroup.go:50 +0x80 +created by github.com/microsoft/typescript-go/internal/core.(*WorkGroup).Queue in goroutine 35 + github.com/microsoft/typescript-go/internal/core/workgroup.go:35 +0x60` + + baseline.Run(t, "crossProjectPanicStackTrace.md", sanitizedStackTraceBaselineContents(t, input, sanitizeStackTrace(input)), baseline.Options{ + Subfolder: "lsp/stackSanitizer/", + }) +} + func sanitizedStackTraceBaselineContents(t *testing.T, input string, output string) string { builder := strings.Builder{} builder.WriteString("Test name: `") diff --git a/testdata/baselines/reference/lsp/stackSanitizer/crossProjectPanicStackTrace.md b/testdata/baselines/reference/lsp/stackSanitizer/crossProjectPanicStackTrace.md new file mode 100644 index 00000000000..afd59099163 --- /dev/null +++ b/testdata/baselines/reference/lsp/stackSanitizer/crossProjectPanicStackTrace.md @@ -0,0 +1,31 @@ +Test name: `TestSanitizedCrossProjectPanicStackTrace` + +# Unsanitized input: + +```` +github.com/microsoft/typescript-go/internal/checker.(*Checker).checkExpression(0xc0045a8000, {0x10f6688, 0xc00c2871d0}, 0xc0001fe008, 0x0) + github.com/microsoft/typescript-go/internal/checker/checker.go:5000 +0x1a0 +github.com/microsoft/typescript-go/internal/ls.(*LanguageService).provideSymbolsAndEntries(0xc008329200, {0x10f6688, 0xc00c2871d0}, {0xc00b472030, 0x28}, {0x2, 0x4}, 0x0, 0x0) + github.com/microsoft/typescript-go/internal/ls/findallreferences.go:100 +0x200 +github.com/microsoft/typescript-go/internal/ls.handleCrossProject[...].func1() + github.com/microsoft/typescript-go/internal/ls/crossproject.go:105 +0x150 +github.com/microsoft/typescript-go/internal/core.(*WorkGroup).worker(0xc000120080) + github.com/microsoft/typescript-go/internal/core/workgroup.go:50 +0x80 +created by github.com/microsoft/typescript-go/internal/core.(*WorkGroup).Queue in goroutine 35 + github.com/microsoft/typescript-go/internal/core/workgroup.go:35 +0x60 +```` + +# Sanitized output: + +```` +typescript-go|>internal|>checker.(*Checker).checkExpression() + typescript-go|>internal|>checker|>checker.go:5000 +typescript-go|>internal|>ls.(*LanguageService).provideSymbolsAndEntries() + typescript-go|>internal|>ls|>findallreferences.go:100 +typescript-go|>internal|>ls.handleCrossProject[...].func1() + typescript-go|>internal|>ls|>crossproject.go:105 +typescript-go|>internal|>core.(*WorkGroup).worker() + typescript-go|>internal|>core|>workgroup.go:50 +typescript-go|>internal|>core.(*WorkGroup).Queue + typescript-go|>internal|>core|>workgroup.go:35 +````