call OnActivate after plugin crash, update example (#7940)

2 files changed, 41 insertions, 13 deletions

diff --git a/plugin/rpcplugin/supervisor.go b/plugin/rpcplugin/supervisor.go
index 7b036d726..ad3c8401d 100644
--- a/plugin/rpcplugin/supervisor.go
+++ b/plugin/rpcplugin/supervisor.go
@@ -30,11 +30,11 @@ var _ plugin.Supervisor = (*Supervisor)(nil)
 
 // Starts the plugin. This method will block until the plugin is successfully launched for the first
 // time and will return an error if the plugin cannot be launched at all.
-func (s *Supervisor) Start() error {
+func (s *Supervisor) Start(api plugin.API) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	s.done = make(chan bool, 1)
 	start := make(chan error, 1)
-	go s.run(ctx, start)
+	go s.run(ctx, start, api)
 
 	select {
 	case <-time.After(time.Second * 3):
@@ -60,13 +60,13 @@ func (s *Supervisor) Hooks() plugin.Hooks {
 	return s.hooks.Load().(plugin.Hooks)
 }
 
-func (s *Supervisor) run(ctx context.Context, start chan<- error) {
+func (s *Supervisor) run(ctx context.Context, start chan<- error, api plugin.API) {
 	defer func() {
 		s.done <- true
 	}()
 	done := ctx.Done()
 	for {
-		s.runPlugin(ctx, start)
+		s.runPlugin(ctx, start, api)
 		select {
 		case <-done:
 			return
@@ -77,7 +77,7 @@ func (s *Supervisor) run(ctx context.Context, start chan<- error) {
 	}
 }
 
-func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error) error {
+func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error, api plugin.API) error {
 	p, ipc, err := NewProcess(ctx, s.executable)
 	if err != nil {
 		if start != nil {
@@ -100,6 +100,10 @@ func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error) error {
 	}()
 
 	hooks, err := ConnectMain(muxer)
+	if err == nil {
+		err = hooks.OnActivate(api)
+	}
+
 	if err != nil {
 		if start != nil {
 			start <- err
@@ -111,6 +115,7 @@ func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error) error {
 	}
 
 	s.hooks.Store(hooks)
+
 	if start != nil {
 		start <- nil
 	}
diff --git a/plugin/rpcplugin/supervisor_test.go b/plugin/rpcplugin/supervisor_test.go
index 6a2b5ec5d..bd20158b5 100644
--- a/plugin/rpcplugin/supervisor_test.go
+++ b/plugin/rpcplugin/supervisor_test.go
@@ -1,6 +1,8 @@
 package rpcplugin
 
 import (
+	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -8,9 +10,11 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/plugin/plugintest"
 )
 
 func TestSupervisor(t *testing.T) {
@@ -38,8 +42,7 @@ func TestSupervisor(t *testing.T) {
 	bundle := model.BundleInfoForPath(dir)
 	supervisor, err := SupervisorProvider(bundle)
 	require.NoError(t, err)
-	require.NoError(t, supervisor.Start())
-	require.NoError(t, supervisor.Hooks().OnActivate(nil))
+	require.NoError(t, supervisor.Start(nil))
 	require.NoError(t, supervisor.Stop())
 }
 
@@ -68,7 +71,7 @@ func TestSupervisor_NonExistentExecutablePath(t *testing.T) {
 	require.NotNil(t, supervisor)
 	require.NoError(t, err)
 
-	require.Error(t, supervisor.Start())
+	require.Error(t, supervisor.Start(nil))
 }
 
 // If plugin development goes really wrong, let's make sure plugin activation won't block forever.
@@ -92,7 +95,7 @@ func TestSupervisor_StartTimeout(t *testing.T) {
 	bundle := model.BundleInfoForPath(dir)
 	supervisor, err := SupervisorProvider(bundle)
 	require.NoError(t, err)
-	require.Error(t, supervisor.Start())
+	require.Error(t, supervisor.Start(nil))
 }
 
 // Crashed plugins should be relaunched.
@@ -112,14 +115,23 @@ func TestSupervisor_PluginCrash(t *testing.T) {
 			"github.com/mattermost/mattermost-server/plugin/rpcplugin"
 		)
 
-		type MyPlugin struct {}
+		type Configuration struct {
+			ShouldExit bool
+		}
+
+		type MyPlugin struct {
+			config Configuration
+		}
 
 		func (p *MyPlugin) OnActivate(api plugin.API) error {
-			os.Exit(1)
+			api.LoadPluginConfiguration(&p.config)
 			return nil
 		}
 
 		func (p *MyPlugin) OnDeactivate() error {
+			if p.config.ShouldExit {
+				os.Exit(1)
+			}
 			return nil
 		}
 
@@ -130,17 +142,28 @@ func TestSupervisor_PluginCrash(t *testing.T) {
 
 	ioutil.WriteFile(filepath.Join(dir, "plugin.json"), []byte(`{"id": "foo", "backend": {"executable": "backend.exe"}}`), 0600)
 
+	var api plugintest.API
+	shouldExit := true
+	api.On("LoadPluginConfiguration", mock.MatchedBy(func(x interface{}) bool { return true })).Return(func(dest interface{}) error {
+		err := json.Unmarshal([]byte(fmt.Sprintf(`{"ShouldExit": %v}`, shouldExit)), dest)
+		shouldExit = false
+		return err
+	})
+
 	bundle := model.BundleInfoForPath(dir)
 	supervisor, err := SupervisorProvider(bundle)
 	require.NoError(t, err)
-	require.NoError(t, supervisor.Start())
-	require.Error(t, supervisor.Hooks().OnActivate(nil))
+	require.NoError(t, supervisor.Start(&api))
 
+	failed := false
 	recovered := false
 	for i := 0; i < 30; i++ {
 		if supervisor.Hooks().OnDeactivate() == nil {
+			require.True(t, failed)
 			recovered = true
 			break
+		} else {
+			failed = true
 		}
 		time.Sleep(time.Millisecond * 100)
 	}