add exit events and embed into eventloop (#21)

Signed-off-by: Alex Goodman <[email protected]>

application.go

@@ -114,7 +114,17 @@ func (a *application) runInitializers() error {
 
 func (a *application) WrapRunE(fn func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) error {
 	return func(cmd *cobra.Command, args []string) error {
-		return a.execute(cmd.Context(), async(cmd, args, fn))
+		wrapper := func(cmd *cobra.Command, args []string) error {
+			defer func() {
+				// when the worker has completed (or errored) we want to exit the event loop gracefully
+				if a.state.Bus != nil {
+					a.state.Bus.Publish(ExitEvent(false))
+				}
+			}()
+			return fn(cmd, args)
+		}
+
+		return a.execute(cmd.Context(), async(cmd, args, wrapper))
 	}
 }
 
@@ -201,12 +211,9 @@ func (a *application) Run() {
 		panic(fmt.Errorf(setupRootCommandNotCalledError))
 	}
 
-	cmd := a.root
-	log := a.state.Logger
-
 	// drive application control from a single context which can be cancelled (notifying the event loop to stop)
 	ctx, cancel := context.WithCancel(context.Background())
-	cmd.SetContext(ctx)
+	a.root.SetContext(ctx)
 
 	// note: it is important to always do signal handling from the main package. In this way if quill is used
 	// as a lib a refactor would not need to be done (since anything from the main package cannot be imported this
@@ -230,16 +237,16 @@ func (a *application) Run() {
 	go func() {
 		select {
 		case <-signals: // first signal, cancel context
-			log.Trace("signal interrupt, stop requested")
+			a.state.Logger.Trace("signal interrupt, stop requested")
 			cancel()
 		case <-ctx.Done():
 		}
 		<-signals // second signal, hard exit
-		log.Trace("signal interrupt, killing")
+		a.state.Logger.Trace("signal interrupt, killing")
 		exitCode = 1
 	}()
 
-	if err := cmd.Execute(); err != nil {
+	if err := a.root.Execute(); err != nil {
 		color.Red.Println(strings.TrimSpace(err.Error()))
 		exitCode = 1
 	}

application_test.go

@@ -353,3 +353,30 @@ func Test_RunExitError(t *testing.T) {
 	}
 	t.Fatalf("process ran with err %v, want exit status 1", err)
 }
+
+func Test_Run_InvokesBusExit(t *testing.T) {
+	runCalled := false
+
+	var sub *partybus.Subscription
+
+	app := New(*NewSetupConfig(Identification{}).WithInitializers(
+		func(state *State) error {
+			sub = state.Bus.Subscribe()
+			return nil
+		},
+	))
+	app.SetupRootCommand(&cobra.Command{
+		RunE: func(cmd *cobra.Command, args []string) error {
+			runCalled = true
+			return nil
+		},
+	})
+	app.Run()
+	require.True(t, runCalled)
+
+	events := sub.Events()
+
+	e := <-events
+
+	assert.Equal(t, e.Type, ExitEventType)
+}

event.go

@@ -0,0 +1,21 @@
+package clio
+
+import (
+	"os"
+
+	"github.com/wagoodman/go-partybus"
+)
+
+const ExitEventType partybus.EventType = "clio-exit"
+
+func ExitEvent(interrupt bool) partybus.Event {
+	if interrupt {
+		return partybus.Event{
+			Type:  ExitEventType,
+			Value: os.Interrupt,
+		}
+	}
+	return partybus.Event{
+		Type: ExitEventType,
+	}
+}

eventloop.go

@@ -3,6 +3,7 @@ package clio
 import (
 	"context"
 	"errors"
+	"os"
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/wagoodman/go-partybus"
@@ -67,9 +68,36 @@ func eventloop(ctx context.Context, log logger.Logger, subscription *partybus.Su
 				events = nil
 				continue
 			}
+
+			if e.Type == ExitEventType {
+				events = nil
+
+				if e.Value == os.Interrupt {
+					// on top of listening to signals from the OS, we also listen to interrupt events from the UI.
+					// Why provide two ways to do the same thing? In an application that has a UI where the terminal
+					// has been set to raw mode, ctrl-c will not register as an interrupt signal to the application.
+					// Instead the UI will capture the ctrl-c and need to signal the event loop to exit gracefully.
+					// Using the same signal channel for both OS signals and UI signals is not advisable and requires
+					// injecting the channel into the UI (also not advisable). Providing a bus event for the UI to
+					// use is a better solution here.
+
+					log.Trace("signal interrupt")
+
+					workerErrs = nil
+					forceTeardown = true
+				} else {
+					log.Trace("signal exit")
+				}
+
+				if subscription != nil {
+					_ = subscription.Unsubscribe()
+				}
+			}
+
 			if ux == nil {
 				continue
 			}
+
 			if err := ux.Handle(e); err != nil {
 				if errors.Is(err, partybus.ErrUnsubscribe) {
 					events = nil

eventloop_test.go

@@ -38,9 +38,9 @@ func (u *uiMock) Handle(event partybus.Event) error {
 	return u.Called(event).Error(0)
 }
 
-func (u *uiMock) Teardown(_ bool) error {
+func (u *uiMock) Teardown(force bool) error {
 	u.t.Logf("UI Teardown called")
-	return u.Called().Error(0)
+	return u.Called(force).Error(0)
 }
 
 func Test_EventLoop_gracefulExit(t *testing.T) {
@@ -80,7 +80,7 @@ func Test_EventLoop_gracefulExit(t *testing.T) {
 		ux.On("Handle", finalEvent).Return(nil)
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(nil)
+		ux.On("Teardown", false).Return(nil)
 
 		assert.NoError(t,
 			eventloop(
@@ -131,7 +131,7 @@ func Test_EventLoop_workerError(t *testing.T) {
 
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(nil)
+		ux.On("Teardown", true).Return(nil)
 
 		// ensure we see an error returned
 		assert.ErrorIs(t,
@@ -190,7 +190,7 @@ func Test_EventLoop_unsubscribeError(t *testing.T) {
 		ux.On("Handle", finalEvent).Return(partybus.ErrUnsubscribe)
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(nil)
+		ux.On("Teardown", false).Return(nil)
 
 		// unsubscribe errors should be handled and ignored, not propagated. We are additionally asserting that
 		// this case is handled as a controlled shutdown (this test should not timeout)
@@ -249,7 +249,7 @@ func Test_EventLoop_handlerError(t *testing.T) {
 		ux.On("Handle", finalEvent).Return(finalEvent.Error)
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(nil)
+		ux.On("Teardown", false).Return(nil)
 
 		// handle errors SHOULD propagate the event loop. We are additionally asserting that this case is
 		// handled as a controlled shutdown (this test should not timeout)
@@ -292,7 +292,7 @@ func Test_EventLoop_contextCancelStopExecution(t *testing.T) {
 
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(nil)
+		ux.On("Teardown", true).Return(nil)
 
 		go cancel()
 
@@ -313,6 +313,122 @@ func Test_EventLoop_contextCancelStopExecution(t *testing.T) {
 	testWithTimeout(t, 5*time.Second, test)
 }
 
+func Test_EventLoop_ExitEventStopExecution(t *testing.T) {
+	test := func(t *testing.T) {
+
+		testBus := partybus.NewBus()
+		subscription := testBus.Subscribe()
+		t.Cleanup(testBus.Close)
+
+		finalEvent := ExitEvent(false)
+
+		worker := func() <-chan error {
+			ret := make(chan error)
+			go func() {
+				t.Log("worker running")
+				// send an empty item (which is ignored) ensuring we've entered the select statement,
+				// then close (a partial shutdown).
+				ret <- nil
+				t.Log("worker sent nothing")
+				close(ret)
+				t.Log("worker closed")
+				// do the other half of the shutdown
+				testBus.Publish(finalEvent)
+				t.Log("worker published final event")
+			}()
+			return ret
+		}
+
+		ux := &uiMock{
+			t: t,
+			// don't force unsubscribe, allow exit to cause it
+		}
+
+		// ensure the mock sees at least the final event... note the event error is propagated
+		ux.On("Handle", finalEvent).Return(nil)
+		// ensure the mock sees basic setup/teardown events
+		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
+		ux.On("Teardown", false).Return(nil)
+
+		// handle errors SHOULD propagate the event loop. We are additionally asserting that this case is
+		// handled as a controlled shutdown (this test should not timeout)
+		assert.ErrorIs(t,
+			eventloop(
+				context.Background(),
+				discard.New(),
+				subscription,
+				worker(),
+				ux,
+			),
+			finalEvent.Error,
+			"should have seen a event error, but did not",
+		)
+
+		ux.AssertExpectations(t)
+	}
+
+	// if there is a bug, then there is a risk of the event loop never returning
+	testWithTimeout(t, 5*time.Second, test)
+}
+
+func Test_EventLoop_InterruptEventKillExecution(t *testing.T) {
+	test := func(t *testing.T) {
+
+		testBus := partybus.NewBus()
+		subscription := testBus.Subscribe()
+		t.Cleanup(testBus.Close)
+
+		finalEvent := ExitEvent(true)
+
+		worker := func() <-chan error {
+			ret := make(chan error)
+			go func() {
+				t.Log("worker running")
+				// send an empty item (which is ignored) ensuring we've entered the select statement,
+				// then close (a partial shutdown).
+				ret <- nil
+				t.Log("worker sent nothing")
+				close(ret)
+				t.Log("worker closed")
+				// do the other half of the shutdown
+				testBus.Publish(finalEvent)
+				t.Log("worker published final event")
+			}()
+			return ret
+		}
+
+		ux := &uiMock{
+			t: t,
+			// don't force unsubscribe, allow exit to cause it
+		}
+
+		// ensure the mock sees at least the final event... note the event error is propagated
+		ux.On("Handle", finalEvent).Return(nil)
+		// ensure the mock sees basic setup/teardown events
+		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
+		ux.On("Teardown", true).Return(nil)
+
+		// handle errors SHOULD propagate the event loop. We are additionally asserting that this case is
+		// handled as a controlled shutdown (this test should not timeout)
+		assert.ErrorIs(t,
+			eventloop(
+				context.Background(),
+				discard.New(),
+				subscription,
+				worker(),
+				ux,
+			),
+			finalEvent.Error,
+			"should have seen a event error, but did not",
+		)
+
+		ux.AssertExpectations(t)
+	}
+
+	// if there is a bug, then there is a risk of the event loop never returning
+	testWithTimeout(t, 5*time.Second, test)
+}
+
 func Test_EventLoop_uiTeardownError(t *testing.T) {
 	test := func(t *testing.T) {
 
@@ -352,7 +468,7 @@ func Test_EventLoop_uiTeardownError(t *testing.T) {
 		ux.On("Handle", finalEvent).Return(nil)
 		// ensure the mock sees basic setup/teardown events
 		ux.On("Setup", mock.AnythingOfType("func() error")).Return(nil)
-		ux.On("Teardown").Return(teardownError)
+		ux.On("Teardown", false).Return(teardownError)
 
 		// ensure we see an error returned
 		assert.ErrorIs(t,