Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions _extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,27 @@
"default": "auto",
"description": "%native-preview.showFailedResponses.description%",
"scope": "window"
},
"js/ts.server.trackFlakyDiagnostics": {
"type": "string",
Comment thread
weswigham marked this conversation as resolved.
"enum": [
"panic",
"log",
"never",
"auto"
],
"enumDescriptions": [
"%native-preview.trackFlakyDiagnostics.panic%",
"%native-preview.trackFlakyDiagnostics.log%",
"%native-preview.trackFlakyDiagnostics.never%",
"%native-preview.trackFlakyDiagnostics.auto%"
],
"default": "auto",
"tags": [
"experimental"
],
"description": "%native-preview.trackFlakyDiagnostics.description%",
"scope": "window"
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions _extension/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,10 @@
"native-preview.showFailedResponses.always": "Always show a notification when a request fails.",
"native-preview.showFailedResponses.never": "Never show a notification when a request fails.",
"native-preview.showFailedResponses.auto": "Show a notification only on VS Code Insiders.",
"native-preview.trackFlakyDiagnostics.description": "Controls whether an additional diagnostics pass is performed to check for and log flaky diagnostics",
"native-preview.trackFlakyDiagnostics.panic": "Panic when a flaky diagnostic is detected.",
"native-preview.trackFlakyDiagnostics.log": "Log an error when a flaky diagnostic is detected.",
"native-preview.trackFlakyDiagnostics.never": "Never perform flaky diagnostic checking and logging.",
"native-preview.trackFlakyDiagnostics.auto": "Perform flaky diagnostic logging only on VS Code Insiders.",
"developer": "Developer"
}
6 changes: 6 additions & 0 deletions _extension/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ export class Client implements vscode.Disposable {
?? readNativePreviewConfig<string | undefined>("pprofDir", undefined);
const pprofArgs = pprofDir ? ["--pprofDir", pprofDir] : [];

const flakesFlag = vscode.workspace
Comment thread
jakebailey marked this conversation as resolved.
.getConfiguration("js/ts")
.get<"panic" | "log" | "never" | "auto">("server.trackFlakyDiagnostics", "auto");
const effectiveflakesFlag = flakesFlag === "auto" ? (isInsiders() ? "log" : "never") : flakesFlag;

const goMemLimit = readNativePreviewConfig<string | undefined>("server.goMemLimit", undefined)
?? readNativePreviewConfig<string | undefined>("goMemLimit", undefined);
const env = { ...process.env };
Expand Down Expand Up @@ -193,6 +198,7 @@ export class Client implements vscode.Disposable {
// Refresh the initial log verbosity in case the output channel's log
// level changed between construction and start.
this.clientOptions.initializationOptions.logVerbosity = this.outputChannel.logLevel;
this.clientOptions.initializationOptions.trackFlakyDiagnostics = effectiveflakesFlag !== "never" ? (effectiveflakesFlag === "panic" ? 2 : 1) : 0;

this.client = new NativePreviewLanguageClient(
"js/ts",
Expand Down
16 changes: 16 additions & 0 deletions internal/lsp/lsproto/_generate/generate.mts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ const customStructures: Structure[] = [
optional: true,
documentation: "The initial log verbosity level, matching the client's output channel log level at startup. Subsequent changes are sent via custom/setLogVerbosity.",
},
{
name: "trackFlakyDiagnostics",
type: { kind: "reference", name: "DiagnosticFlakeLogLevel" },
optional: true,
documentation: "The level at which we track flaky diagnostics, if at all.",
},
],
documentation: "InitializationOptions contains user-provided initialization options.",
},
Expand Down Expand Up @@ -700,6 +706,16 @@ const customEnumerations: Enumeration[] = [
],
documentation: "Log verbosity level, mirroring the VS Code LogLevel enum values.",
},
{
name: "DiagnosticFlakeLogLevel",
type: { kind: "base", name: "integer" },
values: [
{ name: "Off", value: 0, documentation: "All flake logging disabled." },
{ name: "Log", value: 1, documentation: "Log flaky diagnostics to the error log." },
{ name: "Panic", value: 2, documentation: "Panic on flaky diagnostics." },
],
documentation: "Behavior for tracking and logging flaky diagnostics.",
},
{
name: "VSReferenceKind",
type: { kind: "base", name: "integer" },
Expand Down
27 changes: 27 additions & 0 deletions internal/lsp/lsproto/lsp_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

74 changes: 74 additions & 0 deletions internal/lsp/lsproto/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package lsproto

import (
"cmp"
"fmt"
"strconv"
)

// Implements a cmp.Compare like function for two Position
Expand Down Expand Up @@ -35,3 +37,75 @@ func (m StringOrMarkupContent) AsString() string {
}
return ""
}

func (m IntegerOrString) AsString() string {
codeStr := ""
if m.String != nil {
codeStr = *m.String
} else if m.Integer != nil {
codeStr = strconv.Itoa(int(*m.Integer))
} else {
codeStr = "-1"
}
return codeStr
}

func diagnosticExistsInSlice(elem *Diagnostic, diags []*Diagnostic) bool {
for _, diag := range diags {
if diagnosticsEqual(elem, diag) {
Comment thread
weswigham marked this conversation as resolved.
return true
}
}
return false
}

func diagnosticsEqual(diag1 *Diagnostic, diag2 *Diagnostic) bool {
if diagnosticCodesEqual(diag1.Code, diag2.Code) && diagnosticMessagesEqual(diag1.Message, diag2.Message) && CompareRanges(diag1.Range, diag2.Range) == 0 {
return true
}
return false
}

func diagnosticCodesEqual(code1 *IntegerOrString, code2 *IntegerOrString) bool {
if code1.String != nil && code2.String != nil {
return *code1.String == *code2.String
}
if code1.Integer != nil && code2.Integer != nil {
return *code1.Integer == *code2.Integer
}
return false
}

func diagnosticMessagesEqual(message1 StringOrMarkupContent, message2 StringOrMarkupContent) bool {
if message1.String != nil && message2.String != nil {
return *message1.String == *message2.String
}
if message1.MarkupContent != nil && message2.MarkupContent != nil {
return message1.MarkupContent.Kind == message2.MarkupContent.Kind && message1.MarkupContent.Value == message2.MarkupContent.Value
}
return false
}

func CompareDiagnostics(list1 []*Diagnostic, list2 []*Diagnostic) ([]*Diagnostic, []*Diagnostic) {
missingFromList1 := []*Diagnostic{}
missingFromList2 := []*Diagnostic{}
for _, elem := range list1 {
if !diagnosticExistsInSlice(elem, list2) {
missingFromList2 = append(missingFromList2, elem)
}
}
for _, elem := range list2 {
if !diagnosticExistsInSlice(elem, list1) {
missingFromList1 = append(missingFromList1, elem)
}
}
return missingFromList1, missingFromList2
}

func (elem *Diagnostic) AsString() string {
return fmt.Sprintf("%v (%v:%v-%v:%v): %v", elem.Code.AsString(), elem.Range.Start.Line, elem.Range.Start.Character, elem.Range.End.Line, elem.Range.End.Character, elem.Message.AsString())
}

func (elem *Diagnostic) CodeAsString() string {
return fmt.Sprintf("Code(%v)", elem.Code.AsString())
}
61 changes: 60 additions & 1 deletion internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/microsoft/typescript-go/internal/api"
"github.com/microsoft/typescript-go/internal/collections"
"github.com/microsoft/typescript-go/internal/compiler"
"github.com/microsoft/typescript-go/internal/core"
"github.com/microsoft/typescript-go/internal/diagnostics"
"github.com/microsoft/typescript-go/internal/fswatch"
Expand Down Expand Up @@ -217,6 +218,8 @@ type Server struct {
projectProgress *projectLoadingProgress

startWatchdog func(parentPID int)

flakeLogging lsproto.DiagnosticFlakeLogLevel
}

func (s *Server) Session() *project.Session { return s.session }
Expand Down Expand Up @@ -1038,6 +1041,9 @@ func (s *Server) handleInitialize(ctx context.Context, params *lsproto.Initializ
s.logger.SetVerbosity(v)
}
}
if s.initializationOptions.TrackFlakyDiagnostics != nil {
s.flakeLogging = *s.initializationOptions.TrackFlakyDiagnostics
}
s.clientCapabilities = params.Capabilities.Resolve()
if s.clientCapabilities.Window.WorkDoneProgress {
s.projectProgress = newProjectLoadingProgress(s, s.progressDelay)
Expand Down Expand Up @@ -1363,7 +1369,60 @@ func (s *Server) handleSetLogVerbosity(_ context.Context, params *lsproto.SetLog

func (s *Server) handleDocumentDiagnostic(ctx context.Context, ls *ls.LanguageService, params *lsproto.DocumentDiagnosticParams) (lsproto.DocumentDiagnosticResponse, error) {
ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics)
return ls.ProvideDiagnostics(ctx, params.TextDocument.Uri)
if s.flakeLogging == lsproto.DiagnosticFlakeLogLevelOff {
return ls.ProvideDiagnostics(ctx, params.TextDocument.Uri)
}
direct, err := ls.ProvideDiagnostics(ctx, params.TextDocument.Uri)
if err != nil {
return direct, err
}
ls.GetProgram().Emit(ctx, compiler.EmitOptions{
WriteFile: func(fileName, text string, data *compiler.WriteFileData) error {
// do nothing
return nil
},
})
secondary, err2 := ls.ProvideDiagnostics(ctx, params.TextDocument.Uri)
if err2 != nil {
return direct, err
}
missingFromPre, missingFromPost := lsproto.CompareDiagnostics(direct.FullDocumentDiagnosticReport.Items, secondary.FullDocumentDiagnosticReport.Items)
if len(missingFromPre) == 0 && len(missingFromPost) == 0 {
return direct, err
}

diff := generateDiagnosticDiffString(missingFromPre, missingFromPost, (*lsproto.Diagnostic).AsString)

s.logger.Error(diff)

if s.telemetryEnabled {
sanitizedDiff := generateDiagnosticDiffString(missingFromPre, missingFromPost, (*lsproto.Diagnostic).CodeAsString)
_ = sendNotification(s, lsproto.TelemetryEventInfo, lsproto.TelemetryEvent{

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also sending a request failure telemetry event with a new flakeLog method if telemetry is enabled, too - with just the diagnostic codes in the payload. @DanielRosenwasser do you have any thoughts on that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exactly the ideal way to do this, but it will automatically appear in the dashboards so it's probably fine for now.

RequestFailureTelemetryEvent: &lsproto.RequestFailureTelemetryEvent{
Properties: &lsproto.RequestFailureTelemetryProperties{
ErrorCode: lsproto.ErrorCodeInternalError.String(),
RequestMethod: "textDocument.diagnostic.flakeLog",
Stack: sanitizedDiff,
},
},
})
}

if s.flakeLogging == lsproto.DiagnosticFlakeLogLevelPanic {
panic("flaky diagnostic(s) logged:\n" + diff)
}
return direct, err
}

func generateDiagnosticDiffString(missingFromPre []*lsproto.Diagnostic, missingFromPost []*lsproto.Diagnostic, stringifier func(*lsproto.Diagnostic) string) string {
var b strings.Builder
for _, elem := range missingFromPre {
b.WriteString(fmt.Sprintf("Diagnostic %v was present after emit but not before emit\n", stringifier(elem)))
}
for _, elem := range missingFromPost {
b.WriteString(fmt.Sprintf("Diagnostic %v was present before emit but not after emit\n", stringifier(elem)))
}
return b.String()
}

func (s *Server) handleHover(ctx context.Context, ls *ls.LanguageService, params *lsproto.HoverParams) (lsproto.HoverResponse, error) {
Expand Down