pkg/plugins: Check errors (#19715)

* pkg/plugins: Check errors

pkg/plugins/dashboard_importer_test.go

@@ -89,12 +89,13 @@ func pluginScenario(desc string, t *testing.T, fn func()) {
 	Convey("Given a plugin", t, func() {
 		setting.Raw = ini.Empty()
 		sec, _ := setting.Raw.NewSection("plugin.test-app")
-		sec.NewKey("path", "testdata/test-app")
+		_, err := sec.NewKey("path", "testdata/test-app")
+		So(err, ShouldBeNil)
 
 		pm := &PluginManager{}
-		err := pm.Init()
-
+		err = pm.Init()
 		So(err, ShouldBeNil)
+
 		Convey(desc, fn)
 	})
 }

pkg/plugins/dashboards_test.go

@@ -12,15 +12,14 @@ import (
 )
 
 func TestPluginDashboards(t *testing.T) {
-
 	Convey("When asking plugin dashboard info", t, func() {
 		setting.Raw = ini.Empty()
 		sec, _ := setting.Raw.NewSection("plugin.test-app")
-		sec.NewKey("path", "testdata/test-app")
+		_, err := sec.NewKey("path", "testdata/test-app")
+		So(err, ShouldBeNil)
 
 		pm := &PluginManager{}
-		err := pm.Init()
-
+		err = pm.Init()
 		So(err, ShouldBeNil)
 
 		bus.AddHandler("test", func(query *m.GetDashboardQuery) error {

pkg/plugins/datasource_plugin.go

@@ -13,6 +13,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins/datasource/wrapper"
 	"github.com/grafana/grafana/pkg/tsdb"
 	plugin "github.com/hashicorp/go-plugin"
+	"golang.org/x/xerrors"
 )
 
 // DataSourcePlugin contains all metadata about a datasource plugin
@@ -60,12 +61,17 @@ var handshakeConfig = plugin.HandshakeConfig{
 func (p *DataSourcePlugin) startBackendPlugin(ctx context.Context, log log.Logger) error {
 	p.log = log.New("plugin-id", p.Id)
 
-	err := p.spawnSubProcess()
-	if err == nil {
-		go p.restartKilledProcess(ctx)
+	if err := p.spawnSubProcess(); err != nil {
+		return err
 	}
 
-	return err
+	go func() {
+		if err := p.restartKilledProcess(ctx); err != nil {
+			p.log.Error("Attempting to restart killed process failed", "err", err)
+		}
+	}()
+
+	return nil
 }
 
 func (p *DataSourcePlugin) spawnSubProcess() error {
@@ -105,15 +111,21 @@ func (p *DataSourcePlugin) restartKilledProcess(ctx context.Context) error {
 	for {
 		select {
 		case <-ctx.Done():
-			return ctx.Err()
-		case <-ticker.C:
-			if p.client.Exited() {
-				err := p.spawnSubProcess()
-				p.log.Debug("Spawning new sub process", "name", p.Name, "id", p.Id)
-				if err != nil {
-					p.log.Error("Failed to spawn subprocess")
-				}
+			if err := ctx.Err(); err != nil && !xerrors.Is(err, context.Canceled) {
+				return err
 			}
+			return nil
+		case <-ticker.C:
+			if !p.client.Exited() {
+				continue
+			}
+
+			if err := p.spawnSubProcess(); err != nil {
+				p.log.Error("Failed to restart plugin", "err", err)
+				continue
+			}
+
+			p.log.Debug("Plugin process restarted")
 		}
 	}
 }

pkg/plugins/plugins.go

@@ -17,6 +17,8 @@ import (
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"golang.org/x/xerrors"
 )
 
 var (
@@ -63,7 +65,10 @@ func (pm *PluginManager) Init() error {
 	}
 
 	pm.log.Info("Starting plugin search")
-	scan(path.Join(setting.StaticRootPath, "app/plugins"))
+	plugDir := path.Join(setting.StaticRootPath, "app/plugins")
+	if err := pm.scan(plugDir); err != nil {
+		return errutil.Wrapf(err, "Failed to scan main plugin directory '%s'", plugDir)
+	}
 
 	// check if plugins dir exists
 	if _, err := os.Stat(setting.PluginsPath); os.IsNotExist(err) {
@@ -71,14 +76,22 @@ func (pm *PluginManager) Init() error {
 			plog.Error("Failed to create plugin dir", "dir", setting.PluginsPath, "error", err)
 		} else {
 			plog.Info("Plugin dir created", "dir", setting.PluginsPath)
-			scan(setting.PluginsPath)
+			if err := pm.scan(setting.PluginsPath); err != nil {
+				return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'",
+					setting.PluginsPath)
+			}
 		}
 	} else {
-		scan(setting.PluginsPath)
+		if err := pm.scan(setting.PluginsPath); err != nil {
+			return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'",
+				setting.PluginsPath)
+		}
 	}
 
 	// check plugin paths defined in config
-	checkPluginPaths()
+	if err := pm.checkPluginPaths(); err != nil {
+		return err
+	}
 
 	for _, panel := range Panels {
 		panel.initFrontendPlugin()
@@ -95,16 +108,16 @@ func (pm *PluginManager) Init() error {
 	return nil
 }
 
-func (pm *PluginManager) startBackendPlugins(ctx context.Context) error {
+func (pm *PluginManager) startBackendPlugins(ctx context.Context) {
 	for _, ds := range DataSources {
-		if ds.Backend {
-			if err := ds.startBackendPlugin(ctx, plog); err != nil {
-				pm.log.Error("Failed to init plugin.", "error", err, "plugin", ds.Id)
-			}
+		if !ds.Backend {
+			continue
+		}
+
+		if err := ds.startBackendPlugin(ctx, plog); err != nil {
+			pm.log.Error("Failed to init plugin.", "error", err, "plugin", ds.Id)
 		}
 	}
-
-	return nil
 }
 
 func (pm *PluginManager) Run(ctx context.Context) error {
@@ -132,32 +145,49 @@ func (pm *PluginManager) Run(ctx context.Context) error {
 	return ctx.Err()
 }
 
-func checkPluginPaths() error {
+func (pm *PluginManager) checkPluginPaths() error {
 	for _, section := range setting.Raw.Sections() {
-		if strings.HasPrefix(section.Name(), "plugin.") {
-			path := section.Key("path").String()
-			if path != "" {
-				scan(path)
-			}
+		if !strings.HasPrefix(section.Name(), "plugin.") {
+			continue
+		}
+
+		path := section.Key("path").String()
+		if path == "" {
+			continue
+		}
+
+		if err := pm.scan(path); err != nil {
+			return errutil.Wrapf(err, "Failed to scan directory configured for plugin '%s': '%s'",
+				section.Name(), path)
 		}
 	}
+
 	return nil
 }
 
-func scan(pluginDir string) error {
+// scan a directory for plugins.
+func (pm *PluginManager) scan(pluginDir string) error {
 	scanner := &PluginScanner{
 		pluginPath: pluginDir,
 	}
 
 	if err := util.Walk(pluginDir, true, true, scanner.walker); err != nil {
+		if xerrors.Is(err, os.ErrNotExist) {
+			pm.log.Debug("Couldn't scan dir '%s' since it doesn't exist")
+			return nil
+		}
+		if xerrors.Is(err, os.ErrPermission) {
+			pm.log.Debug("Couldn't scan dir '%s' due to lack of permissions")
+			return nil
+		}
 		if pluginDir != "data/plugins" {
-			log.Warn("Could not scan dir \"%v\" error: %s", pluginDir, err)
+			pm.log.Warn("Could not scan dir", "pluginDir", pluginDir, "err", err)
 		}
 		return err
 	}
 
 	if len(scanner.errors) > 0 {
-		return errors.New("Some plugins failed to load")
+		return errutil.Wrapf(scanner.errors[0], "Some plugins failed to load")
 	}
 
 	return nil
@@ -167,7 +197,6 @@ func (scanner *PluginScanner) walker(currentPath string, f os.FileInfo, err erro
 	// We scan all the subfolders for plugin.json (with some exceptions) so that we also load embedded plugins, for
 	// example https://github.com/raintank/worldping-app/tree/master/dist/grafana-worldmap-panel worldmap panel plugin
 	// is embedded in worldping app.
-
 	if err != nil {
 		return err
 	}
@@ -227,7 +256,9 @@ func (scanner *PluginScanner) loadPluginJson(pluginJsonFilePath string) error {
 		}
 	}
 
-	reader.Seek(0, 0)
+	if _, err := reader.Seek(0, 0); err != nil {
+		return err
+	}
 	return loader.Load(jsonParser, currentDir)
 }
 

pkg/plugins/plugins_test.go

@@ -29,15 +29,16 @@ func TestPluginScans(t *testing.T) {
 
 	Convey("When reading app plugin definition", t, func() {
 		setting.Raw = ini.Empty()
-		sec, _ := setting.Raw.NewSection("plugin.nginx-app")
-		sec.NewKey("path", "testdata/test-app")
+		sec, err := setting.Raw.NewSection("plugin.nginx-app")
+		So(err, ShouldBeNil)
+		_, err = sec.NewKey("path", "testdata/test-app")
+		So(err, ShouldBeNil)
 
 		pm := &PluginManager{}
-		err := pm.Init()
-
+		err = pm.Init()
 		So(err, ShouldBeNil)
-		So(len(Apps), ShouldBeGreaterThan, 0)
 
+		So(len(Apps), ShouldBeGreaterThan, 0)
 		So(Apps["test-app"].Info.Logos.Large, ShouldEqual, "public/plugins/test-app/img/logo_large.png")
 		So(Apps["test-app"].Info.Screenshots[1].Path, ShouldEqual, "public/plugins/test-app/img/screenshot2.png")
 	})