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

feat: Add logger override option, use slog and capture driver as logs #497

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
6 changes: 4 additions & 2 deletions binding_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"strings"

"github.com/playwright-community/playwright-go/internal/pwlogger"

"github.com/go-stack/stack"
)

Expand Down Expand Up @@ -34,7 +36,7 @@ func (b *bindingCallImpl) Call(f BindingCallFunction) {
if _, err := b.channel.Send("reject", map[string]interface{}{
"error": serializeError(r.(error)),
}); err != nil {
logger.Printf("could not reject BindingCall: %v\n", err)
logger.Error("could not reject BindingCall", pwlogger.ErrAttr(err))
}
}
}()
Expand All @@ -60,7 +62,7 @@ func (b *bindingCallImpl) Call(f BindingCallFunction) {
"result": serializeArgument(result),
})
if err != nil {
logger.Printf("could not resolve BindingCall: %v\n", err)
logger.Error("could not resolve BindingCall", pwlogger.ErrAttr(err))
}
}

Expand Down
4 changes: 3 additions & 1 deletion browser_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"strings"
"sync"

"github.com/playwright-community/playwright-go/internal/pwlogger"

"github.com/playwright-community/playwright-go/internal/safe"
)

Expand Down Expand Up @@ -589,7 +591,7 @@ func (b *browserContextImpl) onRoute(route *routeImpl) {
return nil, err
}, true)
if err != nil {
logger.Printf("could not update interception patterns: %v\n", err)
logger.Error("Could not update interception patterns", pwlogger.ErrAttr(err))
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions channel.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package playwright

import "encoding/json"
import (
"encoding/json"

"github.com/playwright-community/playwright-go/internal/pwlogger"
)

type channel struct {
eventEmitter
Expand Down Expand Up @@ -59,7 +63,7 @@ func (c *channel) SendNoReply(method string, options ...interface{}) {
return c.connection.sendMessageToServer(c.owner, method, params, true)
}, false)
if err != nil {
logger.Printf("SendNoReply failed: %v\n", err)
logger.Error("SendNoReply failed", pwlogger.ErrAttr(err))
}
}

Expand Down
3 changes: 2 additions & 1 deletion frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package playwright
import (
"errors"
"fmt"
"log/slog"
"os"
"time"

Expand Down Expand Up @@ -210,7 +211,7 @@ func (f *frameImpl) ExpectNavigation(cb func() error, options ...FrameExpectNavi
err, ok := ev["error"]
if ok {
// Any failed navigation results in a rejection.
logger.Printf("navigated to %s error: %v", ev["url"].(string), err)
logger.Error("navigation error", slog.Any("url", ev["url"].(string)), slog.Any("error", err))
return true
}
return matcher == nil || matcher.Matches(ev["url"].(string))
Expand Down
9 changes: 6 additions & 3 deletions har_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package playwright

import (
"errors"
"log/slog"

"github.com/playwright-community/playwright-go/internal/pwlogger"
)

type harRouter struct {
Expand All @@ -19,7 +22,7 @@ func (r *harRouter) addContextRoute(context BrowserContext) error {
err := context.Route(r.urlOrPredicate, func(route Route) {
err := r.handle(route)
if err != nil {
logger.Println(err)
logger.Error("Error handling context route", pwlogger.ErrAttr(err))
}
})
if err != nil {
Expand All @@ -35,7 +38,7 @@ func (r *harRouter) addPageRoute(page Page) error {
err := page.Route(r.urlOrPredicate, func(route Route) {
err := r.handle(route)
if err != nil {
logger.Println(err)
logger.Error("Error handling page route", pwlogger.ErrAttr(err))
}
})
if err != nil {
Expand Down Expand Up @@ -86,7 +89,7 @@ func (r *harRouter) handle(route Route) error {
Headers: deserializeNameAndValueToMap(response.Headers),
})
case "error":
logger.Printf("har action error: %v\n", *response.Message)
logger.Error("har action error", slog.Any("error", *response.Message))
fallthrough
case "noentry":
}
Expand Down
60 changes: 60 additions & 0 deletions internal/pwlogger/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package pwlogger

import (
"context"
"log/slog"
"strings"
)

// StreamType represents the type of output stream
type StreamType int

const (
StdoutStream StreamType = iota
StderrStream
)

func (st StreamType) String() string {
switch st {
case StdoutStream:
return "stdout"
case StderrStream:
return "stderr"
default:
return "unknown"
}
}

func (st StreamType) LogValue() slog.Value {
return slog.StringValue(st.String())
}

// SlogWriter is a custom type that implements io.Writer
type SlogWriter struct {
logger *slog.Logger
stream StreamType
cmdAttrs []slog.Attr
}

// Write implements the io.Writer interface
func (sw *SlogWriter) Write(p []byte) (n int, err error) {
message := strings.TrimSpace(string(p))
attrs := append(sw.cmdAttrs,
slog.String("stream", sw.stream.String()),
)
sw.logger.LogAttrs(context.Background(), slog.LevelInfo, message, attrs...)
return len(p), nil
}

// NewSlogWriter creates a new SlogWriter with the specified stream type and command attributes
func NewSlogWriter(logger *slog.Logger, stream StreamType, cmdAttrs ...slog.Attr) *SlogWriter {
return &SlogWriter{
logger: logger,
stream: stream,
cmdAttrs: cmdAttrs,
}
}

func ErrAttr(err error) slog.Attr {
return slog.Any("error", err)
}
4 changes: 3 additions & 1 deletion page.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"slices"
"sync"

"github.com/playwright-community/playwright-go/internal/pwlogger"

"github.com/playwright-community/playwright-go/internal/safe"
)

Expand Down Expand Up @@ -927,7 +929,7 @@ func (p *pageImpl) onRoute(route *routeImpl) {
return nil, err
}, true)
if err != nil {
logger.Printf("could not update interception patterns: %v\n", err)
logger.Error("could not update interception patterns", pwlogger.ErrAttr(err))
}
}
}
Expand Down
26 changes: 21 additions & 5 deletions run.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,24 @@ import (
"errors"
"fmt"
"io"
"log"
"log/slog"
"net/http"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"

"github.com/playwright-community/playwright-go/internal/pwlogger"
)

const (
playwrightCliVersion = "1.47.2"
playwrightCliVersion = "1.47.2"
playwrightDriverLogSource = "playwright-driver"
)

var (
logger = log.Default()
logger = slog.Default()
playwrightCDNMirrors = []string{
"https://playwright.azureedge.net",
"https://playwright-akamai.azureedge.net",
Expand Down Expand Up @@ -186,7 +189,7 @@ func (d *PlaywrightDriver) DownloadDriver() error {

func (d *PlaywrightDriver) log(s string) {
if d.options.Verbose {
logger.Println(s)
logger.Info(s)
}
}

Expand Down Expand Up @@ -232,6 +235,10 @@ type RunOptions struct {
Verbose bool // default true
Stdout io.Writer
Stderr io.Writer
Logger *slog.Logger
// If set, will capture all output to the logger
// This will override Stdout and Stderr
CaptureAllOutputWithLogger bool
}

// Install does download the driver and the browsers.
Expand Down Expand Up @@ -291,8 +298,17 @@ func transformRunOptions(options ...*RunOptions) (*RunOptions, error) {
}
if option.Stderr == nil {
option.Stderr = os.Stderr
}
if option.Logger == nil {
logger = slog.New(slog.NewTextHandler(option.Stderr, nil))
} else {
logger.SetOutput(option.Stderr)
logger = option.Logger
}

if option.CaptureAllOutputWithLogger {
Copy link
Collaborator

@canstand canstand Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @GuyGoldenberg, stderr of playwrightDriver is already redirected to options.Stderr in newPipTransport, and stdout only contains one-way communication protocol data. Is there a special reason to log the protocol output? Output is redirected to options.stdout and options.stderr. It is recommended that the caller handle logging itself.

sourceLogAttr := slog.String("source", playwrightDriverLogSource) // Indicate that the logs are from the driver
option.Stdout = pwlogger.NewSlogWriter(logger, pwlogger.StdoutStream, sourceLogAttr)
option.Stderr = pwlogger.NewSlogWriter(logger, pwlogger.StderrStream, sourceLogAttr)
}
return option, nil
}
Expand Down
6 changes: 4 additions & 2 deletions transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"io"
"os"

"github.com/playwright-community/playwright-go/internal/pwlogger"

"github.com/go-jose/go-jose/v3/json"
)

Expand Down Expand Up @@ -44,7 +46,7 @@ func (t *pipeTransport) Poll() (*message, error) {
if os.Getenv("DEBUGP") != "" {
fmt.Fprint(os.Stdout, "\x1b[33mRECV>\x1b[0m\n")
if err := json.NewEncoder(os.Stdout).Encode(msg); err != nil {
logger.Printf("could not encode json: %v\n", err)
logger.Error("could not encode json", pwlogger.ErrAttr(err))
}
}
return msg, nil
Expand Down Expand Up @@ -72,7 +74,7 @@ func (t *pipeTransport) Send(msg map[string]interface{}) error {
if os.Getenv("DEBUGP") != "" {
fmt.Fprint(os.Stdout, "\x1b[32mSEND>\x1b[0m\n")
if err := json.NewEncoder(os.Stdout).Encode(msg); err != nil {
logger.Printf("could not encode json: %v\n", err)
logger.Error("could not encode json", pwlogger.ErrAttr(err))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
lengthPadding := make([]byte, 4)
Expand Down
6 changes: 4 additions & 2 deletions websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package playwright
import (
"encoding/base64"
"errors"

"github.com/playwright-community/playwright-go/internal/pwlogger"
)

type webSocketImpl struct {
Expand Down Expand Up @@ -50,7 +52,7 @@ func (ws *webSocketImpl) onFrameSent(opcode float64, data string) {
if opcode == 2 {
payload, err := base64.StdEncoding.DecodeString(data)
if err != nil {
logger.Printf("could not decode WebSocket.onFrameSent payload: %v\n", err)
logger.Error("could not decode WebSocket.onFrameSent payload", pwlogger.ErrAttr(err))
return
}
ws.Emit("framesent", payload)
Expand All @@ -63,7 +65,7 @@ func (ws *webSocketImpl) onFrameReceived(opcode float64, data string) {
if opcode == 2 {
payload, err := base64.StdEncoding.DecodeString(data)
if err != nil {
logger.Printf("could not decode WebSocket.onFrameReceived payload: %v\n", err)
logger.Error("could not decode WebSocket.onFrameReceived payload", pwlogger.ErrAttr(err))
return
}
ws.Emit("framereceived", payload)
Expand Down
Loading