From 4ae1238bc12537632289960f58e52d3c625d37e3 Mon Sep 17 00:00:00 2001
From: Joram Wilander <jwawilander@gmail.com>
Date: Tue, 13 Mar 2018 10:32:24 -0400
Subject: Better error handling for failed plugin activation (#8361)

---
 app/plugin.go      | 30 +++++++++++++++++++++---------
 app/plugin_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/app/plugin.go b/app/plugin.go
index fe671d26a..22d333679 100644
--- a/app/plugin.go
+++ b/app/plugin.go
@@ -91,18 +91,11 @@ func (a *App) ActivatePlugins() {
 		active := a.PluginEnv.IsPluginActive(id)
 
 		if pluginState.Enable && !active {
-			if err := a.PluginEnv.ActivatePlugin(id); err != nil {
-				l4g.Error(err.Error())
+			if err := a.activatePlugin(plugin.Manifest); err != nil {
+				l4g.Error("%v plugin enabled in config.json but failing to activate err=%v", plugin.Manifest.Id, err.DetailedError)
 				continue
 			}
 
-			if plugin.Manifest.HasClient() {
-				message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ACTIVATED, "", "", "", nil)
-				message.Add("manifest", plugin.Manifest.ClientManifest())
-				a.Publish(message)
-			}
-
-			l4g.Info("Activated %v plugin", id)
 		} else if !pluginState.Enable && active {
 			if err := a.deactivatePlugin(plugin.Manifest); err != nil {
 				l4g.Error(err.Error())
@@ -111,6 +104,21 @@ func (a *App) ActivatePlugins() {
 	}
 }
 
+func (a *App) activatePlugin(manifest *model.Manifest) *model.AppError {
+	if err := a.PluginEnv.ActivatePlugin(manifest.Id); err != nil {
+		return model.NewAppError("activatePlugin", "app.plugin.activate.app_error", nil, err.Error(), http.StatusBadRequest)
+	}
+
+	if manifest.HasClient() {
+		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ACTIVATED, "", "", "", nil)
+		message.Add("manifest", manifest.ClientManifest())
+		a.Publish(message)
+	}
+
+	l4g.Info("Activated %v plugin", manifest.Id)
+	return nil
+}
+
 func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError {
 	if err := a.PluginEnv.DeactivatePlugin(manifest.Id); err != nil {
 		return model.NewAppError("removePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest)
@@ -301,6 +309,10 @@ func (a *App) EnablePlugin(id string) *model.AppError {
 		return model.NewAppError("EnablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if err := a.activatePlugin(manifest); err != nil {
+		return err
+	}
+
 	a.UpdateConfig(func(cfg *model.Config) {
 		cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true}
 	})
diff --git a/app/plugin_test.go b/app/plugin_test.go
index 4794d2704..9ad5dc1fa 100644
--- a/app/plugin_test.go
+++ b/app/plugin_test.go
@@ -4,8 +4,10 @@
 package app
 
 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/gorilla/mux"
@@ -195,3 +197,26 @@ func TestPluginCommands(t *testing.T) {
 	require.NotNil(t, err)
 	assert.Equal(t, http.StatusNotFound, err.StatusCode)
 }
+
+type pluginBadActivation struct {
+	testPlugin
+}
+
+func (p *pluginBadActivation) OnActivate(api plugin.API) error {
+	return errors.New("won't activate for some reason")
+}
+
+func TestPluginBadActivation(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	th.InstallPlugin(&model.Manifest{
+		Id: "foo",
+	}, &pluginBadActivation{})
+
+	t.Run("EnablePlugin bad activation", func(t *testing.T) {
+		err := th.App.EnablePlugin("foo")
+		assert.NotNil(t, err)
+		assert.True(t, strings.Contains(err.DetailedError, "won't activate for some reason"))
+	})
+}
-- 
cgit v1.2.3-1-g7c22