commit 5c96bda70a7afb2ce97cbb3cd70c64fc8cb94446 parent 4a96df96d958a8ce122f103c4b417eaba52e6cb1 Author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com> Date: Thu, 12 May 2022 11:43:20 +0200 errors: Misc improvements * Redo the server error template * Always add the content file context if relevant * Remove some now superflous error string matching * Move the server error template to _server/error.html * Add file context (with position) to codeblock render blocks * Improve JS build errors Fixes #9892 Fixes #9891 Fixes #9893 Diffstat:
27 files changed, 600 insertions(+), 204 deletions(-) diff --git a/commands/hugo.go b/commands/hugo.go @@ -736,10 +736,7 @@ func (c *commandeer) buildSites(noBuildLock bool) (err error) { func (c *commandeer) handleBuildErr(err error, msg string) { c.buildErr = err - - c.logger.Errorln(msg + ":\n") - c.logger.Errorln(helpers.FirstUpper(err.Error())) - + c.logger.Errorln(msg + ": " + cleanErrorLog(err.Error())) } func (c *commandeer) rebuildSites(events []fsnotify.Event) error { diff --git a/commands/server.go b/commands/server.go @@ -469,14 +469,26 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string return mu, listener, u.String(), endpoint, nil } -var logErrorRe = regexp.MustCompile(`(?s)ERROR \d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `) +var ( + logErrorRe = regexp.MustCompile(`(?s)ERROR \d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `) + logDuplicateTemplateExecuteRe = regexp.MustCompile(`: template: .*?:\d+:\d+: executing ".*?"`) + logDuplicateTemplateParseRe = regexp.MustCompile(`: template: .*?:\d+:\d*`) +) func removeErrorPrefixFromLog(content string) string { return logErrorRe.ReplaceAllLiteralString(content, "") } + +var logReplacer = strings.NewReplacer( + "can't", "can’t", // Chroma lexer does'nt do well with "can't" + "*hugolib.pageState", "page.Page", // Page is the public interface. +) + func cleanErrorLog(content string) string { - content = strings.ReplaceAll(content, "Rebuild failed:\n", "") - content = strings.ReplaceAll(content, "\n", "") + content = strings.ReplaceAll(content, "\n", " ") + content = logReplacer.Replace(content) + content = logDuplicateTemplateExecuteRe.ReplaceAllString(content, "") + content = logDuplicateTemplateParseRe.ReplaceAllString(content, "") seen := make(map[string]bool) parts := strings.Split(content, ": ") keep := make([]string, 0, len(parts)) @@ -515,7 +527,7 @@ func (c *commandeer) serve(s *serverCmd) error { c: c, s: s, errorTemplate: func(ctx any) (io.Reader, error) { - templ, found := c.hugo().Tmpl().Lookup("server/error.html") + templ, found := c.hugo().Tmpl().Lookup("_server/error.html") if !found { panic("template server/error.html not found") } diff --git a/common/herrors/error_locator.go b/common/herrors/error_locator.go @@ -34,11 +34,22 @@ type LineMatcher struct { } // LineMatcherFn is used to match a line with an error. -type LineMatcherFn func(m LineMatcher) bool +// It returns the column number or 0 if the line was found, but column could not be determinde. Returns -1 if no line match. +type LineMatcherFn func(m LineMatcher) int // SimpleLineMatcher simply matches by line number. -var SimpleLineMatcher = func(m LineMatcher) bool { - return m.Position.LineNumber == m.LineNumber +var SimpleLineMatcher = func(m LineMatcher) int { + if m.Position.LineNumber == m.LineNumber { + // We found the line, but don't know the column. + return 0 + } + return -1 +} + +// NopLineMatcher is a matcher that always returns 1. +// This will effectively give line 1, column 1. +var NopLineMatcher = func(m LineMatcher) int { + return 1 } // ErrorContext contains contextual information about an error. This will @@ -52,6 +63,10 @@ type ErrorContext struct { // The position of the error in the Lines above. 0 based. LinesPos int + // The position of the content in the file. Note that this may be different from the error's position set + // in FileError. + Position text.Position + // The lexer to use for syntax highlighting. // https://gohugo.io/content-management/syntax-highlighting/#list-of-chroma-highlighting-languages ChromaLexer string @@ -78,31 +93,24 @@ func chromaLexerFromFilename(filename string) string { return chromaLexerFromType(ext) } -func locateErrorInString(src string, matcher LineMatcherFn) (*ErrorContext, text.Position) { +func locateErrorInString(src string, matcher LineMatcherFn) *ErrorContext { return locateError(strings.NewReader(src), &fileError{}, matcher) } -func locateError(r io.Reader, le FileError, matches LineMatcherFn) (*ErrorContext, text.Position) { +func locateError(r io.Reader, le FileError, matches LineMatcherFn) *ErrorContext { if le == nil { panic("must provide an error") } - errCtx := &ErrorContext{LinesPos: -1} - pos := text.Position{LineNumber: -1, ColumnNumber: 1, Offset: -1} + ectx := &ErrorContext{LinesPos: -1, Position: text.Position{Offset: -1}} b, err := ioutil.ReadAll(r) if err != nil { - return errCtx, pos + return ectx } - lepos := le.Position() - lines := strings.Split(string(b), "\n") - if lepos.ColumnNumber >= 0 { - pos.ColumnNumber = lepos.ColumnNumber - } - lineNo := 0 posBytes := 0 @@ -115,34 +123,36 @@ func locateError(r io.Reader, le FileError, matches LineMatcherFn) (*ErrorContex Offset: posBytes, Line: line, } - if errCtx.LinesPos == -1 && matches(m) { - pos.LineNumber = lineNo + v := matches(m) + if ectx.LinesPos == -1 && v != -1 { + ectx.Position.LineNumber = lineNo + ectx.Position.ColumnNumber = v break } posBytes += len(line) } - if pos.LineNumber != -1 { - low := pos.LineNumber - 3 + if ectx.Position.LineNumber > 0 { + low := ectx.Position.LineNumber - 3 if low < 0 { low = 0 } - if pos.LineNumber > 2 { - errCtx.LinesPos = 2 + if ectx.Position.LineNumber > 2 { + ectx.LinesPos = 2 } else { - errCtx.LinesPos = pos.LineNumber - 1 + ectx.LinesPos = ectx.Position.LineNumber - 1 } - high := pos.LineNumber + 2 + high := ectx.Position.LineNumber + 2 if high > len(lines) { high = len(lines) } - errCtx.Lines = lines[low:high] + ectx.Lines = lines[low:high] } - return errCtx, pos + return ectx } diff --git a/common/herrors/error_locator_test.go b/common/herrors/error_locator_test.go @@ -24,8 +24,11 @@ import ( func TestErrorLocator(t *testing.T) { c := qt.New(t) - lineMatcher := func(m LineMatcher) bool { - return strings.Contains(m.Line, "THEONE") + lineMatcher := func(m LineMatcher) int { + if strings.Contains(m.Line, "THEONE") { + return 1 + } + return -1 } lines := `LINE 1 @@ -38,23 +41,25 @@ LINE 7 LINE 8 ` - location, pos := locateErrorInString(lines, lineMatcher) + location := locateErrorInString(lines, lineMatcher) + pos := location.Position c.Assert(location.Lines, qt.DeepEquals, []string{"LINE 3", "LINE 4", "This is THEONE", "LINE 6", "LINE 7"}) c.Assert(pos.LineNumber, qt.Equals, 5) c.Assert(location.LinesPos, qt.Equals, 2) locate := func(s string, m LineMatcherFn) *ErrorContext { - ctx, _ := locateErrorInString(s, m) + ctx := locateErrorInString(s, m) return ctx } c.Assert(locate(`This is THEONE`, lineMatcher).Lines, qt.DeepEquals, []string{"This is THEONE"}) - location, pos = locateErrorInString(`L1 + location = locateErrorInString(`L1 This is THEONE L2 `, lineMatcher) + pos = location.Position c.Assert(pos.LineNumber, qt.Equals, 2) c.Assert(location.LinesPos, qt.Equals, 1) c.Assert(location.Lines, qt.DeepEquals, []string{"L1", "This is THEONE", "L2", ""}) @@ -78,16 +83,20 @@ This THEONE c.Assert(location.Lines, qt.DeepEquals, []string{"L1", "L2", "This THEONE", ""}) c.Assert(location.LinesPos, qt.Equals, 2) - location, pos = locateErrorInString("NO MATCH", lineMatcher) - c.Assert(pos.LineNumber, qt.Equals, -1) + location = locateErrorInString("NO MATCH", lineMatcher) + pos = location.Position + c.Assert(pos.LineNumber, qt.Equals, 0) c.Assert(location.LinesPos, qt.Equals, -1) c.Assert(len(location.Lines), qt.Equals, 0) - lineMatcher = func(m LineMatcher) bool { - return m.LineNumber == 6 + lineMatcher = func(m LineMatcher) int { + if m.LineNumber == 6 { + return 1 + } + return -1 } - location, pos = locateErrorInString(`A + location = locateErrorInString(`A B C D @@ -97,35 +106,46 @@ G H I J`, lineMatcher) + pos = location.Position c.Assert(location.Lines, qt.DeepEquals, []string{"D", "E", "F", "G", "H"}) c.Assert(pos.LineNumber, qt.Equals, 6) c.Assert(location.LinesPos, qt.Equals, 2) // Test match EOF - lineMatcher = func(m LineMatcher) bool { - return m.LineNumber == 4 + lineMatcher = func(m LineMatcher) int { + if m.LineNumber == 4 { + return 1 + } + return -1 } - location, pos = locateErrorInString(`A + location = locateErrorInString(`A B C `, lineMatcher) + pos = location.Position + c.Assert(location.Lines, qt.DeepEquals, []string{"B", "C", ""}) c.Assert(pos.LineNumber, qt.Equals, 4) c.Assert(location.LinesPos, qt.Equals, 2) - offsetMatcher := func(m LineMatcher) bool { - return m.Offset == 1 + offsetMatcher := func(m LineMatcher) int { + if m.Offset == 1 { + return 1 + } + return -1 } - location, pos = locateErrorInString(`A + location = locateErrorInString(`A B C D E`, offsetMatcher) + pos = location.Position + c.Assert(location.Lines, qt.DeepEquals, []string{"A", "B", "C", "D"}) c.Assert(pos.LineNumber, qt.Equals, 2) c.Assert(location.LinesPos, qt.Equals, 1) diff --git a/common/herrors/file_error.go b/common/herrors/file_error.go @@ -73,37 +73,40 @@ func (fe *fileError) UpdateContent(r io.Reader, linematcher LineMatcherFn) FileE } var ( - contentPos text.Position - posle = fe.position - errorContext *ErrorContext + posle = fe.position + ectx *ErrorContext ) if posle.LineNumber <= 1 && posle.Offset > 0 { // Try to locate the line number from the content if offset is set. - errorContext, contentPos = locateError(r, fe, func(m LineMatcher) bool { + ectx = locateError(r, fe, func(m LineMatcher) int { if posle.Offset >= m.Offset && posle.Offset < m.Offset+len(m.Line) { lno := posle.LineNumber - m.Position.LineNumber + m.LineNumber m.Position = text.Position{LineNumber: lno} return linematcher(m) } - return false + return -1 }) } else { - errorContext, contentPos = locateError(r, fe, linematcher) + ectx = locateError(r, fe, linematcher) } - if errorContext.ChromaLexer == "" { + if ectx.ChromaLexer == "" { if fe.fileType != "" { - errorContext.ChromaLexer = chromaLexerFromType(fe.fileType) + ectx.ChromaLexer = chromaLexerFromType(fe.fileType) } else { - errorContext.ChromaLexer = chromaLexerFromFilename(fe.Position().Filename) + ectx.ChromaLexer = chromaLexerFromFilename(fe.Position().Filename) } } - fe.errorContext = errorContext + fe.errorContext = ectx - if contentPos.LineNumber > 0 { - fe.position.LineNumber = contentPos.LineNumber + if ectx.Position.LineNumber > 0 { + fe.position.LineNumber = ectx.Position.LineNumber + } + + if ectx.Position.ColumnNumber > 0 { + fe.position.ColumnNumber = ectx.Position.ColumnNumber } return fe @@ -144,10 +147,6 @@ func (e *fileError) Unwrap() error { // The value for name should identify the file, the best // being the full filename to the file on disk. func NewFileError(name string, err error) FileError { - if err == nil { - panic("err is nil") - } - // Filetype is used to determine the Chroma lexer to use. fileType, pos := extractFileTypePos(err) pos.Filename = name @@ -155,10 +154,17 @@ func NewFileError(name string, err error) FileError { _, fileType = paths.FileAndExtNoDelimiter(filepath.Clean(name)) } - if pos.LineNumber < 0 { - panic(fmt.Sprintf("invalid line number: %d", pos.LineNumber)) - } + return &fileError{cause: err, fileType: fileType, position: pos} +} + +// NewFileErrorFromPos will use the filename and line number from pos to create a new FileError, wrapping err. +func NewFileErrorFromPos(pos text.Position, err error) FileError { + // Filetype is used to determine the Chroma lexer to use. + fileType, _ := extractFileTypePos(err) + if fileType == "" { + _, fileType = paths.FileAndExtNoDelimiter(filepath.Clean(pos.Filename)) + } return &fileError{cause: err, fileType: fileType, position: pos} } @@ -191,7 +197,7 @@ func extractFileTypePos(err error) (string, text.Position) { err = Cause(err) var fileType string - // Fall back to line/col 1:1 if we cannot find any better information. + // Default to line 1 col 1 if we don't find any better. pos := text.Position{ Offset: -1, LineNumber: 1, @@ -242,6 +248,18 @@ func UnwrapFileError(err error) FileError { return nil } +// UnwrapFileErrors tries to unwrap all FileError. +func UnwrapFileErrors(err error) []FileError { + var errs []FileError + for err != nil { + if v, ok := err.(FileError); ok { + errs = append(errs, v) + } + err = errors.Unwrap(err) + } + return errs +} + // UnwrapFileErrorsWithErrorContext tries to unwrap all FileError in err that has an ErrorContext. func UnwrapFileErrorsWithErrorContext(err error) []FileError { var errs []FileError diff --git a/common/herrors/file_error_test.go b/common/herrors/file_error_test.go @@ -41,7 +41,7 @@ func TestNewFileError(t *testing.T) { fe.UpdatePosition(text.Position{LineNumber: 32, ColumnNumber: 2}) c.Assert(fe.Error(), qt.Equals, `"foo.html:32:2": bar`) fe.UpdatePosition(text.Position{LineNumber: 0, ColumnNumber: 0, Offset: 212}) - fe.UpdateContent(strings.NewReader(lines), SimpleLineMatcher) + fe.UpdateContent(strings.NewReader(lines), nil) c.Assert(fe.Error(), qt.Equals, `"foo.html:32:0": bar`) errorContext := fe.ErrorContext() c.Assert(errorContext, qt.IsNotNil) diff --git a/common/loggers/loggers.go b/common/loggers/loggers.go @@ -240,6 +240,11 @@ func NewBasicLoggerForWriter(t jww.Threshold, w io.Writer) Logger { return newLogger(t, jww.LevelError, w, ioutil.Discard, false) } +// RemoveANSIColours removes all ANSI colours from the given string. +func RemoveANSIColours(s string) string { + return ansiColorRe.ReplaceAllString(s, "") +} + var ( ansiColorRe = regexp.MustCompile("(?s)\\033\\[\\d*(;\\d*)*m") errorRe = regexp.MustCompile("^(ERROR|FATAL|WARN)") diff --git a/common/loggers/loggers_test.go b/common/loggers/loggers_test.go @@ -46,3 +46,15 @@ func TestLoggerToWriterWithPrefix(t *testing.T) { c.Assert(b.String(), qt.Equals, "myprefix: Hello Hugo!\n") } + +func TestRemoveANSIColours(t *testing.T) { + c := qt.New(t) + + c.Assert(RemoveANSIColours(""), qt.Equals, "") + c.Assert(RemoveANSIColours("\033[31m"), qt.Equals, "") + c.Assert(RemoveANSIColours("\033[31mHello"), qt.Equals, "Hello") + c.Assert(RemoveANSIColours("\033[31mHello\033[0m"), qt.Equals, "Hello") + c.Assert(RemoveANSIColours("\033[31mHello\033[0m World"), qt.Equals, "Hello World") + c.Assert(RemoveANSIColours("\033[31mHello\033[0m World\033[31m!"), qt.Equals, "Hello World!") + c.Assert(RemoveANSIColours("\x1b[90m 5 |"), qt.Equals, " 5 |") +} diff --git a/config/configLoader.go b/config/configLoader.go @@ -59,7 +59,7 @@ func FromConfigString(config, configType string) (Provider, error) { func FromFile(fs afero.Fs, filename string) (Provider, error) { m, err := loadConfigFromFile(fs, filename) if err != nil { - return nil, herrors.NewFileErrorFromFile(err, filename, filename, fs, herrors.SimpleLineMatcher) + return nil, herrors.NewFileErrorFromFile(err, filename, filename, fs, nil) } return NewFrom(m), nil } diff --git a/hugolib/cascade_test.go b/hugolib/cascade_test.go @@ -77,7 +77,7 @@ kind = '{section,term}' } builders := make([]*IntegrationTestBuilder, b.N) - for i, _ := range builders { + for i := range builders { builders[i] = NewIntegrationTestBuilder(cfg) } diff --git a/hugolib/config.go b/hugolib/config.go @@ -511,5 +511,5 @@ func (configLoader) loadSiteConfig(cfg config.Provider) (scfg SiteConfig, err er } func (l configLoader) wrapFileError(err error, filename string) error { - return herrors.NewFileErrorFromFile(err, filename, filename, l.Fs, herrors.SimpleLineMatcher) + return herrors.NewFileErrorFromFile(err, filename, filename, l.Fs, nil) } diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go @@ -968,7 +968,7 @@ func (h *HugoSites) errWithFileContext(err error, f source.File) error { } realFilename := fim.Meta().Filename - return herrors.NewFileErrorFromFile(err, realFilename, realFilename, h.SourceSpec.Fs.Source, herrors.SimpleLineMatcher) + return herrors.NewFileErrorFromFile(err, realFilename, realFilename, h.SourceSpec.Fs.Source, nil) } diff --git a/hugolib/hugo_sites_build_errors_test.go b/hugolib/hugo_sites_build_errors_test.go @@ -347,13 +347,75 @@ minify = true } -func TestErrorNested(t *testing.T) { +func TestErrorNestedRender(t *testing.T) { t.Parallel() files := ` -- config.toml -- +-- content/_index.md -- +--- +title: "Home" +--- +-- layouts/index.html -- +line 1 +line 2 +1{{ .Render "myview" }} +-- layouts/_default/myview.html -- +line 1 +12{{ partial "foo.html" . }} +line 4 +line 5 +-- layouts/partials/foo.html -- +line 1 +line 2 +123{{ .ThisDoesNotExist }} +line 4 +` + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + errors := herrors.UnwrapFileErrorsWithErrorContext(err) + b.Assert(errors, qt.HasLen, 4) + b.Assert(errors[0].Position().LineNumber, qt.Equals, 3) + b.Assert(errors[0].Position().ColumnNumber, qt.Equals, 4) + b.Assert(errors[0].Error(), qt.Contains, filepath.FromSlash(`"/layouts/index.html:3:4": execute of template failed`)) + b.Assert(errors[0].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "line 2", "1{{ .Render \"myview\" }}"}) + b.Assert(errors[2].Position().LineNumber, qt.Equals, 2) + b.Assert(errors[2].Position().ColumnNumber, qt.Equals, 5) + b.Assert(errors[2].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "12{{ partial \"foo.html\" . }}", "line 4", "line 5"}) + + b.Assert(errors[3].Position().LineNumber, qt.Equals, 3) + b.Assert(errors[3].Position().ColumnNumber, qt.Equals, 6) + b.Assert(errors[3].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "line 2", "123{{ .ThisDoesNotExist }}", "line 4"}) + +} + +func TestErrorNestedShortocde(t *testing.T) { + t.Parallel() + + files := ` +-- config.toml -- +-- content/_index.md -- +--- +title: "Home" +--- + +## Hello +{{< hello >}} + -- layouts/index.html -- line 1 +line 2 +{{ .Content }} +line 5 +-- layouts/shortcodes/hello.html -- +line 1 12{{ partial "foo.html" . }} line 4 line 5 @@ -373,15 +435,183 @@ line 4 b.Assert(err, qt.IsNotNil) errors := herrors.UnwrapFileErrorsWithErrorContext(err) + + b.Assert(errors, qt.HasLen, 3) + + b.Assert(errors[0].Position().LineNumber, qt.Equals, 6) + b.Assert(errors[0].Position().ColumnNumber, qt.Equals, 1) + b.Assert(errors[0].ErrorContext().ChromaLexer, qt.Equals, "md") + b.Assert(errors[0].Error(), qt.Contains, filepath.FromSlash(`"/content/_index.md:6:1": failed to render shortcode "hello": failed to process shortcode: "/layouts/shortcodes/hello.html:2:5":`)) + b.Assert(errors[0].ErrorContext().Lines, qt.DeepEquals, []string{"", "## Hello", "{{< hello >}}", ""}) + b.Assert(errors[1].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "12{{ partial \"foo.html\" . }}", "line 4", "line 5"}) + b.Assert(errors[2].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "line 2", "123{{ .ThisDoesNotExist }}", "line 4"}) + +} + +func TestErrorRenderHookHeading(t *testing.T) { + t.Parallel() + + files := ` +-- config.toml -- +-- content/_index.md -- +--- +title: "Home" +--- + +## Hello + +-- layouts/index.html -- +line 1 +line 2 +{{ .Content }} +line 5 +-- layouts/_default/_markup/render-heading.html -- +line 1 +12{{ .Levels }} +line 4 +line 5 +` + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + errors := herrors.UnwrapFileErrorsWithErrorContext(err) + b.Assert(errors, qt.HasLen, 2) - fmt.Println(errors[0]) - b.Assert(errors[0].Position().LineNumber, qt.Equals, 2) - b.Assert(errors[0].Position().ColumnNumber, qt.Equals, 5) - b.Assert(errors[0].Error(), qt.Contains, filepath.FromSlash(`"/layouts/index.html:2:5": execute of template failed`)) - b.Assert(errors[0].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "12{{ partial \"foo.html\" . }}", "line 4", "line 5"}) - b.Assert(errors[1].Position().LineNumber, qt.Equals, 3) - b.Assert(errors[1].Position().ColumnNumber, qt.Equals, 6) - b.Assert(errors[1].ErrorContext().Lines, qt.DeepEquals, []string{"line 1", "line 2", "123{{ .ThisDoesNotExist }}", "line 4"}) + b.Assert(errors[0].Error(), qt.Contains, filepath.FromSlash(`"/content/_index.md:1:1": "/layouts/_default/_markup/render-heading.html:2:5": execute of template failed`)) + +} + +func TestErrorRenderHookCodeblock(t *testing.T) { + t.Parallel() + + files := ` +-- config.toml -- +-- content/_index.md -- +--- +title: "Home" +--- + +## Hello + +§§§ foo +bar +§§§ + + +-- layouts/index.html -- +line 1 +line 2 +{{ .Content }} +line 5 +-- layouts/_default/_markup/render-codeblock-foo.html -- +line 1 +12{{ .Foo }} +line 4 +line 5 +` + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + errors := herrors.UnwrapFileErrorsWithErrorContext(err) + + b.Assert(errors, qt.HasLen, 2) + first := errors[0] + b.Assert(first.Error(), qt.Contains, filepath.FromSlash(`"/content/_index.md:7:1": "/layouts/_default/_markup/render-codeblock-foo.html:2:5": execute of template failed`)) + +} + +func TestErrorInBaseTemplate(t *testing.T) { + t.Parallel() + + filesTemplate := ` +-- config.toml -- +-- content/_index.md -- +--- +title: "Home" +--- +-- layouts/baseof.html -- +line 1 base +line 2 base +{{ block "main" . }}empty{{ end }} +line 4 base +{{ block "toc" . }}empty{{ end }} +-- layouts/index.html -- +{{ define "main" }} +line 2 index +line 3 index +line 4 index +{{ end }} +{{ define "toc" }} +TOC: {{ partial "toc.html" . }} +{{ end }} +-- layouts/partials/toc.html -- +toc line 1 +toc line 2 +toc line 3 +toc line 4 + + + + +` + + t.Run("base template", func(t *testing.T) { + files := strings.Replace(filesTemplate, "line 4 base", "123{{ .ThisDoesNotExist \"abc\" }}", 1) + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, filepath.FromSlash(`render of "home" failed: "/layouts/baseof.html:4:6"`)) + + }) + + t.Run("index template", func(t *testing.T) { + files := strings.Replace(filesTemplate, "line 3 index", "1234{{ .ThisDoesNotExist \"abc\" }}", 1) + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, filepath.FromSlash(`render of "home" failed: "/layouts/index.html:3:7"`)) + + }) + + t.Run("partial from define", func(t *testing.T) { + files := strings.Replace(filesTemplate, "toc line 2", "12345{{ .ThisDoesNotExist \"abc\" }}", 1) + + b, err := NewIntegrationTestBuilder( + IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, filepath.FromSlash(`render of "home" failed: "/layouts/index.html:7:8": execute of template failed`)) + b.Assert(err.Error(), qt.Contains, `execute of template failed: template: partials/toc.html:2:8: executing "partials/toc.html"`) + + }) } diff --git a/hugolib/page.go b/hugolib/page.go @@ -572,25 +572,23 @@ func (p *pageState) wrapError(err error) error { filename := p.File().Filename() - if ferr := herrors.UnwrapFileError(err); ferr != nil { + // Check if it's already added. + for _, ferr := range herrors.UnwrapFileErrors(err) { errfilename := ferr.Position().Filename - if ferr.ErrorContext() != nil || errfilename == "" || !(errfilename == pageFileErrorName || filepath.IsAbs(errfilename)) { - return err - } - if filepath.IsAbs(errfilename) { - filename = errfilename - } - f, ferr2 := p.s.SourceSpec.Fs.Source.Open(filename) - if ferr2 != nil { + if errfilename == filename { + if ferr.ErrorContext() == nil { + f, ioerr := p.s.SourceSpec.Fs.Source.Open(filename) + if ioerr != nil { + return err + } + defer f.Close() + ferr.UpdateContent(f, nil) + } return err } - defer f.Close() - pos := ferr.Position() - pos.Filename = filename - return ferr.UpdatePosition(pos).UpdateContent(f, herrors.SimpleLineMatcher) } - return herrors.NewFileErrorFromFile(err, filename, filename, p.s.SourceSpec.Fs.Source, herrors.SimpleLineMatcher) + return herrors.NewFileErrorFromFile(err, filename, filename, p.s.SourceSpec.Fs.Source, herrors.NopLineMatcher) } @@ -644,8 +642,10 @@ Loop: m, err := metadecoders.Default.UnmarshalToMap(it.Val, f) if err != nil { if fe, ok := err.(herrors.FileError); ok { - // Offset the starting position of front matter. pos := fe.Position() + // Apply the error to the content file. + pos.Filename = p.File().Filename() + // Offset the starting position of front matter. offset := iter.LineNumber() - 1 if f == metadecoders.YAML { offset -= 1 @@ -788,7 +788,7 @@ func (p *pageState) outputFormat() (f output.Format) { func (p *pageState) parseError(err error, input []byte, offset int) error { pos := p.posFromInput(input, offset) - return herrors.NewFileError("page.md", err).UpdatePosition(pos) + return herrors.NewFileError(p.File().Filename(), err).UpdatePosition(pos) } func (p *pageState) pathOrTitle() string { diff --git a/hugolib/page__per_output.go b/hugolib/page__per_output.go @@ -417,7 +417,7 @@ func (p *pageContentOutput) Render(layout ...string) (template.HTML, error) { // Make sure to send the *pageState and not the *pageContentOutput to the template. res, err := executeToString(p.p.s.Tmpl(), templ, p.p) if err != nil { - return "", p.p.wrapError(fmt.Errorf("failed to execute template %q v: %w", layout, err)) + return "", p.p.wrapError(fmt.Errorf("failed to execute template %s: %w", templ.Name(), err)) } return template.HTML(res), nil } diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go @@ -270,7 +270,6 @@ const ( innerNewlineRegexp = "\n" innerCleanupRegexp = `\A<p>(.*)</p>\n\z` innerCleanupExpand = "$1" - pageFileErrorName = "page.md" ) func renderShortcode( @@ -299,7 +298,7 @@ func renderShortcode( var err error tmpl, err = s.TextTmpl().Parse(templName, templStr) if err != nil { - fe := herrors.NewFileError(pageFileErrorName, err) + fe := herrors.NewFileError(p.File().Filename(), err) pos := fe.Position() pos.LineNumber += p.posOffset(sc.pos).LineNumber fe = fe.UpdatePosition(pos) @@ -392,7 +391,7 @@ func renderShortcode( result, err := renderShortcodeWithPage(s.Tmpl(), tmpl, data) if err != nil && sc.isInline { - fe := herrors.NewFileError("shortcode.md", err) + fe := herrors.NewFileError(p.File().Filename(), err) pos := fe.Position() pos.LineNumber += p.posOffset(sc.pos).LineNumber fe = fe.UpdatePosition(pos) diff --git a/langs/i18n/translationProvider.go b/langs/i18n/translationProvider.go @@ -138,6 +138,6 @@ func errWithFileContext(inerr error, r source.File) error { } defer f.Close() - return herrors.NewFileError(realFilename, inerr).UpdateContent(f, herrors.SimpleLineMatcher) + return herrors.NewFileError(realFilename, inerr).UpdateContent(f, nil) } diff --git a/markup/goldmark/codeblocks/render.go b/markup/goldmark/codeblocks/render.go @@ -19,6 +19,7 @@ import ( "sync" "github.com/alecthomas/chroma/lexers" + "github.com/gohugoio/hugo/common/herrors" htext "github.com/gohugoio/hugo/common/text" "github.com/gohugoio/hugo/markup/converter/hooks" "github.com/gohugoio/hugo/markup/goldmark/internal/render" @@ -114,8 +115,8 @@ func (r *htmlRenderer) renderCodeBlock(w util.BufWriter, src []byte, node ast.No } return htext.Position{ Filename: ctx.DocumentContext().Filename, - LineNumber: 0, - ColumnNumber: 0, + LineNumber: 1, + ColumnNumber: 1, } } @@ -128,7 +129,11 @@ func (r *htmlRenderer) renderCodeBlock(w util.BufWriter, src []byte, node ast.No ctx.AddIdentity(cr) - return ast.WalkContinue, err + if err != nil { + return ast.WalkContinue, herrors.NewFileErrorFromPos(cbctx.createPos(), err) + } + + return ast.WalkContinue, nil } type codeBlockContext struct { diff --git a/resources/resource_transformers/js/build.go b/resources/resource_transformers/js/build.go @@ -137,6 +137,12 @@ func (t *buildTransformation) Transform(ctx *resources.ResourceTransformationCtx return errors.New(msg.Text) } path := loc.File + if path == stdinImporter { + path = ctx.SourcePath + } + + errorMessage := msg.Text + errorMessage = strings.ReplaceAll(errorMessage, nsImportHugo+":", "") var ( f afero.File @@ -158,15 +164,16 @@ func (t *buildTransformation) Transform(ctx *resources.ResourceTransformationCtx } if err == nil { - fe := herrors.NewFileError(path, errors.New(msg.Text)). + fe := herrors. + NewFileError(path, errors.New(errorMessage)). UpdatePosition(text.Position{Offset: -1, LineNumber: loc.Line, ColumnNumber: loc.Column}). - UpdateContent(f, herrors.SimpleLineMatcher) + UpdateContent(f, nil) f.Close() return fe } - return fmt.Errorf("%s", msg.Text) + return fmt.Errorf("%s", errorMessage) } var errors []error diff --git a/resources/resource_transformers/js/integration_test.go b/resources/resource_transformers/js/integration_test.go @@ -209,3 +209,53 @@ TS2: {{ template "print" $ts2 }} function greeter(person) { `) } + +func TestBuildError(t *testing.T) { + c := qt.New(t) + + filesTemplate := ` +-- config.toml -- +disableKinds=["page", "section", "taxonomy", "term", "sitemap", "robotsTXT"] +-- assets/js/main.js -- +// A comment. +import { hello1, hello2 } from './util1'; +hello1(); +hello2(); +-- assets/js/util1.js -- +/* Some +comments. +*/ +import { hello3 } from './util2'; +export function hello1() { + return 'abcd'; +} +export function hello2() { + return hello3(); +} +-- assets/js/util2.js -- +export function hello3() { + return 'efgh'; +} +-- layouts/index.html -- +{{ $js := resources.Get "js/main.js" | js.Build }} +JS Content:{{ $js.Content }}:End: + + ` + + c.Run("Import from main not found", func(c *qt.C) { + c.Parallel() + files := strings.Replace(filesTemplate, "import { hello1, hello2 }", "import { hello1, hello2, FOOBAR }", 1) + b, err := hugolib.NewIntegrationTestBuilder(hugolib.IntegrationTestConfig{T: c, NeedsOsFS: true, TxtarString: files}).BuildE() + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, `main.js:2:25": No matching export`) + }) + + c.Run("Import from import not found", func(c *qt.C) { + c.Parallel() + files := strings.Replace(filesTemplate, "import { hello3 } from './util2';", "import { hello3, FOOBAR } from './util2';", 1) + b, err := hugolib.NewIntegrationTestBuilder(hugolib.IntegrationTestConfig{T: c, NeedsOsFS: true, TxtarString: files}).BuildE() + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, `util1.js:4:17": No matching export in`) + }) + +} diff --git a/resources/resource_transformers/postcss/integration_test.go b/resources/resource_transformers/postcss/integration_test.go @@ -22,7 +22,6 @@ import ( jww "github.com/spf13/jwalterweatherman" qt "github.com/frankban/quicktest" - "github.com/gohugoio/hugo/common/herrors" "github.com/gohugoio/hugo/htesting" "github.com/gohugoio/hugo/hugolib" ) @@ -116,7 +115,7 @@ func TestTransformPostCSS(t *testing.T) { b.AssertFileContent("public/index.html", ` Styles RelPermalink: /css/styles.css -Styles Content: Len: 770875| +Styles Content: Len: 770917| `) } @@ -138,13 +137,6 @@ func TestTransformPostCSSError(t *testing.T) { }).BuildE() s.AssertIsFileError(err) - fe := herrors.UnwrapFileError(err) - pos := fe.Position() - c.Assert(strings.TrimPrefix(pos.Filename, s.H.WorkingDir), qt.Equals, filepath.FromSlash("/assets/css/components/a.css")) - c.Assert(pos.LineNumber, qt.Equals, 4) - errctx := fe.ErrorContext() - c.Assert(errctx, qt.IsNotNil) - c.Assert(errctx.Lines, qt.DeepEquals, []string{"/* Another comment. */", "class-in-a {", "\t@apply foo;", "}", ""}) - c.Assert(errctx.ChromaLexer, qt.Equals, "css") + c.Assert(err.Error(), qt.Contains, "a.css:4:2") } diff --git a/resources/resource_transformers/postcss/postcss.go b/resources/resource_transformers/postcss/postcss.go @@ -368,7 +368,8 @@ func (imp *importResolver) shouldImport(s string) bool { } func (imp *importResolver) toFileError(output string) error { - inErr := errors.New(strings.TrimSpace(output)) + output = strings.TrimSpace(loggers.RemoveANSIColours(output)) + inErr := errors.New(output) match := cssSyntaxErrorRe.FindStringSubmatch(output) if match == nil { diff --git a/resources/resource_transformers/tocss/dartsass/transform.go b/resources/resource_transformers/tocss/dartsass/transform.go @@ -116,8 +116,13 @@ func (t *transform) Transform(ctx *resources.ResourceTransformationCtx) error { filename = filename[len(stdinPrefix):] } - offsetMatcher := func(m herrors.LineMatcher) bool { - return m.Offset+len(m.Line) >= start.Offset && strings.Contains(m.Line, context) + offsetMatcher := func(m herrors.LineMatcher) int { + if m.Offset+len(m.Line) >= start.Offset && strings.Contains(m.Line, context) { + // We found the line, but return 0 to signal that we want to determine + // the column from the error. + return 0 + } + return -1 } return herrors.NewFileErrorFromFile(sassErr, filename, filename, hugofs.Os, offsetMatcher) diff --git a/tpl/tplimpl/embedded/templates/_server/error.html b/tpl/tplimpl/embedded/templates/_server/error.html @@ -0,0 +1,87 @@ +<!DOCTYPE html> +<html class="no-js" lang=""> + <head> + <meta charset="utf-8" /> + <title>Hugo Server: Error</title> + <style type="text/css"> + body { + font-family: "Muli", system-ui, -apple-system, "Segoe UI", Roboto, + "Helvetica Neue", "Noto Sans", "Liberation Sans", Arial, sans-serif, + "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", + "Noto Color Emoji"; + font-size: 14px; + background-color: #272a36; + } + main { + max-width: 100ch; + padding: 2ch; + margin: auto; + } + + .version { + font-size: 0.75rem; + color: #7c7c7c; + } + + hr { + margin-bottom: 1rem; + border: none; + height: 1px; + background-color: #3d3d3d; + } + pre, + code { + white-space: pre-wrap; + white-space: -moz-pre-wrap; + white-space: -pre-wrap; + white-space: -o-pre-wrap; + word-wrap: break-word; + font-family: SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", + "Courier New", monospace; + } + .error pre { + line-height: 1.5; + } + .filename { + color: #eef78a; + font-size: 0.9rem; + line-height: 1.5; + } + .highlight { + overflow-x: auto; + } + a { + color: #0594cb; + text-decoration: none; + } + a:hover { + color: #ccc; + } + </style> + </head> + <body> + <main> + {{ $codeStyle := "dracula" }} + <div class="error"> + {{ highlight .Error "apl" (printf "linenos=false,noclasses=true,style=%s" $codeStyle ) }} + </div> + <hr /> + {{ range $i, $e := .Files }} + {{ if not .ErrorContext }} + {{ continue }} + {{ end }} + {{ $params := printf "noclasses=true,style=%s,linenos=table,hl_lines=%d,linenostart=%d" $codeStyle (add .ErrorContext.LinesPos 1) (sub .Position.LineNumber .ErrorContext.LinesPos) }} + {{ $lexer := .ErrorContext.ChromaLexer | default "go-html-template" }} + {{ with .Position }} + <code class="filename" + >{{ printf "%s:%d:%d" .Filename .LineNumber .ColumnNumber }}:</code + > + {{ end }} + {{ highlight (delimit .ErrorContext.Lines "\n") $lexer $params }} + <hr /> + {{ end }} + <p class="version">{{ .Version }}</p> + <a href="">Reload Page</a> + </main> + </body> +</html> diff --git a/tpl/tplimpl/embedded/templates/server/error.html b/tpl/tplimpl/embedded/templates/server/error.html @@ -1,63 +0,0 @@ -<!DOCTYPE html> -<html class="no-js" lang=""> - <head> - <meta charset="utf-8" /> - <title>Hugo Server: Error</title> - <style type="text/css"> - 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 { - margin: auto; - width: 95%; - padding: 1rem; - } - .version { - color: #ccc; - padding: 1rem 0; - } - .stack { - margin-top: 4rem; - } - pre { - white-space: pre-wrap; - white-space: -moz-pre-wrap; - white-space: -pre-wrap; - white-space: -o-pre-wrap; - word-wrap: break-word; - } - .highlight { - overflow-x: auto; - margin-bottom: 1rem; - } - a { - color: #0594cb; - text-decoration: none; - } - a:hover { - color: #ccc; - } - </style> - </head> - <body> - <main> - {{ highlight .Error "apl" "linenos=false,noclasses=true,style=paraiso-dark" }} - {{ range $i, $e := .Files }} - {{ if not .ErrorContext }} - {{ continue }} - {{ end }} - {{ $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> - </main> - </body> -</html> diff --git a/tpl/tplimpl/template.go b/tpl/tplimpl/template.go @@ -61,6 +61,7 @@ const ( // The identifiers may be truncated in the log, e.g. // "executing "main" at <$scaled.SRelPermalin...>: can't evaluate field SRelPermalink in type *resource.Image" +// We need this to identify position in templates with base templates applied. var identifiersRe = regexp.MustCompile(`at \<(.*?)(\.{3})?\>:`) var embeddedTemplatesAliases = map[string][]string{ @@ -524,25 +525,27 @@ func (t *templateHandler) addFileContext(templ tpl.Template, inerr error) error return inerr } + identifiers := t.extractIdentifiers(inerr.Error()) + //lint:ignore ST1008 the error is the main result checkFilename := func(info templateInfo, inErr error) (error, bool) { if info.filename == "" { return inErr, false } - lineMatcher := func(m herrors.LineMatcher) bool { + lineMatcher := func(m herrors.LineMatcher) int { if m.Position.LineNumber != m.LineNumber { - return false + return -1 } - identifiers := t.extractIdentifiers(m.Error.Error()) - for _, id := range identifiers { if strings.Contains(m.Line, id) { - return true + // We found the line, but return a 0 to signal to + // use the column from the error message. + return 0 } } - return false + return -1 } f, err := t.layoutsFs.Open(info.filename) @@ -551,7 +554,13 @@ func (t *templateHandler) addFileContext(templ tpl.Template, inerr error) error } defer f.Close() - return herrors.NewFileError(info.realFilename, inErr).UpdateContent(f, lineMatcher), true + fe := herrors.NewFileError(info.realFilename, inErr) + fe.UpdateContent(f, lineMatcher) + + if !fe.ErrorContext().Position.IsValid() { + return inErr, false + } + return fe, true } inerr = fmt.Errorf("execute of template failed: %w", inerr) @@ -565,6 +574,15 @@ func (t *templateHandler) addFileContext(templ tpl.Template, inerr error) error return err } +func (t *templateHandler) extractIdentifiers(line string) []string { + m := identifiersRe.FindAllStringSubmatch(line, -1) + identifiers := make([]string, len(m)) + for i := 0; i < len(m); i++ { + identifiers[i] = m[i][1] + } + return identifiers +} + func (t *templateHandler) addShortcodeVariant(ts *templateState) { name := ts.Name() base := templateBaseName(templateShortcode, name) @@ -721,18 +739,9 @@ func (t *templateHandler) applyTemplateTransformers(ns *templateNamespace, ts *t return c, err } -func (t *templateHandler) extractIdentifiers(line string) []string { - m := identifiersRe.FindAllStringSubmatch(line, -1) - identifiers := make([]string, len(m)) - for i := 0; i < len(m); i++ { - identifiers[i] = m[i][1] - } - return identifiers -} - //go:embed embedded/templates/* //go:embed embedded/templates/_default/* -//go:embed embedded/templates/server/* +//go:embed embedded/templates/_server/* var embededTemplatesFs embed.FS func (t *templateHandler) loadEmbedded() error { @@ -755,7 +764,7 @@ func (t *templateHandler) loadEmbedded() error { // For the render hooks and the server templates it does not make sense to preseve the // double _indternal double book-keeping, // just add it if its now provided by the user. - if !strings.Contains(path, "_default/_markup") && !strings.HasPrefix(name, "server/") { + if !strings.Contains(path, "_default/_markup") && !strings.HasPrefix(name, "_server/") { templateName = internalPathPrefix + name } diff --git a/tpl/tplimpl/template_errors.go b/tpl/tplimpl/template_errors.go @@ -59,6 +59,6 @@ func (info templateInfo) errWithFileContext(what string, err error) error { return err } defer f.Close() - return fe.UpdateContent(f, herrors.SimpleLineMatcher) + return fe.UpdateContent(f, nil) }