From f149ada16abd33f8b44193c109cc3559d04cdb91 Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Mon, 30 Mar 2020 15:00:45 -0300
Subject: [PATCH] MM-23261 plugin stderr debug logs (#14166)

* explicitly assert panic as error log

* Revert "[MM-18150] plugin panic trace should not be lost (#13559)"

This reverts commit 5d928b4f942bf86b94d80a9cdcb69c51da5b9f77, while leaving the unit tests intact
and now asserting debug logs instead.

* missing license header
---
 app/plugin_test.go    |  6 +++---
 mlog/log.go           |  9 ---------
 mlog/stdlog.go        | 10 ----------
 plugin/supervisor.go  |  1 -
 testlib/assertions.go | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 36 insertions(+), 23 deletions(-)
 create mode 100644 testlib/assertions.go

diff --git a/app/plugin_test.go b/app/plugin_test.go
index fe59405e6c..55d95c61eb 100644
--- a/app/plugin_test.go
+++ b/app/plugin_test.go
@@ -13,7 +13,6 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -21,8 +20,10 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/plugin"
+	"github.com/mattermost/mattermost-server/v5/testlib"
 	"github.com/mattermost/mattermost-server/v5/utils"
 	"github.com/mattermost/mattermost-server/v5/utils/fileutils"
 )
@@ -665,8 +666,7 @@ func TestPluginPanicLogs(t *testing.T) {
 		_, err := th.App.CreatePost(post, th.BasicChannel, false)
 		assert.Nil(t, err)
 
-		logs := th.LogBuffer.String()
-		assert.True(t, strings.Contains(logs, "some text from panic"))
+		testlib.AssertLog(t, th.LogBuffer, mlog.LevelDebug, "panic: some text from panic")
 	})
 }
 
diff --git a/mlog/log.go b/mlog/log.go
index f2432166f1..1a6c2de949 100644
--- a/mlog/log.go
+++ b/mlog/log.go
@@ -144,15 +144,6 @@ func (l *Logger) StdLogWriter() io.Writer {
 	return &loggerWriter{f}
 }
 
-// StdErrPanicLogWriter returns a writer that can be hooked up to the output of a golang standard logger
-// all of the stderr will be interpreted as log entry.
-func (l *Logger) StdErrPanicLogWriter() io.Writer {
-	newLogger := *l
-	newLogger.zap = newLogger.zap.WithOptions(zap.AddCallerSkip(4), getStdLogOption())
-	f := newLogger.Error
-	return &panicLoggerWriter{f}
-}
-
 func (l *Logger) WithCallerSkip(skip int) *Logger {
 	newlogger := *l
 	newlogger.zap = newlogger.zap.WithOptions(zap.AddCallerSkip(skip))
diff --git a/mlog/stdlog.go b/mlog/stdlog.go
index 93bacb6d57..fd702abffa 100644
--- a/mlog/stdlog.go
+++ b/mlog/stdlog.go
@@ -85,13 +85,3 @@ func (l *loggerWriter) Write(p []byte) (int, error) {
 	}
 	return len(p), nil
 }
-
-type panicLoggerWriter struct {
-	logFunc func(msg string, fields ...Field)
-}
-
-func (l *panicLoggerWriter) Write(p []byte) (int, error) {
-	trimmed := string(bytes.TrimSpace(p))
-	l.logFunc(trimmed)
-	return len(p), nil
-}
diff --git a/plugin/supervisor.go b/plugin/supervisor.go
index 40aaf6d6c1..348ea4a46f 100644
--- a/plugin/supervisor.go
+++ b/plugin/supervisor.go
@@ -63,7 +63,6 @@ func newSupervisor(pluginInfo *model.BundleInfo, apiImpl API, parentLogger *mlog
 		HandshakeConfig: handshake,
 		Plugins:         pluginMap,
 		Cmd:             cmd,
-		Stderr:          wrappedLogger.With(mlog.String("source", "plugin_stderr_panic")).StdErrPanicLogWriter(),
 		SyncStdout:      wrappedLogger.With(mlog.String("source", "plugin_stdout")).StdLogWriter(),
 		SyncStderr:      wrappedLogger.With(mlog.String("source", "plugin_stderr")).StdLogWriter(),
 		Logger:          hclogAdaptedLogger,
diff --git a/testlib/assertions.go b/testlib/assertions.go
new file mode 100644
index 0000000000..4fb56ba45a
--- /dev/null
+++ b/testlib/assertions.go
@@ -0,0 +1,33 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package testlib
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"testing"
+)
+
+// AssertLog asserts that a JSON-encoded buffer of logs contains one with the given level and message.
+func AssertLog(t *testing.T, logs *bytes.Buffer, level, message string) {
+	dec := json.NewDecoder(logs)
+	for {
+		var log struct {
+			Level string
+			Msg   string
+		}
+		if err := dec.Decode(&log); err == io.EOF {
+			break
+		} else if err != nil {
+			continue
+		}
+
+		if log.Level == level && log.Msg == message {
+			return
+		}
+	}
+
+	t.Fatalf("failed to find %s log message: %s", level, message)
+}