Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Js console fixes #1577

Merged
merged 4 commits into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions js/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,33 +23,33 @@ package js
import (
"context"
"os"
"strconv"
"strings"

"github.com/dop251/goja"
"github.com/sirupsen/logrus"
)

// console represents a JS console implemented as a logrus.Logger.
type console struct {
Logger *logrus.Logger
logger logrus.FieldLogger
}

// Creates a console with the standard logrus logger.
func newConsole() *console {
return &console{logrus.StandardLogger()}
return &console{logrus.StandardLogger().WithField("source", "console")}
}

// Creates a console logger with its output set to the file at the provided `filepath`.
func newFileConsole(filepath string) (*console, error) {
f, err := os.OpenFile(filepath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
f, err := os.OpenFile(filepath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) //nolint:gosec
if err != nil {
return nil, err
}

l := logrus.New()
l.SetOutput(f)

//TODO: refactor to not rely on global variables, albeit external ones
// TODO: refactor to not rely on global variables, albeit external ones
l.SetFormatter(logrus.StandardLogger().Formatter)

return &console{l}, nil
Expand All @@ -64,21 +64,25 @@ func (c console) log(ctx *context.Context, level logrus.Level, msgobj goja.Value
}
}

fields := make(logrus.Fields)
for i, arg := range args {
fields[strconv.Itoa(i)] = arg.String()
msg := msgobj.String()
if len(args) > 0 {
strs := make([]string, 1+len(args))
strs[0] = msg
for i, v := range args {
strs[i+1] = v.String()
}

msg = strings.Join(strs, " ")
}
msg := msgobj.ToString()
e := c.Logger.WithFields(fields)
switch level {
switch level { //nolint:exhaustive
case logrus.DebugLevel:
e.Debug(msg)
c.logger.Debug(msg)
case logrus.InfoLevel:
e.Info(msg)
c.logger.Info(msg)
case logrus.WarnLevel:
e.Warn(msg)
c.logger.Warn(msg)
case logrus.ErrorLevel:
e.Error(msg)
c.logger.Error(msg)
}
}

Expand Down
52 changes: 32 additions & 20 deletions js/console_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func TestConsoleContext(t *testing.T) {
assert.Equal(t, "b", entry.Message)
}
}

func getSimpleRunner(filename, data string, opts ...interface{}) (*Runner, error) {
var (
fs = afero.NewMemMapFs()
Expand All @@ -93,6 +94,16 @@ func getSimpleRunner(filename, data string, opts ...interface{}) (*Runner, error
)
}

func extractLogger(fl logrus.FieldLogger) *logrus.Logger {
switch e := fl.(type) {
case *logrus.Entry:
return e.Logger
case *logrus.Logger:
return e
}
return nil
}

func TestConsole(t *testing.T) {
levels := map[string]logrus.Level{
"log": logrus.InfoLevel,
Expand All @@ -105,10 +116,10 @@ func TestConsole(t *testing.T) {
Message string
Data logrus.Fields
}{
`"string"`: {Message: "string"},
`"string","a","b"`: {Message: "string", Data: logrus.Fields{"0": "a", "1": "b"}},
`"string",1,2`: {Message: "string", Data: logrus.Fields{"0": "1", "1": "2"}},
`{}`: {Message: "[object Object]"},
`"string"`: {Message: "string", Data: logrus.Fields{"source": "console"}},
`"string","a","b"`: {Message: "string a b", Data: logrus.Fields{"source": "console"}},
`"string",1,2`: {Message: "string 1 2", Data: logrus.Fields{"source": "console"}},
`{}`: {Message: "[object Object]", Data: logrus.Fields{"source": "console"}},
}
for name, level := range levels {
name, level := name, level
Expand All @@ -130,10 +141,11 @@ func TestConsole(t *testing.T) {
defer cancel()
vu := initVU.Activate(&lib.VUActivationParams{RunContext: ctx})

logger, hook := logtest.NewNullLogger()
logger := extractLogger(vu.(*ActiveVU).Console.logger)

logger.Out = ioutil.Discard
logger.Level = logrus.DebugLevel
jsVU := vu.(*ActiveVU)
jsVU.Console.Logger = logger
hook := logtest.NewLocal(logger)

err = vu.RunOnce()
assert.NoError(t, err)
Expand Down Expand Up @@ -168,10 +180,10 @@ func TestFileConsole(t *testing.T) {
Message string
Data logrus.Fields
}{
`"string"`: {Message: "string"},
`"string","a","b"`: {Message: "string", Data: logrus.Fields{"0": "a", "1": "b"}},
`"string",1,2`: {Message: "string", Data: logrus.Fields{"0": "1", "1": "2"}},
`{}`: {Message: "[object Object]"},
`"string"`: {Message: "string", Data: logrus.Fields{}},
`"string","a","b"`: {Message: "string a b", Data: logrus.Fields{}},
`"string",1,2`: {Message: "string 1 2", Data: logrus.Fields{}},
`{}`: {Message: "[object Object]", Data: logrus.Fields{}},
}
preExisting = map[string]bool{
"log exists": false,
Expand All @@ -186,11 +198,11 @@ func TestFileConsole(t *testing.T) {
// whether the file is existed before logging
for msg, deleteFile := range preExisting {
t.Run(msg, func(t *testing.T) {
var f, err = ioutil.TempFile("", "")
f, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("Couldn't create temporary file for testing: %s", err)
}
var logFilename = f.Name()
logFilename := f.Name()
defer os.Remove(logFilename)
// close it as we will want to reopen it and maybe remove it
if deleteFile {
Expand All @@ -200,8 +212,8 @@ func TestFileConsole(t *testing.T) {
}
} else {
// TODO: handle case where the string was no written in full ?
_, err := f.WriteString(preExistingText)
f.Close()
_, err = f.WriteString(preExistingText)
_ = f.Close()
if err != nil {
t.Fatalf("Error while writing text to preexisting logfile: %s", err)
}
Expand All @@ -226,9 +238,10 @@ func TestFileConsole(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
vu := initVU.Activate(&lib.VUActivationParams{RunContext: ctx})
jsVU := vu.(*ActiveVU)
jsVU.Console.Logger.Level = logrus.DebugLevel
hook := logtest.NewLocal(jsVU.Console.Logger)
logger := extractLogger(vu.(*ActiveVU).Console.logger)

logger.Level = logrus.DebugLevel
hook := logtest.NewLocal(logger)

err = vu.RunOnce()
assert.NoError(t, err)
Expand Down Expand Up @@ -259,13 +272,12 @@ func TestFileConsole(t *testing.T) {
fileContent, err := ioutil.ReadAll(f)
assert.NoError(t, err)

var expectedStr = entryStr
expectedStr := entryStr
if !deleteFile {
expectedStr = preExistingText + expectedStr
}
assert.Equal(t, expectedStr, string(fileContent))
}

})
}
})
Expand Down
4 changes: 3 additions & 1 deletion lib/executor/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ func getIterationRunner(
default:
if err != nil {
if s, ok := err.(fmt.Stringer); ok {
logger.Error(s.String())
// TODO better detection for stack traces
// TODO don't count this as a full iteration?
logger.WithField("source", "stacktrace").Error(s.String())
} else {
logger.Error(err.Error())
}
Expand Down
12 changes: 6 additions & 6 deletions lib/netext/httpext/httpdebug_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@
package httpext

import (
"fmt"
"bytes"
"net/http"
"net/http/httputil"

"github.com/sirupsen/logrus"
)

type httpDebugTransport struct {
//TODO: get the state and log to its Logger
originalTransport http.RoundTripper
httpDebugOption string
logger logrus.FieldLogger
}

// RoundTrip prints passing HTTP requests and received responses
Expand All @@ -50,17 +50,17 @@ func (t httpDebugTransport) RoundTrip(req *http.Request) (*http.Response, error)
func (t httpDebugTransport) debugRequest(req *http.Request) {
dump, err := httputil.DumpRequestOut(req, t.httpDebugOption == "full")
if err != nil {
logrus.Error(err)
t.logger.Error(err)
}
fmt.Printf("Request:\n%s\n", dump) //TODO: fix...
t.logger.Infof("Request:\n%s\n", bytes.ReplaceAll(dump, []byte("\r\n"), []byte{'\n'}))
}

func (t httpDebugTransport) debugResponse(res *http.Response) {
if res != nil {
dump, err := httputil.DumpResponse(res, t.httpDebugOption == "full")
if err != nil {
logrus.Error(err)
t.logger.Error(err)
}
fmt.Printf("Response:\n%s\n", dump) //TODO: fix...
t.logger.Infof("Response:\n%s\n", bytes.ReplaceAll(dump, []byte("\r\n"), []byte{'\n'}))
}
}
1 change: 1 addition & 0 deletions lib/netext/httpext/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
transport = httpDebugTransport{
originalTransport: transport,
httpDebugOption: state.Options.HTTPDebug.String,
logger: state.Logger.WithField("source", "http-debug"),
}
}

Expand Down