Fix multi-line. Ensure Init errors are correctly reported. Closes #2260

pkg/interactive/interactive_client.go

@@ -51,14 +51,14 @@ type InteractiveClient struct {
 	cancelPrompt      context.CancelFunc
 	// channel used internally to pass the initialisation result
 	initResultChan chan *db_common.InitResult
-	afterClose     AfterPromptCloseAction
+	// flag set when initialisation is complete (with or without errors)
+	initialisationComplete bool
+	afterClose             AfterPromptCloseAction
 	// lock while execution is occurring to avoid errors/warnings being shown
 	executionLock sync.Mutex
-
 	// the schema metadata - this is loaded asynchronously during init
 	schemaMetadata *schema.Metadata
-
-	highlighter *Highlighter
+	highlighter    *Highlighter
 }
 
 func getHighlighter(theme string) *Highlighter {
@@ -91,14 +91,13 @@ func newInteractiveClient(ctx context.Context, initData *query.InitData, results
 }
 
 // InteractivePrompt starts an interactive prompt and return
-func (c *InteractiveClient) InteractivePrompt(ctx context.Context) {
+func (c *InteractiveClient) InteractivePrompt(parentContext context.Context) {
 	// start a cancel handler for the interactive client - this will call activeQueryCancelFunc if it is set
 	// (registered when we call createQueryContext)
 	quitChannel := c.startCancelHandler()
 
 	// create a cancel context for the prompt - this will set c.cancelPrompt
-	parentContext := ctx
-	ctx = c.createPromptContext(parentContext)
+	ctx := c.createPromptContext(parentContext)
 
 	defer func() {
 		if r := recover(); r != nil {
@@ -140,8 +139,8 @@ func (c *InteractiveClient) InteractivePrompt(ctx context.Context) {
 			if c.afterClose == AfterPromptCloseExit {
 				return
 			}
-			// create new context
-			ctx = c.createPromptContext(parentContext)
+			// create new context with a cancellation func
+			ctx := c.createPromptContext(parentContext)
 			// now run it again
 			c.runInteractivePromptAsync(ctx, &promptResultChan)
 		}
@@ -312,20 +311,17 @@ func (c *InteractiveClient) executor(ctx context.Context, line string) {
 	c.afterClose = AfterPromptCloseRestart
 
 	line = strings.TrimSpace(line)
-	// store the history (the raw line which was entered)
-	// we want to store even if we fail to resolve a query
-	c.interactiveQueryHistory.Push(line)
 
-	query, err := c.getQuery(ctx, line)
+	query := c.getQuery(ctx, line)
 	if query == "" {
-		if err != nil {
-			utils.ShowError(ctx, utils.HandleCancelError(err))
-		}
-		// restart the prompt
+		// we failed to resolve a query, or are in the middle of a multi-line entry
+		// restart the prompt, DO NOT clear the interactive buffer
 		c.restartInteractiveSession()
 		return
 	}
 
+	// we successfully retrieved a query
+
 	// create a  context for the execution of the query
 	queryContext := c.createQueryContext(ctx)
 
@@ -350,12 +346,22 @@ func (c *InteractiveClient) executor(ctx context.Context, line string) {
 	c.restartInteractiveSession()
 }
 
-func (c *InteractiveClient) getQuery(ctx context.Context, line string) (string, error) {
+func (c *InteractiveClient) getQuery(ctx context.Context, line string) string {
 	// if it's an empty line, then we don't need to do anything
 	if line == "" {
-		return "", nil
+		return ""
 	}
 
+	// store the history (the raw line which was entered)
+	historyEntry := line
+	defer func() {
+		if len(historyEntry) > 0 {
+			// we want to store even if we fail to resolve a query
+			c.interactiveQueryHistory.Push(historyEntry)
+		}
+
+	}()
+
 	// wait for initialisation to complete so we can access the workspace
 	if !c.isInitialised() {
 		// create a context used purely to detect cancellation during initialisation
@@ -371,8 +377,12 @@ func (c *InteractiveClient) getQuery(ctx context.Context, line string) (string,
 		err := c.waitForInitData(queryContext)
 		statushooks.Done(ctx)
 		if err != nil {
-			// if it failed, report error and quit
-			return "", err
+			// clear history entry
+			historyEntry = ""
+			// clear the interactive buffer
+			c.interactiveBuffer = nil
+			// error will have been handled elsewhere
+			return ""
 		}
 	}
 
@@ -385,29 +395,45 @@ func (c *InteractiveClient) getQuery(ctx context.Context, line string) (string,
 	// in case of a named query call with params, parse the where clause
 	query, _, err := c.workspace().ResolveQueryAndArgsFromSQLString(queryString)
 	if err != nil {
-		// if we fail to resolve, show error but do not return it - we want to stay in the prompt
+		// if we fail to resolve:
+		// - show error but do not return it so we  stay in the prompt
+		// - do not clear history item - we want to store bad entry in history
+		// - clear interactive buffer
+		c.interactiveBuffer = nil
 		utils.ShowError(ctx, err)
-		return "", nil
+		return ""
 	}
 	isNamedQuery := query != queryString
 
-	// if it is a multiline query, execute even without `;`
-	if !isNamedQuery {
-		// should we execute?
-		if !c.shouldExecute(queryString) {
-			return "", nil
-		}
+	// should we execute?
+	// we will NOT execute if we are in multiline mode, there is no semi-colon
+	// and it is NOT a metaquery or a named query
+	if !c.shouldExecute(queryString, isNamedQuery) {
+		// is we are not executing, do not store history
+		historyEntry = ""
+		// do not clear interactive buffer
+		return ""
 	}
 
-	// so we need to execute - what are we executing
+	// so we need to execute
+	// clear the interactive buffer
+	c.interactiveBuffer = nil
+
+	// what are we executing?
 
 	// if the line is ONLY a semicolon, do nothing and restart interactive session
 	if strings.TrimSpace(query) == ";" {
+		// do not store in history
+		historyEntry = ""
 		c.restartInteractiveSession()
-		return "", nil
+		return ""
+	}
+	// if this is a multiline query, update history entry
+	if len(strings.Split(query, "\n")) > 1 {
+		historyEntry = query
 	}
 
-	return query, nil
+	return query
 }
 
 func (c *InteractiveClient) executeMetaquery(ctx context.Context, query string) error {
@@ -439,14 +465,29 @@ func (c *InteractiveClient) executeMetaquery(ctx context.Context, query string)
 }
 
 func (c *InteractiveClient) restartInteractiveSession() {
-	// empty the buffer
-	c.interactiveBuffer = []string{}
 	// restart the prompt
 	c.ClosePrompt(c.afterClose)
 }
 
-func (c *InteractiveClient) shouldExecute(line string) bool {
-	return !cmdconfig.Viper().GetBool(constants.ArgMultiLine) || strings.HasSuffix(line, ";") || metaquery.IsMetaQuery(line)
+func (c *InteractiveClient) shouldExecute(line string, namedQuery bool) bool {
+	if namedQuery {
+		// execute named queries with no ';' even in multiline mode
+		return true
+	}
+	if !cmdconfig.Viper().GetBool(constants.ArgMultiLine) {
+		// NOT multiline mode
+		return true
+	}
+	if metaquery.IsMetaQuery(line) {
+		// execute metaqueries with no ';' even in multiline mode
+		return true
+	}
+	if strings.HasSuffix(line, ";") {
+		// statement has terminating ';'
+		return true
+	}
+
+	return false
 }
 
 func (c *InteractiveClient) queryCompleter(d prompt.Document) []prompt.Suggest {

pkg/interactive/interactive_client_init.go

@@ -14,25 +14,27 @@ import (
 	"github.com/turbot/steampipe/pkg/workspace"
 )
 
-var initTimeout = 40 * time.Second
-
 // init data has arrived, handle any errors/warnings/messages
 func (c *InteractiveClient) handleInitResult(ctx context.Context, initResult *db_common.InitResult) {
 	// try to take an execution lock, so that we don't end up showing warnings and errors
 	// while an execution is underway
 	c.executionLock.Lock()
 	defer c.executionLock.Unlock()
 
-	if utils.IsContextCancelled(ctx) {
-		log.Printf("[TRACE] prompt context has been cancelled - not handling init result")
+	if initResult.Error != nil {
+		c.ClosePrompt(AfterPromptCloseExit)
+		// add newline to ensure error is not printed at end of current prompt line
+		fmt.Println()
+		utils.ShowError(ctx, initResult.Error)
 		return
 	}
 
-	if initResult.Error != nil {
+	if utils.IsContextCancelled(ctx) {
 		c.ClosePrompt(AfterPromptCloseExit)
 		// add newline to ensure error is not printed at end of current prompt line
 		fmt.Println()
 		utils.ShowError(ctx, initResult.Error)
+		log.Printf("[TRACE] prompt context has been cancelled - not handling init result")
 		return
 	}
 
@@ -56,13 +58,15 @@ func (c *InteractiveClient) readInitDataStream(ctx context.Context) {
 			utils.ShowError(ctx, helpers.ToError(r))
 
 		}
+		// whatever happens, set initialisationComplete
+		c.initialisationComplete = true
 	}()
+
 	<-c.initData.Loaded
 
 	defer func() { c.initResultChan <- c.initData.Result }()
 
 	if c.initData.Result.Error != nil {
-
 		return
 	}
 
@@ -93,10 +97,11 @@ func (c *InteractiveClient) workspaceWatcherErrorHandler(ctx context.Context, er
 // return whether the client is initialises
 // there are 3 conditions>
 func (c *InteractiveClient) isInitialised() bool {
-	return c.initData != nil && c.schemaMetadata != nil
+	return c.initialisationComplete
 }
 
 func (c *InteractiveClient) waitForInitData(ctx context.Context) error {
+	var initTimeout = 40 * time.Second
 	ticker := time.NewTicker(20 * time.Millisecond)
 	for {
 		select {

pkg/query/init_data.go

@@ -3,7 +3,6 @@ package query
 import (
 	"context"
 	"fmt"
-
 	"github.com/spf13/viper"
 	"github.com/turbot/go-kit/helpers"
 	"github.com/turbot/steampipe-plugin-sdk/v4/telemetry"