Merge pull request #4863 from getlantern/issue4803

Simplify golog and errors package by calling PrintStack directly

Author: oxtoacart

src/github.com/getlantern/errors/errors.go

@@ -59,7 +59,6 @@ package errors
 
 import (
 	"bufio"
-	"bytes"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/hex"
@@ -105,8 +104,11 @@ type Error interface {
 	// This can be useful when performing analytics on the error.
 	ErrorClean() string
 
-	// MultiLinePrinter implements the interface golog.MultiLine
-	MultiLinePrinter() func(buf *bytes.Buffer) bool
+	// PrintStack prints the stacktrace when this Error is created, together
+	// with the cause of this Error (if any), then the stacktrace when the
+	// cause is created, and so on.
+	// The output is an analogy of Java's stacktrace.
+	PrintStack(w io.Writer, linePrefix string)
 
 	// Op attaches a hint of the operation triggers this Error. Many error types
 	// returned by net and os package have Op pre-filled.
@@ -223,53 +225,19 @@ func (e *structured) Error() string {
 	return e.data["error_text"].(string) + e.hiddenID
 }
 
-func (e *structured) MultiLinePrinter() func(buf *bytes.Buffer) bool {
-	first := true
-	indent := false
+func (e *structured) PrintStack(w io.Writer, linePrefix string) {
 	err := e
-	stackPosition := 0
-	switchedCause := false
-	return func(buf *bytes.Buffer) bool {
-		if indent {
-			buf.WriteString("  ")
-		}
-		if first {
-			buf.WriteString(e.Error())
-			first = false
-			indent = true
-			return true
-		}
-		if switchedCause {
-			fmt.Fprintf(buf, "Caused by: %v", err)
-			if err.callStack != nil && len(err.callStack) > 0 {
-				switchedCause = false
-				indent = true
-				return true
-			}
-			if err.cause == nil {
-				return false
-			}
-			err = err.cause.(*structured)
-			return true
-		}
-		if stackPosition < len(err.callStack) {
-			buf.WriteString("at ")
+	for {
+		for stackPosition := 0; stackPosition < len(err.callStack); stackPosition++ {
 			call := err.callStack[stackPosition]
-			fmt.Fprintf(buf, "%+n (%s:%d)", call, call, call)
-			stackPosition++
+			fmt.Fprintf(w, "%s  at %+n (%s:%d)\n", linePrefix, call, call, call)
 		}
-		if stackPosition >= len(err.callStack) {
-			switch cause := err.cause.(type) {
-			case *structured:
-				err = cause
-				indent = false
-				stackPosition = 0
-				switchedCause = true
-			default:
-				return false
-			}
+		cause, ok := err.cause.(*structured)
+		if !ok || cause == nil {
+			return
 		}
-		return err != nil
+		err = cause
+		fmt.Fprintf(w, "%sCaused by: %v\n", linePrefix, err)
 	}
 }
 

src/github.com/getlantern/errors/errors_test.go

@@ -62,14 +62,9 @@ func TestNewWithCause(t *testing.T) {
 
 	// Make sure that stacktrace prints out okay
 	buf := &bytes.Buffer{}
-	print := outer.MultiLinePrinter()
-	for {
-		more := print(buf)
-		buf.WriteByte('\n')
-		if !more {
-			break
-		}
-	}
+	buf.WriteString(outer.Error())
+	buf.WriteByte('\n')
+	outer.PrintStack(buf, "")
 	expected := `Hello World
   at github.com/getlantern/errors.TestNewWithCause (errors_test.go:999)
   at testing.tRunner (testing.go:999)

src/github.com/getlantern/golog/golog.go

@@ -2,7 +2,6 @@
 // debug messages to stdout. Trace logging is also supported.
 // Trace logs go to stdout as well, but they are only written if the program
 // is run with environment variable "TRACE=true".
-// A stack dump will be printed after the message if "PRINT_STACK=true".
 package golog
 
 import (
@@ -67,16 +66,6 @@ type outputs struct {
 	DebugOut io.Writer
 }
 
-// MultiLine is an interface for arguments that support multi-line output.
-type MultiLine interface {
-	// MultiLinePrinter returns a function that can be used to print the
-	// multi-line output. The returned function writes one line to the buffer and
-	// returns true if there are more lines to write. This function does not need
-	// to take care of trailing carriage returns, golog handles that
-	// automatically.
-	MultiLinePrinter() func(buf *bytes.Buffer) bool
-}
-
 // ErrorReporter is a function to which the logger will report errors.
 // It the given error and corresponding message along with associated ops
 // context. This should return quickly as it executes on the critical code
@@ -145,20 +134,15 @@ func LoggerFor(prefix string) Logger {
 		l.traceOut = ioutil.Discard
 	}
 
-	printStack := os.Getenv("PRINT_STACK")
-	l.printStack, _ = strconv.ParseBool(printStack)
-
 	return l
 }
 
 type logger struct {
-	prefix     string
-	traceOn    bool
-	traceOut   io.Writer
-	printStack bool
-	outs       atomic.Value
-	pc         []uintptr
-	funcForPc  *runtime.Func
+	prefix   string
+	traceOn  bool
+	traceOut io.Writer
+	outs     atomic.Value
+	pc       []uintptr
 }
 
 // attaches the file and line number corresponding to
@@ -175,44 +159,27 @@ func (l *logger) print(out io.Writer, skipFrames int, severity string, arg inter
 	defer bufferPool.Put(buf)
 
 	linePrefix := l.linePrefix(skipFrames)
-	writeHeader := func() {
-		buf.WriteString(severity)
-		buf.WriteString(" ")
-		buf.WriteString(linePrefix)
-	}
+	sp := severity + " " + linePrefix
+	buf.WriteString(sp)
 	if arg != nil {
-		ml, isMultiline := arg.(MultiLine)
-		if !isMultiline {
-			writeHeader()
-			fmt.Fprintf(buf, "%v", arg)
+		if err, isError := arg.(errors.Error); isError && err != nil {
+			buf.WriteString(err.Error())
 			printContext(buf, arg)
 			buf.WriteByte('\n')
-		} else {
-			mlp := ml.MultiLinePrinter()
-			first := true
-			for {
-				writeHeader()
-				more := mlp(buf)
-				if first {
-					printContext(buf, arg)
-					first = false
-				}
-				buf.WriteByte('\n')
-				if !more {
-					break
-				}
+			if severity == "FATAL" || severity == "ERROR" {
+				err.PrintStack(buf, sp)
 			}
+		} else {
+			fmt.Fprintf(buf, "%v", arg)
+			printContext(buf, arg)
+			buf.WriteByte('\n')
 		}
 	}
 	b := []byte(hidden.Clean(buf.String()))
 	_, err := out.Write(b)
 	if err != nil {
 		errorOnLogging(err)
 	}
-	if l.printStack {
-		l.doPrintStack()
-	}
-
 	return linePrefix
 }
 
@@ -232,9 +199,6 @@ func (l *logger) printf(out io.Writer, skipFrames int, severity string, err erro
 	if err2 != nil {
 		errorOnLogging(err)
 	}
-	if l.printStack {
-		l.doPrintStack()
-	}
 	return linePrefix
 }
 
@@ -349,26 +313,6 @@ func (l *logger) AsStdLogger() *log.Logger {
 	return log.New(&errorWriter{l}, "", 0)
 }
 
-func (l *logger) doPrintStack() {
-	var b []byte
-	buf := bytes.NewBuffer(b)
-	for _, pc := range l.pc {
-		funcForPc := runtime.FuncForPC(pc)
-		if funcForPc == nil {
-			break
-		}
-		name := funcForPc.Name()
-		if strings.HasPrefix(name, "runtime.") {
-			break
-		}
-		file, line := funcForPc.FileLine(pc)
-		fmt.Fprintf(buf, "\t%s\t%s: %d\n", name, file, line)
-	}
-	if _, err := buf.WriteTo(os.Stderr); err != nil {
-		errorOnLogging(err)
-	}
-}
-
 func errorOnLogging(err error) {
 	fmt.Fprintf(os.Stderr, "Unable to log: %v\n", err)
 }