Skip to content

Commit

Permalink
fix: panic on shutdown (#2810)
Browse files Browse the repository at this point in the history
Also some cleanup around shutdown handling

fixes: #2807
  • Loading branch information
stuartwdouglas authored Sep 25, 2024
1 parent bda7e15 commit e2c54bc
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 37 deletions.
7 changes: 5 additions & 2 deletions backend/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import (
"github.com/TBD54566975/ftl/backend/schema"
frontend "github.com/TBD54566975/ftl/frontend/console"
cf "github.com/TBD54566975/ftl/internal/configuration/manager"
status "github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/cors"
ftlhttp "github.com/TBD54566975/ftl/internal/http"
"github.com/TBD54566975/ftl/internal/log"
Expand All @@ -70,6 +69,7 @@ import (
"github.com/TBD54566975/ftl/internal/rpc/headers"
"github.com/TBD54566975/ftl/internal/sha256"
"github.com/TBD54566975/ftl/internal/slices"
status "github.com/TBD54566975/ftl/internal/terminal"
)

// CommonConfig between the production controller and development server.
Expand Down Expand Up @@ -1846,7 +1846,10 @@ func (s *Service) syncSchema(ctx context.Context) {
})
if err != nil {
next := retry.Duration()
logger.Warnf("Failed to watch module changes, retrying in %s: %s", next, err)
if ctx.Err() == nil {
// Don't log when the context is done
logger.Warnf("Failed to watch module changes, retrying in %s: %s", next, err)
}
select {
case <-time.After(next):
case <-ctx.Done():
Expand Down
2 changes: 1 addition & 1 deletion frontend/cli/cmd_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import (
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/go-runtime/ftl/reflection"
status "github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/rpc"
status "github.com/TBD54566975/ftl/internal/terminal"
)

type callCmd struct {
Expand Down
6 changes: 3 additions & 3 deletions frontend/cli/cmd_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import (

"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/internal/buildengine"
"github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/lsp"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/rpc"
"github.com/TBD54566975/ftl/internal/terminal"
)

type devCmd struct {
Expand All @@ -39,7 +39,7 @@ func (d *devCmd) Run(ctx context.Context, k *kong.Kong, projConfig projectconfig
}

client := rpc.ClientFromContext[ftlv1connect.ControllerServiceClient](ctx)
console.LaunchEmbeddedConsole(ctx, k, projConfig, bindContext, cancel, client)
terminal.LaunchEmbeddedConsole(ctx, k, projConfig, bindContext, cancel, client)

g, ctx := errgroup.WithContext(ctx)

Expand All @@ -56,7 +56,7 @@ func (d *devCmd) Run(ctx context.Context, k *kong.Kong, projConfig projectconfig
fmt.Println(dsn)
return nil
}
sm := console.FromContext(ctx)
sm := terminal.FromContext(ctx)
starting := sm.NewStatus("\u001B[92mStarting FTL Server 🚀\u001B[39m")
// cmdServe will notify this channel when startup commands are complete and the controller is ready
controllerReady := make(chan bool, 1)
Expand Down
6 changes: 3 additions & 3 deletions frontend/cli/cmd_interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import (
"github.com/alecthomas/kong"

"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/terminal"
)

type interactiveCmd struct {
}

func (i *interactiveCmd) Run(ctx context.Context, k *kong.Kong, projectConfig projectconfig.Config, binder console.KongContextBinder, cancel context.CancelFunc, client ftlv1connect.ControllerServiceClient) error {
err := console.RunInteractiveConsole(ctx, k, projectConfig, binder, nil, cancel, client)
func (i *interactiveCmd) Run(ctx context.Context, k *kong.Kong, projectConfig projectconfig.Config, binder terminal.KongContextBinder, cancel context.CancelFunc, client ftlv1connect.ControllerServiceClient) error {
err := terminal.RunInteractiveConsole(ctx, k, projectConfig, binder, nil, cancel, client)
if err != nil {
return fmt.Errorf("interactive console: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/cli/cmd_replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import (
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/go-runtime/ftl/reflection"
status "github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/rpc"
status "github.com/TBD54566975/ftl/internal/terminal"
)

type replayCmd struct {
Expand Down
4 changes: 2 additions & 2 deletions frontend/cli/cmd_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/terminal"
)

type statusCmd struct {
Expand All @@ -36,6 +36,6 @@ func (s *statusCmd) Run(ctx context.Context, client ftlv1connect.ControllerServi
if err != nil {
return fmt.Errorf("failed to marshal status: %w", err)
}
console.PrintJSON(ctx, data)
terminal.PrintJSON(ctx, data)
return nil
}
12 changes: 6 additions & 6 deletions frontend/cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
"github.com/TBD54566975/ftl"
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
_ "github.com/TBD54566975/ftl/internal/automaxprocs" // Set GOMAXPROCS to match Linux container CPU quota.
"github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/rpc"
"github.com/TBD54566975/ftl/internal/terminal"
)

type InteractiveCLI struct {
Expand Down Expand Up @@ -78,7 +78,7 @@ func main() {
}

if !cli.Plain {
sm := console.NewStatusManager(ctx)
sm := terminal.NewStatusManager(ctx)
ctx = sm.IntoContext(ctx)
defer sm.Close()
}
Expand All @@ -95,7 +95,7 @@ func main() {
kctx.Fatalf("could not determine default config path, either place an ftl-project.toml file in the root of your project, use --config=FILE, or set the FTL_CONFIG envar")
}
}
if console.IsANSITerminal(ctx) {
if terminal.IsANSITerminal(ctx) {
cli.LogConfig.Color = true
}

Expand Down Expand Up @@ -152,7 +152,7 @@ func createKongApplication(cli any) *kong.Kong {
return app
}

var _ console.KongContextBinder = bindContext
var _ terminal.KongContextBinder = bindContext

func bindContext(ctx context.Context, kctx *kong.Context, projectConfig projectconfig.Config, app *kong.Kong, cancel context.CancelFunc) context.Context {

Expand All @@ -163,15 +163,15 @@ func bindContext(ctx context.Context, kctx *kong.Context, projectConfig projectc
ctx = rpc.ContextWithClient(ctx, controllerServiceClient)
kctx.BindTo(controllerServiceClient, (*ftlv1connect.ControllerServiceClient)(nil))

kongcompletion.Register(app, kongcompletion.WithPredictors(console.Predictors(ctx, controllerServiceClient)))
kongcompletion.Register(app, kongcompletion.WithPredictors(terminal.Predictors(ctx, controllerServiceClient)))

verbServiceClient := rpc.Dial(ftlv1connect.NewVerbServiceClient, cli.Endpoint.String(), log.Error)
ctx = rpc.ContextWithClient(ctx, verbServiceClient)
kctx.BindTo(verbServiceClient, (*ftlv1connect.VerbServiceClient)(nil))

kctx.Bind(cli.Endpoint)
kctx.BindTo(ctx, (*context.Context)(nil))
kctx.BindTo(bindContext, (*console.KongContextBinder)(nil))
kctx.BindTo(bindContext, (*terminal.KongContextBinder)(nil))
kctx.BindTo(cancel, (*context.CancelFunc)(nil))
return ctx
}
20 changes: 10 additions & 10 deletions internal/buildengine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import (
ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/internal/console"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/moduleconfig"
"github.com/TBD54566975/ftl/internal/rpc"
"github.com/TBD54566975/ftl/internal/slices"
"github.com/TBD54566975/ftl/internal/terminal"
)

type CompilerBuildError struct {
Expand Down Expand Up @@ -356,19 +356,19 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration
e.schemaChanges.Subscribe(schemaChanges)
defer func() {
e.schemaChanges.Unsubscribe(schemaChanges)
close(schemaChanges)
}()

watchEvents := make(chan WatchEvent, 128)
ctx, cancel := context.WithCancel(ctx)
topic, err := e.watcher.Watch(ctx, period, e.moduleDirs)
if err != nil {
cancel()
return err
}
topic.Subscribe(watchEvents)
defer func() {
topic.Unsubscribe(watchEvents)
topic.Close()
close(watchEvents)
// Cancel will close the topic and channel
cancel()
}()

// Build and deploy all modules first.
Expand Down Expand Up @@ -464,7 +464,7 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration
if err != nil {
didError = true
e.reportBuildFailed(err)
console.UpdateModuleState(ctx, config.Module, console.BuildStateFailed)
terminal.UpdateModuleState(ctx, config.Module, terminal.BuildStateFailed)
logger.Errorf(err, "build and deploy failed for module %q", event.Module.Config.Module)
} else {
didUpdateDeployments = true
Expand Down Expand Up @@ -547,7 +547,7 @@ func (e *Engine) BuildAndDeploy(ctx context.Context, replicas int32, waitForDepl
return e.buildWithCallback(ctx, func(buildCtx context.Context, module Module) error {
buildGroup.Go(func() error {
e.modulesToBuild.Store(module.Config.Module, false)
console.UpdateModuleState(ctx, module.Config.Module, console.BuildStateDeploying)
terminal.UpdateModuleState(ctx, module.Config.Module, terminal.BuildStateDeploying)
return Deploy(buildCtx, module, replicas, waitForDeployOnline, e.client)
})
return nil
Expand Down Expand Up @@ -597,7 +597,7 @@ func (e *Engine) buildWithCallback(ctx context.Context, callback buildCallback,
e.moduleMetas.Store(name, moduleMeta{module: module})
mustBuild[name] = true

console.UpdateModuleState(ctx, name, console.BuildStateWaiting)
terminal.UpdateModuleState(ctx, name, terminal.BuildStateWaiting)
}
graph, err := e.Graph(moduleNames...)
if err != nil {
Expand Down Expand Up @@ -724,7 +724,7 @@ func (e *Engine) mustSchema(ctx context.Context, moduleName string, builtModules
//
// Assumes that all dependencies have been built and are available in "built".
func (e *Engine) build(ctx context.Context, moduleName string, builtModules map[string]*schema.Module, schemas chan<- *schema.Module) error {
console.UpdateModuleState(ctx, moduleName, console.BuildStateBuilding)
terminal.UpdateModuleState(ctx, moduleName, terminal.BuildStateBuilding)
meta, ok := e.moduleMetas.Load(moduleName)
if !ok {
return fmt.Errorf("module %q not found", moduleName)
Expand All @@ -745,7 +745,7 @@ func (e *Engine) build(ctx context.Context, moduleName string, builtModules map[
if err != nil {
return fmt.Errorf("could not load schema for module %q: %w", config.Module, err)
}
console.UpdateModuleState(ctx, moduleName, console.BuildStateBuilt)
terminal.UpdateModuleState(ctx, moduleName, terminal.BuildStateBuilt)
schemas <- moduleSchema
return nil
}
Expand Down
8 changes: 6 additions & 2 deletions internal/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package exec
import (
"context"
"errors"
"fmt"
"os"
"os/exec" //nolint:depguard
"syscall"
Expand Down Expand Up @@ -64,8 +65,11 @@ func (c *Cmd) RunBuffered(ctx context.Context) error {

err := c.Run()
if err != nil {
log.FromContext(ctx).Errorf(err, "%s", outputBuffer.Bytes())
return err
if ctx.Err() == nil {
// Don't log on context cancellation
log.FromContext(ctx).Errorf(err, "%s", outputBuffer.Bytes())
}
return fmt.Errorf("command failed: %w", err)
}

return nil
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package console
package terminal

import (
"context"
"errors"
"fmt"
"io"
"os"
"strings"

"github.com/alecthomas/kong"
Expand All @@ -26,9 +27,10 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, projectConfig proj
l, err := readline.NewEx(&readline.Config{
Prompt: "\033[32m>\033[0m ",
InterruptPrompt: "^C",
EOFPrompt: "exit",
AutoComplete: &FTLCompletion{app: k, ctx: ctx, client: client},
Listener: &ExitListener{cancel: cancelContext},
Listener: &ExitListener{cancel: func() {
os.Exit(0)
}},
})
if refreshFunction != nil {
refreshFunction(l.Refresh)
Expand All @@ -50,7 +52,7 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, projectConfig proj
}
continue
} else if errors.Is(err, io.EOF) {
break
os.Exit(0)
}
line = strings.TrimSpace(line)
if line == "" {
Expand Down
2 changes: 1 addition & 1 deletion internal/console/noop.go → internal/terminal/noop.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package console
package terminal

import "context"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package console
package terminal

import (
"context"
Expand Down
2 changes: 1 addition & 1 deletion internal/console/status.go → internal/terminal/status.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package console
package terminal

import (
"bytes"
Expand Down

0 comments on commit e2c54bc

Please sign in to comment.