From 6e2a7d25f558f66fdcfc2259cd9aa9eba4ac7f2b Mon Sep 17 00:00:00 2001 From: gak Date: Wed, 19 Jun 2024 15:20:53 +1000 Subject: [PATCH] feat: lsp/extension to show build status for failures and errs (#1830) Fixes #1808 https://github.com/TBD54566975/ftl/assets/31338/54e6f71e-1ad5-4ef0-9b03-630a8ae428f5 --------- Co-authored-by: Alec Thomas --- buildengine/deploy.go | 2 +- buildengine/engine.go | 49 +++++++++++++++++++++++++++--- cmd/ftl/cmd_dev.go | 6 +--- extensions/vscode/src/client.ts | 28 +++++++++++++---- extensions/vscode/src/extension.ts | 10 +++--- extensions/vscode/src/status.ts | 45 ++++++++++++++++++++++----- lsp/lsp.go | 45 +++++++++++++++++++++++++-- 7 files changed, 152 insertions(+), 33 deletions(-) diff --git a/buildengine/deploy.go b/buildengine/deploy.go index 983d8aea74..9afaa012bb 100644 --- a/buildengine/deploy.go +++ b/buildengine/deploy.go @@ -105,7 +105,7 @@ func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnl return nil } -func teminateModuleDeployment(ctx context.Context, client ftlv1connect.ControllerServiceClient, module string) error { +func terminateModuleDeployment(ctx context.Context, client ftlv1connect.ControllerServiceClient, module string) error { logger := log.FromContext(ctx).Scope(module) status, err := client.Status(ctx, connect.NewRequest(&ftlv1.StatusRequest{})) diff --git a/buildengine/engine.go b/buildengine/engine.go index aaee445878..da33a2a456 100644 --- a/buildengine/engine.go +++ b/buildengine/engine.go @@ -37,12 +37,16 @@ type moduleMeta struct { } type Listener interface { + // OnBuildStarted is called when a build is started for a project. OnBuildStarted(module Module) -} -type BuildStartedListenerFunc func(module Module) + // OnBuildSuccess is called when all modules have been built successfully and deployed. + OnBuildSuccess() -func (b BuildStartedListenerFunc) OnBuildStarted(module Module) { b(module) } + // OnBuildFailed is called for any build failures. + // OnBuildSuccess should not be called if this is called after a OnBuildStarted. + OnBuildFailed(err error) +} // Engine for building a set of modules. type Engine struct { @@ -227,6 +231,18 @@ func (e *Engine) Dev(ctx context.Context, period time.Duration) error { return e.watchForModuleChanges(ctx, period) } +func (e *Engine) reportBuildFailed(err error) { + if e.listener != nil { + e.listener.OnBuildFailed(err) + } +} + +func (e *Engine) reportSuccess() { + if e.listener != nil { + e.listener.OnBuildSuccess() + } +} + func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration) error { logger := log.FromContext(ctx) @@ -253,8 +269,10 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration err = e.buildAndDeploy(ctx, 1, true) if err != nil { logger.Errorf(err, "initial deploy failed") + e.reportBuildFailed(err) } else { logger.Infof("All modules deployed, watching for changes...") + e.reportSuccess() } moduleHashes := map[string][]byte{} @@ -262,14 +280,17 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration hash, err := computeModuleHash(sch) if err != nil { logger.Errorf(err, "compute hash for %s failed", name) + e.reportBuildFailed(err) return false } moduleHashes[name] = hash return true }) - // Watch for file and schema changes didUpdateDeployments := false + // Track if there was an error, so that when deployments are complete we don't report success. + didError := false + // Watch for file and schema changes for { var completedUpdatesTimer <-chan time.Time if didUpdateDeployments { @@ -280,6 +301,11 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration return ctx.Err() case <-completedUpdatesTimer: logger.Infof("All modules deployed, watching for changes...") + // Some cases, this will trigger after a build failure, so report accordingly. + if !didError { + e.reportSuccess() + } + didUpdateDeployments = false case event := <-watchEvents: switch event := event.(type) { @@ -287,8 +313,11 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration config := event.Module.Config if _, exists := e.moduleMetas.Load(config.Module); !exists { e.moduleMetas.Store(config.Module, moduleMeta{module: event.Module}) + didError = false err := e.buildAndDeploy(ctx, 1, true, config.Module) if err != nil { + didError = true + e.reportBuildFailed(err) logger.Errorf(err, "deploy %s failed", config.Module) } else { didUpdateDeployments = true @@ -297,8 +326,10 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration case WatchEventModuleRemoved: config := event.Module.Config - err := teminateModuleDeployment(ctx, e.client, config.Module) + err := terminateModuleDeployment(ctx, e.client, config.Module) if err != nil { + didError = true + e.reportBuildFailed(err) logger.Errorf(err, "terminate %s failed", config.Module) } else { didUpdateDeployments = true @@ -318,8 +349,11 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration logger.Debugf("Skipping build and deploy; event time %v is before the last build time %v", event.Time, meta.lastBuildStartTime) continue // Skip this event as it's outdated } + didError = false err := e.buildAndDeploy(ctx, 1, true, config.Module) if err != nil { + didError = true + e.reportBuildFailed(err) logger.Errorf(err, "build and deploy failed for module %q", event.Module.Config.Module) } else { didUpdateDeployments = true @@ -332,6 +366,8 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration hash, err := computeModuleHash(change.Module) if err != nil { + didError = true + e.reportBuildFailed(err) logger.Errorf(err, "compute hash for %s failed", change.Name) continue } @@ -346,8 +382,11 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration dependentModuleNames := e.getDependentModuleNames(change.Name) if len(dependentModuleNames) > 0 { logger.Infof("%s's schema changed; processing %s", change.Name, strings.Join(dependentModuleNames, ", ")) + didError = false err = e.buildAndDeploy(ctx, 1, true, dependentModuleNames...) if err != nil { + didError = true + e.reportBuildFailed(err) logger.Errorf(err, "deploy %s failed", change.Name) } else { didUpdateDeployments = true diff --git a/cmd/ftl/cmd_dev.go b/cmd/ftl/cmd_dev.go index 90f7cb52b7..84160f75bf 100644 --- a/cmd/ftl/cmd_dev.go +++ b/cmd/ftl/cmd_dev.go @@ -77,7 +77,7 @@ func (d *devCmd) Run(ctx context.Context, projConfig projectconfig.Config) error opts := []buildengine.Option{buildengine.Parallelism(d.Parallelism)} if d.Lsp { d.languageServer = lsp.NewServer(ctx) - opts = append(opts, buildengine.WithListener(buildengine.BuildStartedListenerFunc(d.OnBuildStarted))) + opts = append(opts, buildengine.WithListener(d.languageServer)) ctx = log.ContextWithLogger(ctx, log.FromContext(ctx).AddSink(lsp.NewLogSink(d.languageServer))) g.Go(func() error { return d.languageServer.Run() @@ -93,7 +93,3 @@ func (d *devCmd) Run(ctx context.Context, projConfig projectconfig.Config) error return g.Wait() } - -func (d *devCmd) OnBuildStarted(module buildengine.Module) { - d.languageServer.BuildStarted(module.Config.Dir) -} diff --git a/extensions/vscode/src/client.ts b/extensions/vscode/src/client.ts index e4220438e7..002c0643ca 100644 --- a/extensions/vscode/src/client.ts +++ b/extensions/vscode/src/client.ts @@ -7,7 +7,7 @@ import { import { FTLStatus } from './status' export class FTLClient { - private clientName = 'ftl languge server' + private clientName = 'ftl language server' private clientId = 'ftl' private statusBarItem: vscode.StatusBarItem @@ -59,18 +59,34 @@ export class FTLClient { serverOptions, clientOptions ) - context.subscriptions.push(this.client) + const buildStatus = this.client.onNotification('ftl/buildState', (message) => { + console.log('Build status', message) + const state = message.state + + if (state == 'building') { + FTLStatus.buildRunning(this.statusBarItem) + } else if (state == 'success') { + FTLStatus.buildOK(this.statusBarItem) + } else if (state == 'failure') { + FTLStatus.buildError(this.statusBarItem, message.error) + } else { + FTLStatus.ftlError(this.statusBarItem, 'Unknown build status from FTL LSP server') + this.outputChannel.appendLine(`Unknown build status from FTL LSP server: ${state}`) + } + }) + context.subscriptions.push(buildStatus) + this.outputChannel.appendLine('Starting lsp client') try { await this.client.start() this.outputChannel.appendLine('Client started') console.log(`${this.clientName} started`) - FTLStatus.started(this.statusBarItem) + FTLStatus.buildOK(this.statusBarItem) } catch (error) { console.error(`Error starting ${this.clientName}: ${error}`) - FTLStatus.error(this.statusBarItem, `Error starting ${this.clientName}: ${error}`) + FTLStatus.ftlError(this.statusBarItem, `Error starting ${this.clientName}: ${error}`) this.outputChannel.appendLine(`Error starting ${this.clientName}: ${error}`) } @@ -124,7 +140,7 @@ export class FTLClient { try { // Forcefully terminate if SIGTERM fails process.kill(serverProcess.pid, 'SIGKILL') - console.log('Server process terminiated with SIGKILL') + console.log('Server process terminated with SIGKILL') } catch (killError) { console.log('Failed to kill server process', killError) } @@ -133,6 +149,6 @@ export class FTLClient { console.log('Server process was already killed') } - FTLStatus.stopped(this.statusBarItem) + FTLStatus.ftlStopped(this.statusBarItem) } } diff --git a/extensions/vscode/src/extension.ts b/extensions/vscode/src/extension.ts index 39df37028d..8bf4f72f20 100644 --- a/extensions/vscode/src/extension.ts +++ b/extensions/vscode/src/extension.ts @@ -106,7 +106,7 @@ const promptStartClient = async (context: vscode.ExtensionContext) => { outputChannel.appendLine(`FTL configuration: ${JSON.stringify(configuration)}`) const automaticallyStartServer = configuration.get('automaticallyStartServer') - FTLStatus.stopped(statusBarItem) + FTLStatus.ftlStopped(statusBarItem) if (automaticallyStartServer === 'always') { outputChannel.appendLine(`FTL development server automatically started`) @@ -132,19 +132,19 @@ const promptStartClient = async (context: vscode.ExtensionContext) => { break case 'No': outputChannel.appendLine('FTL development server disabled') - FTLStatus.stopped(statusBarItem) + FTLStatus.ftlStopped(statusBarItem) break case 'Never': outputChannel.appendLine('FTL development server set to never auto start') configuration.update('automaticallyStartServer', 'never', vscode.ConfigurationTarget.Global) - FTLStatus.stopped(statusBarItem) + FTLStatus.ftlStopped(statusBarItem) break } }) } const startClient = async (context: ExtensionContext) => { - FTLStatus.starting(statusBarItem) + FTLStatus.ftlStarting(statusBarItem) const ftlConfig = vscode.workspace.getConfiguration('ftl') const workspaceRootPath = await getProjectOrWorkspaceRoot() @@ -155,7 +155,7 @@ const startClient = async (context: ExtensionContext) => { const ftlOK = await FTLPreflightCheck(resolvedFtlPath) if (!ftlOK) { - FTLStatus.stopped(statusBarItem) + FTLStatus.ftlStopped(statusBarItem) return } diff --git a/extensions/vscode/src/status.ts b/extensions/vscode/src/status.ts index 18ca095e80..59007897a1 100644 --- a/extensions/vscode/src/status.ts +++ b/extensions/vscode/src/status.ts @@ -1,20 +1,49 @@ import * as vscode from 'vscode' +const resetColors = (statusBarItem: vscode.StatusBarItem) => { + statusBarItem.backgroundColor = undefined + statusBarItem.color = undefined +} + +const errorColors = (statusBarItem: vscode.StatusBarItem) => { + statusBarItem.backgroundColor = new vscode.ThemeColor('statusBarItem.errorBackground') + statusBarItem.color = new vscode.ThemeColor('statusBarItem.errorForeground') +} + +const loadingColors = (statusBarItem: vscode.StatusBarItem) => { + statusBarItem.backgroundColor = new vscode.ThemeColor('statusBarItem.warningBackground') + statusBarItem.color = new vscode.ThemeColor('statusBarItem.warningForeground') +} + export const FTLStatus = { - starting: (statusBarItem: vscode.StatusBarItem) => { + ftlStarting: (statusBarItem: vscode.StatusBarItem) => { + loadingColors(statusBarItem) statusBarItem.text = `$(sync~spin) FTL` statusBarItem.tooltip = 'FTL is starting...' }, - started: (statusBarItem: vscode.StatusBarItem) => { - statusBarItem.text = `$(zap) FTL` - statusBarItem.tooltip = 'FTL is running.' - }, - stopped: (statusBarItem: vscode.StatusBarItem) => { + ftlStopped: (statusBarItem: vscode.StatusBarItem) => { + resetColors(statusBarItem) statusBarItem.text = `$(primitive-square) FTL` statusBarItem.tooltip = 'FTL is stopped.' }, - error: (statusBarItem: vscode.StatusBarItem, message: string) => { + ftlError: (statusBarItem: vscode.StatusBarItem, message: string) => { + errorColors(statusBarItem) statusBarItem.text = `$(error) FTL` statusBarItem.tooltip = message - } + }, + buildRunning: (statusBarItem: vscode.StatusBarItem) => { + loadingColors(statusBarItem) + statusBarItem.text = `$(gear~spin) FTL` + statusBarItem.tooltip = 'FTL project building...' + }, + buildOK: (statusBarItem: vscode.StatusBarItem) => { + resetColors(statusBarItem) + statusBarItem.text = `$(zap) FTL` + statusBarItem.tooltip = 'FTL project is successfully built.' + }, + buildError: (statusBarItem: vscode.StatusBarItem, message: string) => { + errorColors(statusBarItem) + statusBarItem.text = `$(error) FTL` + statusBarItem.tooltip = message + }, } diff --git a/lsp/lsp.go b/lsp/lsp.go index f255d9e8ff..631769c3b3 100644 --- a/lsp/lsp.go +++ b/lsp/lsp.go @@ -16,6 +16,7 @@ import ( "github.com/tliron/kutil/version" "github.com/TBD54566975/ftl/backend/schema" + "github.com/TBD54566975/ftl/buildengine" ftlErrors "github.com/TBD54566975/ftl/internal/errors" "github.com/TBD54566975/ftl/internal/log" ) @@ -71,9 +72,10 @@ func (s *Server) Run() error { type errSet []*schema.Error -// BuildStarted clears diagnostics for the given directory. New errors will arrive later if they still exist. -func (s *Server) BuildStarted(dir string) { - dirURI := "file://" + dir +// OnBuildStarted clears diagnostics for the given directory. New errors will arrive later if they still exist. +// Also emit an FTL message to set the status. +func (s *Server) OnBuildStarted(module buildengine.Module) { + dirURI := "file://" + module.Config.Dir s.diagnostics.Range(func(uri protocol.DocumentUri, diagnostics []protocol.Diagnostic) bool { if strings.HasPrefix(uri, dirURI) { @@ -82,6 +84,16 @@ func (s *Server) BuildStarted(dir string) { } return true }) + + s.publishBuildState(buildStateBuilding, nil) +} + +func (s *Server) OnBuildSuccess() { + s.publishBuildState(buildStateSuccess, nil) +} + +func (s *Server) OnBuildFailed(err error) { + s.publishBuildState(buildStateFailure, err) } // Post sends diagnostics to the client. @@ -182,6 +194,33 @@ func (s *Server) publishDiagnostics(uri protocol.DocumentUri, diagnostics []prot }) } +type buildState string + +const ( + buildStateBuilding buildState = "building" + buildStateSuccess buildState = "success" + buildStateFailure buildState = "failure" +) + +type buildStateMessage struct { + State buildState `json:"state"` + Err string `json:"error,omitempty"` +} + +func (s *Server) publishBuildState(state buildState, err error) { + msg := buildStateMessage{State: state} + if err != nil { + msg.Err = err.Error() + } + + s.logger.Debugf("Publishing build state: %s\n", msg) + if s.glspContext == nil { + return + } + + go s.glspContext.Notify("ftl/buildState", msg) +} + func (s *Server) initialize() protocol.InitializeFunc { return func(context *glsp.Context, params *protocol.InitializeParams) (any, error) { s.glspContext = context