From 3a7c4822f82575c9e1f3d71fc8a84c6b25a04875 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Sun, 27 Dec 2020 13:55:58 -0700 Subject: [PATCH] Improve logged stacktraces --- loggers/cli/cli.go | 56 +++++++++++++++++++++++++++++++-- router/downloader/downloader.go | 6 +++- server/backup.go | 13 +++----- server/filesystem/archive.go | 21 ++++++++----- 4 files changed, 77 insertions(+), 19 deletions(-) diff --git a/loggers/cli/cli.go b/loggers/cli/cli.go index 181d094..a87cf7a 100644 --- a/loggers/cli/cli.go +++ b/loggers/cli/cli.go @@ -9,6 +9,7 @@ import ( "github.com/mattn/go-colorable" "io" "os" + "strings" "sync" "time" ) @@ -65,12 +66,63 @@ func (h *Handler) HandleLog(e *log.Entry) error { if name != "error" { continue } + if err, ok := e.Fields.Get("error").(error); ok { // Attach the stacktrace if it is missing at this point, but don't point // it specifically to this line since that is irrelevant. - err = errors.WithStackDepthIf(err, 1) - fmt.Fprintf(h.Writer, "\n%s\n%+v\n\n", boldred.Sprintf("Stacktrace:"), err) + err = errors.WithStackDepthIf(err, 4) + formatted := fmt.Sprintf("\n%s\n%+v\n\n", boldred.Sprintf("Stacktrace:"), err) + + if !strings.Contains(formatted, "runtime.goexit") { + _, _ = fmt.Fprint(h.Writer, formatted) + break + } + + // Inserts a new-line between sections of a stack. + // When wrapping errors, you get multiple separate stacks that start with their message, + // this allows us to separate them with a new-line and view them more easily. + // + // For example: + // + // Stacktrace: + // readlink test: no such file or directory + // failed to read symlink target for 'test' + // github.com/pterodactyl/wings/server/filesystem.(*Archive).addToArchive + // github.com/pterodactyl/wings/server/filesystem/archive.go:166 + // ... (Truncated the stack for easier reading) + // runtime.goexit + // runtime/asm_amd64.s:1374 + // **NEW LINE INSERTED HERE** + // backup: error while generating server backup + // github.com/pterodactyl/wings/server.(*Server).Backup + // github.com/pterodactyl/wings/server/backup.go:84 + // ... (Truncated the stack for easier reading) + // runtime.goexit + // runtime/asm_amd64.s:1374 + // + var b strings.Builder + var endOfStack bool + for _, s := range strings.Split(formatted, "\n") { + b.WriteString(s + "\n") + + if s == "runtime.goexit" { + endOfStack = true + continue + } + + if !endOfStack { + continue + } + + b.WriteString("\n") + endOfStack = false + } + + _, _ = fmt.Fprint(h.Writer, b.String()) } + + // Only one key with the name "error" can be in the map. + break } return nil diff --git a/router/downloader/downloader.go b/router/downloader/downloader.go index 84b410e..85034ed 100644 --- a/router/downloader/downloader.go +++ b/router/downloader/downloader.go @@ -108,7 +108,11 @@ func (dl *Download) Execute() error { dl.cancelFunc = &cancel defer dl.Cancel() - req, _ := http.NewRequestWithContext(ctx, http.MethodGet, dl.req.URL.String(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, dl.req.URL.String(), nil) + if err != nil { + return errors.WrapIf(err, "downloader: failed to create request") + } + req.Header.Set("User-Agent", "Pterodactyl Panel (https://pterodactyl.io)") res, err := client.Do(req) // lgtm [go/request-forgery] if err != nil { diff --git a/server/backup.go b/server/backup.go index c9817c2..7ec94cf 100644 --- a/server/backup.go +++ b/server/backup.go @@ -64,16 +64,13 @@ func (s *Server) Backup(b backup.BackupInterface) error { ad, err := b.Generate(s.Filesystem().Path(), ignored) if err != nil { - if notifyError := s.notifyPanelOfBackup(b.Identifier(), &backup.ArchiveDetails{}, false); notifyError != nil { - s.Log().WithFields(log.Fields{ - "backup": b.Identifier(), - "error": notifyError, - }).Warn("failed to notify panel of failed backup state") - } else { + if err := s.notifyPanelOfBackup(b.Identifier(), &backup.ArchiveDetails{}, false); err != nil { s.Log().WithFields(log.Fields{ "backup": b.Identifier(), "error": err, - }).Info("notified panel of failed backup state") + }).Warn("failed to notify panel of failed backup state") + } else { + s.Log().WithField("backup", b.Identifier()).Info("notified panel of failed backup state") } _ = s.Events().PublishJson(BackupCompletedEvent+":"+b.Identifier(), map[string]interface{}{ @@ -84,7 +81,7 @@ func (s *Server) Backup(b backup.BackupInterface) error { "file_size": 0, }) - return errors.WithMessage(err, "backup: error while generating server backup") + return errors.WrapIf(err, "backup: error while generating server backup") } // Try to notify the panel about the status of this backup. If for some reason this request diff --git a/server/filesystem/archive.go b/server/filesystem/archive.go index 347e8cb..771b532 100644 --- a/server/filesystem/archive.go +++ b/server/filesystem/archive.go @@ -78,10 +78,12 @@ func (a *Archive) Create(dst string) error { // that request. if len(a.Files) == 0 && len(a.Ignore) > 0 { i := ignore.CompileIgnoreLines(strings.Split(a.Ignore, "\n")...) + options.Callback = a.callback(tw, func(_ string, rp string) error { if i.MatchesPath(rp) { return godirwalk.SkipThis } + return nil }) } else if len(a.Files) > 0 { @@ -102,6 +104,7 @@ func (a *Archive) callback(tw *tar.Writer, opts ...func(path string, relative st } relative := filepath.ToSlash(strings.TrimPrefix(path, a.BasePath+string(filepath.Separator))) + // Call the additional options passed to this callback function. If any of them return // a non-nil error we will exit immediately. for _, opt := range opts { @@ -125,12 +128,14 @@ func (a *Archive) withFilesCallback(tw *tar.Writer) func(path string, de *godirw if p != f && !strings.HasPrefix(p, f) { continue } + // Once we have a match return a nil value here so that the loop stops and the // call to this function will correctly include the file in the archive. If there // are no matches we'll never make it to this line, and the final error returned // will be the godirwalk.SkipThis error. return nil } + return godirwalk.SkipThis }) } @@ -144,7 +149,7 @@ func (a *Archive) addToArchive(p string, rp string, w *tar.Writer) error { if os.IsNotExist(err) { return nil } - return errors.WithMessage(err, "failed to Lstat '"+rp+"'") + return errors.WrapIff(err, "failed to Lstat '"+rp+"'") } // Resolve the symlink target if the file is a symlink. @@ -154,18 +159,18 @@ func (a *Archive) addToArchive(p string, rp string, w *tar.Writer) error { target, err = os.Readlink(s.Name()) if err != nil { // Skip symlinks if the target does not exist. - if os.IsNotExist(err) { + /*if os.IsNotExist(err) { return nil - } + }*/ - return errors.WithMessagef(err, "failed to read symlink target for '%s'", rp) + return errors.WrapIff(err, "failed to read symlink target for '%s'", rp) } } // Get the tar FileInfoHeader in order to add the file to the archive. header, err := tar.FileInfoHeader(s, filepath.ToSlash(target)) if err != nil { - return errors.WithMessagef(err, "failed to get tar#FileInfoHeader for '%s'", rp) + return errors.WrapIff(err, "failed to get tar#FileInfoHeader for '%s'", rp) } // Fix the header name if the file is not a symlink. @@ -175,7 +180,7 @@ func (a *Archive) addToArchive(p string, rp string, w *tar.Writer) error { // Write the tar FileInfoHeader to the archive. if err := w.WriteHeader(header); err != nil { - return errors.WithMessagef(err, "failed to write tar#FileInfoHeader for '%s'", rp) + return errors.WrapIff(err, "failed to write tar#FileInfoHeader for '%s'", rp) } // If the size of the file is less than 1 (most likely for symlinks), skip writing the file. @@ -202,13 +207,13 @@ func (a *Archive) addToArchive(p string, rp string, w *tar.Writer) error { if os.IsNotExist(err) { return nil } - return errors.WithMessagef(err, "failed to open '%s' for copying", header.Name) + return errors.WrapIff(err, "failed to open '%s' for copying", header.Name) } defer f.Close() // Copy the file's contents to the archive using our buffer. if _, err := io.CopyBuffer(w, io.LimitReader(f, header.Size), buf); err != nil { - return errors.WithMessagef(err, "failed to copy '%s' to archive", header.Name) + return errors.WrapIff(err, "failed to copy '%s' to archive", header.Name) } return nil