Skip to content

Commit

Permalink
Improves error reporting
Browse files Browse the repository at this point in the history
- In concise mode, errors are rendered cleanly at the end of the terminal output
- Silenced cobra error logging to avoid breaking our TUI output
- Added error detection for language host startup
- Added error messages to clarify issues with the klotho sdk installation
  • Loading branch information
DavidSeptimus-Klotho committed Aug 16, 2024
1 parent e1b3124 commit e9f8535
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 33 deletions.
14 changes: 10 additions & 4 deletions cmd/klotho/down.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func newDownCmd() *cobra.Command {
}

func getProjectPath(ctx context.Context, inputPath string) (string, error) {
langHost, addr, err := language_host.StartPythonClient(ctx, language_host.DebugConfig{
langHost, srvState, err := language_host.StartPythonClient(ctx, language_host.DebugConfig{
Port: downConfig.debugPort,
Mode: downConfig.debugMode,
}, filepath.Dir(inputPath))
Expand All @@ -62,15 +62,21 @@ func getProjectPath(ctx context.Context, inputPath string) (string, error) {
log.Debug("Waiting for Python server to start")
if downConfig.debugMode != "" {
// Don't add a timeout in case there are breakpoints in the language host before an address is printed
<-addr.HasAddr
select {
case <-srvState.HasAddr:
case <-srvState.HasErr:
return "", srvState.Error
}
} else {
select {
case <-addr.HasAddr:
case <-srvState.HasAddr:
case <-time.After(30 * time.Second):
return "", errors.New("timeout waiting for Python server to start")
case <-srvState.HasErr:
return "", srvState.Error
}
}
conn, err := grpc.NewClient(addr.Address, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.NewClient(srvState.Address, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
return "", fmt.Errorf("failed to connect to Python server: %w", err)
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/klotho/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ func cli() int {
}()

var rootCmd = &cobra.Command{
Use: "app",
SilenceUsage: true,
Use: "app",
SilenceUsage: true,
SilenceErrors: true,
}

rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {
Expand Down
16 changes: 11 additions & 5 deletions pkg/k2/language_host/ir_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,30 @@ func (irs *LanguageHost) Start(ctx context.Context, debug DebugConfig, pythonPat

irs.debugCfg = debug

var addr *ServerAddress
irs.langHost, addr, err = StartPythonClient(ctx, debug, pythonPath)
var srvState *ServerState
irs.langHost, srvState, err = StartPythonClient(ctx, debug, pythonPath)
if err != nil {
return
}
log.Debug("Waiting for Python server to start")
if debug.Enabled() {
// Don't add a timeout in case there are breakpoints in the language host before an address is printed
<-addr.HasAddr
select {
case <-srvState.HasAddr:
case <-srvState.HasErr:
return srvState.Error
}
} else {
select {
case <-addr.HasAddr:
case <-srvState.HasAddr:
case <-time.After(30 * time.Second):
return errors.New("timeout waiting for Python server to start")
case <-srvState.HasErr:
return srvState.Error
}
}

irs.conn, err = grpc.NewClient(addr.Address, grpc.WithTransportCredentials(insecure.NewCredentials()))
irs.conn, err = grpc.NewClient(srvState.Address, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
return fmt.Errorf("failed to connect to Python server: %w", err)
}
Expand Down
30 changes: 23 additions & 7 deletions pkg/k2/language_host/language_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package language_host
import (
"context"
_ "embed"
"errors"
"fmt"
"io"
"os"
"os/exec"
"regexp"
"strings"
"syscall"

"github.com/klothoplatform/klotho/pkg/command"
Expand All @@ -19,15 +21,26 @@ import (
//go:embed python/python_language_host.py
var pythonLanguageHost string

type ServerAddress struct {
type ServerState struct {
Log *zap.SugaredLogger
Address string
Error error
HasErr chan error
HasAddr chan struct{}
}

func NewServerState(log *zap.SugaredLogger) *ServerState {
return &ServerState{
Log: log,
HasErr: make(chan error),
HasAddr: make(chan struct{}),
}
}

var listenOnPattern = regexp.MustCompile(`(?m)^\s*Listening on (\S+)$`)
var exceptionPattern = regexp.MustCompile(`(?s)(?:^|\n)\s*Exception occurred: (.+)$`)

func (f *ServerAddress) Write(b []byte) (int, error) {
func (f *ServerState) Write(b []byte) (int, error) {
if f.Address != "" {
return len(b), nil
}
Expand All @@ -39,6 +52,12 @@ func (f *ServerAddress) Write(b []byte) (int, error) {
f.Log.Debugf("Found language host listening on %s", f.Address)
close(f.HasAddr)
}
matches = exceptionPattern.FindStringSubmatch(s)
if len(matches) >= 2 {
f.Error = errors.New(strings.TrimSpace(matches[1]))
close(f.HasErr)
close(f.HasAddr)
}

return len(b), nil
}
Expand Down Expand Up @@ -66,7 +85,7 @@ func copyToTempDir(name, content string) (string, error) {

}

func StartPythonClient(ctx context.Context, debugConfig DebugConfig, pythonPath string) (*exec.Cmd, *ServerAddress, error) {
func StartPythonClient(ctx context.Context, debugConfig DebugConfig, pythonPath string) (*exec.Cmd, *ServerState, error) {
log := logging.GetLogger(ctx).Sugar()
hostPath, err := copyToTempDir("python_language_host", pythonLanguageHost)
if err != nil {
Expand Down Expand Up @@ -97,10 +116,7 @@ func StartPythonClient(ctx context.Context, debugConfig DebugConfig, pythonPath
}
cmd.Env = append(cmd.Env, "PYTHONPATH="+pythonPath)

lf := &ServerAddress{
Log: log,
HasAddr: make(chan struct{}),
}
lf := NewServerState(log)
cmd.Stdout = io.MultiWriter(cmd.Stdout, lf)
command.SetProcAttr(cmd)

Expand Down
29 changes: 25 additions & 4 deletions pkg/k2/language_host/python/python_language_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,38 @@
from concurrent import futures
from enum import Enum
from time import sleep
import importlib.util
from typing import Optional

import grpc
import yaml
from klotho import service_pb2, service_pb2_grpc
from klotho.runtime import instance as runtime


class DebugMode(Enum):
NONE = 0
VSCODE = 1
INTELLIJ = 2

class CLIErrorReporter:
def __init__(self, error_message: Optional[str] = None):
self.error_message = error_message
def __enter__(self):
pass

def __exit__(self, exc_type, exc_value, traceback):
message = self.error_message if self.error_message else (f"Exception occurred: {exc_type.__name__}: {exc_value}" if exc_type is not None else "The language host exited unexpectedly.")
if exc_type is not None:
print(f"Exception occurred: {message}", flush=True)

with CLIErrorReporter("The Klotho Python IaC SDK is not installed. Please install it by running `pipenv install klotho`."):
spec = importlib.util.find_spec("klotho")
if spec is None:
raise ModuleNotFoundError

with CLIErrorReporter("The Klotho Python IaC SDK could not be imported. Please check the installation."):
# TODO: Add a check for the minimum required version of the SDK
from klotho import service_pb2, service_pb2_grpc
from klotho.runtime import instance as runtime


def configure_debugging(port: int, mode: DebugMode = DebugMode.VSCODE):
if mode == DebugMode.VSCODE:
Expand Down Expand Up @@ -113,4 +133,5 @@ def signal_handler(sig, frame):


if __name__ == "__main__":
cli()
with CLIErrorReporter("The Python language host exited unexpectedly."):
cli()
10 changes: 9 additions & 1 deletion pkg/tui/log_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (c *LogCore) With(f []zapcore.Field) zapcore.Core {
if c.verbosity.CombineLogs() {
field.AddTo(nc.enc)
}
// else (if the field is the construct and we're not combining logs) don't add it to the encoder
// else (if the field is the construct and we're not combining errors) don't add it to the encoder
// because the log lines will already be in its own construct section of the output.
} else {
field.AddTo(nc.enc)
Expand Down Expand Up @@ -93,6 +93,14 @@ func (c *LogCore) Write(ent zapcore.Entry, fields []zapcore.Field) error {
s := buf.String()
s = strings.TrimSuffix(s, "\n")

if c.construct == "" && zapcore.ErrorLevel.Enabled(ent.Level) {
c.program.Send(ErrorMessage{
Message: s,
})
buf.Free()
return nil
}

c.program.Send(LogMessage{
Construct: construct,
Message: s,
Expand Down
8 changes: 7 additions & 1 deletion pkg/tui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type (
consoleWidth int
constructWidth int
statusWidth int
errors []string
}

constructModel struct {
Expand Down Expand Up @@ -81,7 +82,8 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
cm := m.constructModel(msg.Construct)
cm.logs.Push(msg.Message)
}

case ErrorMessage:
m.errors = append(m.errors, msg.Message)
case UpdateMessage:
cm := m.constructModel(msg.Construct)

Expand Down Expand Up @@ -195,6 +197,10 @@ func (m *model) viewCompact() string {
}
}
}
for _, log := range m.errors {
sb.WriteString(log)
sb.WriteRune('\n')
}
return sb.String()
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/tui/program.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ type (
Value any
}

ErrorMessage struct {
Message string
}

TuiProgress struct {
Prog *tea.Program
Construct string
Expand Down
12 changes: 3 additions & 9 deletions pkg/tui/verbosity.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,8 @@ func (v Verbosity) LogLevel() zapcore.Level {
}
}

// CombineLogs controls whether to show all logs comingled in the TUI. In other words,
// sorted by timestamp, not grouped by construct.
// CombineLogs controls whether to show all errors commingled in the TUI.
// In other words, sorted by timestamp, not grouped by construct.
func (v Verbosity) CombineLogs() bool {
switch v {
case VerbosityConcise, VerbosityDebugMore:
return true

default:
return false
}
return VerbosityDebugMore == v
}

0 comments on commit e9f8535

Please sign in to comment.