Skip to content

Commit

Permalink
Improve error messages, esp. when the server is running
Browse files Browse the repository at this point in the history
* Add file context to minifier errors when publishing
* Misc fixes (see issues)

To get to this, this commit also cleans up and simplifies the code surrounding errors and files. This also removes the usage of `github.com/pkg/errors`, mostly because of pkg/errors#223 -- but also because most of this is now built-in to Go.

Fixes gohugoio#9852
Fixes gohugoio#9857
Fixes gohugoio#9863
  • Loading branch information
bep committed May 6, 2022
1 parent fa2c45c commit 31c11dc
Show file tree
Hide file tree
Showing 108 changed files with 796 additions and 714 deletions.
14 changes: 8 additions & 6 deletions cache/filecache/filecache_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package filecache

import (
"fmt"
"path"
"path/filepath"
"strings"
Expand All @@ -25,8 +26,9 @@ import (

"github.com/gohugoio/hugo/helpers"

"errors"

"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"github.com/spf13/afero"
)

Expand Down Expand Up @@ -153,7 +155,7 @@ func DecodeConfig(fs afero.Fs, cfg config.Provider) (Configs, error) {
}

if err := decoder.Decode(v); err != nil {
return nil, errors.Wrap(err, "failed to decode filecache config")
return nil, fmt.Errorf("failed to decode filecache config: %w", err)
}

if cc.Dir == "" {
Expand All @@ -162,7 +164,7 @@ func DecodeConfig(fs afero.Fs, cfg config.Provider) (Configs, error) {

name := strings.ToLower(k)
if !valid[name] {
return nil, errors.Errorf("%q is not a valid cache name", name)
return nil, fmt.Errorf("%q is not a valid cache name", name)
}

c[name] = cc
Expand Down Expand Up @@ -197,12 +199,12 @@ func DecodeConfig(fs afero.Fs, cfg config.Provider) (Configs, error) {

if !v.isResourceDir {
if isOsFs && !filepath.IsAbs(v.Dir) {
return c, errors.Errorf("%q must resolve to an absolute directory", v.Dir)
return c, fmt.Errorf("%q must resolve to an absolute directory", v.Dir)
}

// Avoid cache in root, e.g. / (Unix) or c:\ (Windows)
if len(strings.TrimPrefix(v.Dir, filepath.VolumeName(v.Dir))) == 1 {
return c, errors.Errorf("%q is a root folder and not allowed as cache dir", v.Dir)
return c, fmt.Errorf("%q is a root folder and not allowed as cache dir", v.Dir)
}
}

Expand Down Expand Up @@ -242,5 +244,5 @@ func resolveDirPlaceholder(fs afero.Fs, cfg config.Provider, placeholder string)
return filepath.Base(workingDir), false, nil
}

return "", false, errors.Errorf("%q is not a valid placeholder (valid values are :cacheDir or :resourceDir)", placeholder)
return "", false, fmt.Errorf("%q is not a valid placeholder (valid values are :cacheDir or :resourceDir)", placeholder)
}
4 changes: 2 additions & 2 deletions cache/filecache/filecache_pruner.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
package filecache

import (
"fmt"
"io"
"os"

"github.com/gohugoio/hugo/hugofs"

"github.com/pkg/errors"
"github.com/spf13/afero"
)

Expand All @@ -39,7 +39,7 @@ func (c Caches) Prune() (int, error) {
if os.IsNotExist(err) {
continue
}
return counter, errors.Wrapf(err, "failed to prune cache %q", k)
return counter, fmt.Errorf("failed to prune cache %q: %w", k, err)
}

}
Expand Down
17 changes: 4 additions & 13 deletions commands/commandeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package commands

import (
"bytes"
"errors"
"io/ioutil"
"net"
Expand Down Expand Up @@ -141,19 +140,11 @@ func (c *commandeer) getErrorWithContext() any {

m := make(map[string]any)

m["Error"] = errors.New(removeErrorPrefixFromLog(c.logger.Errors()))
//xwm["Error"] = errors.New(cleanErrorLog(removeErrorPrefixFromLog(c.logger.Errors())))
m["Error"] = errors.New(cleanErrorLog(removeErrorPrefixFromLog(c.logger.Errors())))
m["Version"] = hugo.BuildVersionString()

fe := herrors.UnwrapErrorWithFileContext(c.buildErr)
if fe != nil {
m["File"] = fe
}

if c.h.verbose {
var b bytes.Buffer
herrors.FprintStackTraceFromErr(&b, c.buildErr)
m["StackTrace"] = b.String()
}
ferrors := herrors.UnwrapFileErrorsWithErrorContext(c.buildErr)
m["Files"] = ferrors

return m
}
Expand Down
4 changes: 1 addition & 3 deletions commands/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import (
"github.com/gohugoio/hugo/parser"
"github.com/gohugoio/hugo/parser/metadecoders"

"github.com/pkg/errors"

"github.com/gohugoio/hugo/hugolib"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -193,7 +191,7 @@ func (cc *convertCmd) convertAndSavePage(p page.Page, site *hugolib.Site, target

fs := hugofs.Os
if err := helpers.WriteToDisk(newFilename, &newContent, fs); err != nil {
return errors.Wrapf(err, "Failed to save file %q:", newFilename)
return fmt.Errorf("Failed to save file %q:: %w", newFilename, err)
}

return nil
Expand Down
19 changes: 7 additions & 12 deletions commands/hugo.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ import (

"github.com/gohugoio/hugo/resources/page"

"github.com/pkg/errors"

"github.com/gohugoio/hugo/common/herrors"
"github.com/gohugoio/hugo/common/hugo"
"github.com/gohugoio/hugo/common/loggers"
"github.com/gohugoio/hugo/common/terminal"
Expand Down Expand Up @@ -300,14 +297,14 @@ func (c *commandeer) fullBuild(noBuildLock bool) error {
copyStaticFunc := func() error {
cnt, err := c.copyStatic()
if err != nil {
return errors.Wrap(err, "Error copying static files")
return fmt.Errorf("Error copying static files: %w", err)
}
langCount = cnt
return nil
}
buildSitesFunc := func() error {
if err := c.buildSites(noBuildLock); err != nil {
return errors.Wrap(err, "Error building site")
return fmt.Errorf("Error building site: %w", err)
}
return nil
}
Expand Down Expand Up @@ -354,10 +351,10 @@ func (c *commandeer) initCPUProfile() (func(), error) {

f, err := os.Create(c.h.cpuprofile)
if err != nil {
return nil, errors.Wrap(err, "failed to create CPU profile")
return nil, fmt.Errorf("failed to create CPU profile: %w", err)
}
if err := pprof.StartCPUProfile(f); err != nil {
return nil, errors.Wrap(err, "failed to start CPU profile")
return nil, fmt.Errorf("failed to start CPU profile: %w", err)
}
return func() {
pprof.StopCPUProfile()
Expand Down Expand Up @@ -388,11 +385,11 @@ func (c *commandeer) initTraceProfile() (func(), error) {

f, err := os.Create(c.h.traceprofile)
if err != nil {
return nil, errors.Wrap(err, "failed to create trace file")
return nil, fmt.Errorf("failed to create trace file: %w", err)
}

if err := trace.Start(f); err != nil {
return nil, errors.Wrap(err, "failed to start trace")
return nil, fmt.Errorf("failed to start trace: %w", err)
}

return func() {
Expand Down Expand Up @@ -735,9 +732,7 @@ func (c *commandeer) handleBuildErr(err error, msg string) {

c.logger.Errorln(msg + ":\n")
c.logger.Errorln(helpers.FirstUpper(err.Error()))
if !c.h.quiet && c.h.verbose {
herrors.PrintStackTraceFromErr(err)
}

}

func (c *commandeer) rebuildSites(events []fsnotify.Event) error {
Expand Down
5 changes: 2 additions & 3 deletions commands/new_site.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ package commands
import (
"bytes"
"errors"
"fmt"
"path/filepath"
"strings"

"github.com/gohugoio/hugo/config"
"github.com/gohugoio/hugo/parser/metadecoders"

_errors "github.com/pkg/errors"

"github.com/gohugoio/hugo/create"
"github.com/gohugoio/hugo/helpers"
"github.com/gohugoio/hugo/hugofs"
Expand Down Expand Up @@ -94,7 +93,7 @@ func (n *newSiteCmd) doNewSite(fs *hugofs.Fs, basepath string, force bool) error

for _, dir := range dirs {
if err := fs.Source.MkdirAll(dir, 0777); err != nil {
return _errors.Wrap(err, "Failed to create dir")
return fmt.Errorf("Failed to create dir: %w", err)
}
}

Expand Down
21 changes: 17 additions & 4 deletions commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ import (
"github.com/gohugoio/hugo/common/paths"
"golang.org/x/sync/errgroup"

"github.com/pkg/errors"

"github.com/gohugoio/hugo/livereload"

"github.com/gohugoio/hugo/config"
Expand Down Expand Up @@ -366,7 +364,7 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
// We're only interested in the path
u, err := url.Parse(baseURL)
if err != nil {
return nil, nil, "", "", errors.Wrap(err, "Invalid baseURL")
return nil, nil, "", "", fmt.Errorf("Invalid baseURL: %w", err)
}

decorate := func(h http.Handler) http.Handler {
Expand Down Expand Up @@ -480,6 +478,21 @@ var logErrorRe = regexp.MustCompile(`(?s)ERROR \d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{
func removeErrorPrefixFromLog(content string) string {
return logErrorRe.ReplaceAllLiteralString(content, "")
}
func cleanErrorLog(content string) string {
content = strings.ReplaceAll(content, "Rebuild failed:\n", "")
content = strings.ReplaceAll(content, "\n", "")
seen := make(map[string]bool)
parts := strings.Split(content, ": ")
keep := make([]string, 0, len(parts))
for _, part := range parts {
if seen[part] {
continue
}
seen[part] = true
keep = append(keep, part)
}
return strings.Join(keep, ": ")
}

func (c *commandeer) serve(s *serverCmd) error {
isMultiHost := c.hugo().IsMultihost()
Expand Down Expand Up @@ -627,7 +640,7 @@ func (sc *serverCmd) fixURL(cfg config.Provider, s string, port int) (string, er
if strings.Contains(u.Host, ":") {
u.Host, _, err = net.SplitHostPort(u.Host)
if err != nil {
return "", errors.Wrap(err, "Failed to split baseURL hostpost")
return "", fmt.Errorf("Failed to split baseURL hostpost: %w", err)
}
}
u.Host += fmt.Sprintf(":%d", port)
Expand Down
14 changes: 8 additions & 6 deletions commands/server_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var buildErrorTemplate = `<!doctype html>
body {
font-family: "Muli",avenir, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol";
font-size: 16px;
color: #48b685;
background-color: #2f1e2e;
}
main {
Expand Down Expand Up @@ -68,13 +69,14 @@ var buildErrorTemplate = `<!doctype html>
<body>
<main>
{{ highlight .Error "apl" "linenos=false,noclasses=true,style=paraiso-dark" }}
{{ with .File }}
{{ $params := printf "noclasses=true,style=paraiso-dark,linenos=table,hl_lines=%d,linenostart=%d" (add .LinesPos 1) (sub .Position.LineNumber .LinesPos) }}
{{ $lexer := .ChromaLexer | default "go-html-template" }}
{{ highlight (delimit .Lines "\n") $lexer $params }}
{{ range $i, $e := .Files }}
{{ if not .ErrorContext }}
{{ continue }}
{{ end }}
{{ with .StackTrace }}
{{ highlight . "apl" "noclasses=true,style=paraiso-dark" }}
{{ $params := printf "noclasses=true,style=paraiso-dark,linenos=table,hl_lines=%d,linenostart=%d" (add .ErrorContext.LinesPos 1) (sub .Position.LineNumber .ErrorContext.LinesPos) }}
{{ $lexer := .ErrorContext.ChromaLexer | default "go-html-template" }}
<h3><code>{{ path.Base .Position.Filename }}:</code></h3>
{{ highlight (delimit .ErrorContext.Lines "\n") $lexer $params }}
{{ end }}
<p class="version">{{ .Version }}</p>
<a href="">Reload Page</a>
Expand Down
Loading

0 comments on commit 31c11dc

Please sign in to comment.