From 5c96bda70a7afb2ce97cbb3cd70c64fc8cb94446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Thu, 12 May 2022 11:43:20 +0200 Subject: [PATCH] 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 --- commands/hugo.go | 5 +- commands/server.go | 20 +- common/herrors/error_locator.go | 58 ++-- common/herrors/error_locator_test.go | 52 ++-- common/herrors/file_error.go | 58 ++-- common/herrors/file_error_test.go | 2 +- common/loggers/loggers.go | 5 + common/loggers/loggers_test.go | 12 + config/configLoader.go | 2 +- hugolib/cascade_test.go | 2 +- hugolib/config.go | 2 +- hugolib/hugo_sites.go | 2 +- hugolib/hugo_sites_build_errors_test.go | 248 +++++++++++++++++- hugolib/page.go | 32 +-- hugolib/page__per_output.go | 2 +- hugolib/shortcode.go | 5 +- langs/i18n/translationProvider.go | 2 +- markup/goldmark/codeblocks/render.go | 11 +- resources/resource_transformers/js/build.go | 13 +- .../js/integration_test.go | 50 ++++ .../postcss/integration_test.go | 12 +- .../resource_transformers/postcss/postcss.go | 3 +- .../tocss/dartsass/transform.go | 9 +- .../embedded/templates/_server/error.html | 87 ++++++ .../embedded/templates/server/error.html | 63 ----- tpl/tplimpl/template.go | 45 ++-- tpl/tplimpl/template_errors.go | 2 +- 27 files changed, 600 insertions(+), 204 deletions(-) create mode 100644 tpl/tplimpl/embedded/templates/_server/error.html delete mode 100644 tpl/tplimpl/embedded/templates/server/error.html diff --git a/commands/hugo.go b/commands/hugo.go index ada1e1cef..8dfd4b4bd 100644 --- 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 index 27b12cb32..036fa9eaa 100644 --- 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 index 5d2c10c04..0f22f545f 100644 --- 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 index 10b016fa8..6135657d8 100644 --- 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 index abd36cfbc..5cdb0e53b 100644 --- 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 index e6595aa28..41e244109 100644 --- 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 index 14c76ae45..c61c55a67 100644 --- 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 index 0c0cc859b..a7bd1ae12 100644 --- 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 index 6bbad7002..008ebbfae 100644 --- 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 index 0a7f66e6c..dff2082b6 100644 --- 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 index b2713758a..1e7cf6b06 100644 --- 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 index 9f18f33bc..4026f58d3 100644 --- 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 index 8f983075d..ffbfe1c17 100644 --- a/hugolib/hugo_sites_build_errors_test.go +++ b/hugolib/hugo_sites_build_errors_test.go @@ -347,13 +347,21 @@ 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 @@ -373,15 +381,237 @@ line 4 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 +-- 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, 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 index 4faefa3cc..e9f937105 100644 --- 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)) { + 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 } - if filepath.IsAbs(errfilename) { - filename = errfilename - } - f, ferr2 := p.s.SourceSpec.Fs.Source.Open(filename) - if ferr2 != 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 index fdce8e802..6460b120b 100644 --- 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 index 42877b537..f8cda6b8d 100644 --- a/hugolib/shortcode.go +++ b/hugolib/shortcode.go @@ -270,7 +270,6 @@ const ( innerNewlineRegexp = "\n" innerCleanupRegexp = `\A

(.*)

\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 index 52f9349c2..4c0934bad 100644 --- 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 index fa6d236a1..5ebc2a9ef 100644 --- 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 index d2fbf5065..00012b4e8 100644 --- 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 index 0c5b04a51..b9f466873 100644 --- 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 index c748f036b..4101818be 100644 --- 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 index dd7ad1418..a5c86df6f 100644 --- 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 index 082e30710..be4367b2f 100644 --- 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 new file mode 100644 index 000000000..77d581391 --- /dev/null +++ b/tpl/tplimpl/embedded/templates/_server/error.html @@ -0,0 +1,87 @@ + + + + + Hugo Server: Error + + + +
+ {{ $codeStyle := "dracula" }} +
+ {{ highlight .Error "apl" (printf "linenos=false,noclasses=true,style=%s" $codeStyle ) }} +
+
+ {{ 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 }} + {{ printf "%s:%d:%d" .Filename .LineNumber .ColumnNumber }}: + {{ end }} + {{ highlight (delimit .ErrorContext.Lines "\n") $lexer $params }} +
+ {{ end }} +

{{ .Version }}

+ Reload Page +
+ + diff --git a/tpl/tplimpl/embedded/templates/server/error.html b/tpl/tplimpl/embedded/templates/server/error.html deleted file mode 100644 index e5c6d0cbf..000000000 --- a/tpl/tplimpl/embedded/templates/server/error.html +++ /dev/null @@ -1,63 +0,0 @@ - - - - - Hugo Server: Error - - - -
- {{ 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" }} -

{{ path.Base .Position.Filename }}:

- {{ highlight (delimit .ErrorContext.Lines "\n") $lexer $params }} - {{ end }} -

{{ .Version }}

- Reload Page -
- - diff --git a/tpl/tplimpl/template.go b/tpl/tplimpl/template.go index c092ff638..25f7957fd 100644 --- 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 index d553a12e9..751b4ddbc 100644 --- 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) }