Skip to content

Commit

Permalink
Improve SASS errors
Browse files Browse the repository at this point in the history
  • Loading branch information
bep committed May 15, 2022
1 parent 8b6361f commit 522e44b
Show file tree
Hide file tree
Showing 24 changed files with 306 additions and 69 deletions.
1 change: 1 addition & 0 deletions commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ func removeErrorPrefixFromLog(content string) string {
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.
"Rebuild failed:", "",
)

func cleanErrorLog(content string) string {
Expand Down
10 changes: 10 additions & 0 deletions common/herrors/error_locator.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ var NopLineMatcher = func(m LineMatcher) int {
return 1
}

// OffsetMatcher is a line matcher that matches by offset.
var OffsetMatcher = func(m LineMatcher) int {
if m.Offset+len(m.Line) >= m.Position.Offset {
// We found the line, but return 0 to signal that we want to determine
// the column from the error.
return 0
}
return -1
}

// ErrorContext contains contextual information about an error. This will
// typically be the lines surrounding some problem in a file.
type ErrorContext struct {
Expand Down
78 changes: 74 additions & 4 deletions common/herrors/file_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"io"
"path/filepath"

"github.com/bep/godartsass"
"github.com/bep/golibsass/libsass/libsasserrors"
"github.com/gohugoio/hugo/common/paths"
"github.com/gohugoio/hugo/common/text"
"github.com/pelletier/go-toml/v2"
Expand Down Expand Up @@ -132,17 +134,40 @@ func (e fileError) Position() text.Position {
}

func (e *fileError) Error() string {
return fmt.Sprintf("%s: %s", e.position, e.cause)
return fmt.Sprintf("%s: %s", e.position, e.causeString())
}

func (e *fileError) causeString() string {
if e.cause == nil {
return ""
}
switch v := e.cause.(type) {
// Avoid repeating the file info in the error message.
case godartsass.SassError:
return v.Message
case libsasserrors.Error:
return v.Message
default:
return v.Error()
}
}

func (e *fileError) Unwrap() error {
return e.cause
}

// NewFileError creates a new FileError that wraps err.
// It will try to extract the filename and line number from err.
func NewFileError(err error) FileError {
// Filetype is used to determine the Chroma lexer to use.
fileType, pos := extractFileTypePos(err)
return &fileError{cause: err, fileType: fileType, position: pos}
}

// NewFileErrorFromName creates a new FileError that wraps err.
// The value for name should identify the file, the best
// being the full filename to the file on disk.
func NewFileError(err error, name string) FileError {
func NewFileErrorFromName(err error, name string) FileError {
// Filetype is used to determine the Chroma lexer to use.
fileType, pos := extractFileTypePos(err)
pos.Filename = name
Expand All @@ -165,6 +190,23 @@ func NewFileErrorFromPos(err error, pos text.Position) FileError {

}

func NewFileErrorFromFileInErr(err error, fs afero.Fs, linematcher LineMatcherFn) FileError {
fe := NewFileError(err)
pos := fe.Position()
if pos.Filename == "" {
return fe
}

f, realFilename, err2 := openFile(pos.Filename, fs)
if err2 != nil {
return fe
}

pos.Filename = realFilename
defer f.Close()
return fe.UpdateContent(f, linematcher)
}

func NewFileErrorFromFileInPos(err error, pos text.Position, fs afero.Fs, linematcher LineMatcherFn) FileError {
if err == nil {
panic("err is nil")
Expand All @@ -185,10 +227,10 @@ func NewFileErrorFromFile(err error, filename string, fs afero.Fs, linematcher L
}
f, realFilename, err2 := openFile(filename, fs)
if err2 != nil {
return NewFileError(err, realFilename)
return NewFileErrorFromName(err, realFilename)
}
defer f.Close()
return NewFileError(err, realFilename).UpdateContent(f, linematcher)
return NewFileErrorFromName(err, realFilename).UpdateContent(f, linematcher)
}

func openFile(filename string, fs afero.Fs) (afero.File, string, error) {
Expand Down Expand Up @@ -223,8 +265,15 @@ func Cause(err error) error {

func extractFileTypePos(err error) (string, text.Position) {
err = Cause(err)

var fileType string

// LibSass, DartSass
if pos := extractPosition(err); pos.LineNumber > 0 || pos.Offset > 0 {
_, fileType = paths.FileAndExtNoDelimiter(pos.Filename)
return fileType, pos
}

// Default to line 1 col 1 if we don't find any better.
pos := text.Position{
Offset: -1,
Expand Down Expand Up @@ -259,6 +308,10 @@ func extractFileTypePos(err error) (string, text.Position) {
}
}

if fileType == "" && pos.Filename != "" {
_, fileType = paths.FileAndExtNoDelimiter(pos.Filename)
}

return fileType, pos
}

Expand Down Expand Up @@ -322,3 +375,20 @@ func exctractLineNumberAndColumnNumber(e error) (int, int) {

return -1, -1
}

func extractPosition(e error) (pos text.Position) {
switch v := e.(type) {
case godartsass.SassError:
span := v.Span
start := span.Start
filename, _ := paths.UrlToFilename(span.Url)
pos.Filename = filename
pos.Offset = start.Offset
pos.ColumnNumber = start.Column
case libsasserrors.Error:
pos.Filename = v.File
pos.LineNumber = v.Line
pos.ColumnNumber = v.Column
}
return
}
4 changes: 2 additions & 2 deletions common/herrors/file_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestNewFileError(t *testing.T) {

c := qt.New(t)

fe := NewFileError(errors.New("bar"), "foo.html")
fe := NewFileErrorFromName(errors.New("bar"), "foo.html")
c.Assert(fe.Error(), qt.Equals, `"foo.html:1:1": bar`)

lines := ""
Expand Down Expand Up @@ -70,7 +70,7 @@ func TestNewFileErrorExtractFromMessage(t *testing.T) {
{errors.New(`execute of template failed: template: index.html:2:5: executing "index.html" at <partial "foo.html" .>: error calling partial: "/layouts/partials/foo.html:3:6": execute of template failed: template: partials/foo.html:3:6: executing "partials/foo.html" at <.ThisDoesNotExist>: can't evaluate field ThisDoesNotExist in type *hugolib.pageStat`), 0, 2, 5},
} {

got := NewFileError(test.in, "test.txt")
got := NewFileErrorFromName(test.in, "test.txt")

errMsg := qt.Commentf("[%d][%T]", i, got)

Expand Down
27 changes: 27 additions & 0 deletions common/paths/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"fmt"
"net/url"
"path"
"path/filepath"
"strings"
)

Expand Down Expand Up @@ -152,3 +153,29 @@ func Uglify(in string) string {
// /section/name.html -> /section/name.html
return path.Clean(in)
}

// UrlToFilename converts the URL s to a filename.
// If ParseRequestURI fails, the input is just converted to OS specific slashes and returned.
func UrlToFilename(s string) (string, bool) {
u, err := url.ParseRequestURI(s)

if err != nil {
return filepath.FromSlash(s), false
}

p := u.Path

if p == "" {
p, _ = url.QueryUnescape(u.Opaque)
return filepath.FromSlash(p), true
}

p = filepath.FromSlash(p)

if u.Host != "" {
// C:\data\file.txt
p = strings.ToUpper(u.Host) + ":" + p
}

return p, true
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/bep/gitmap v1.1.2
github.com/bep/goat v0.5.0
github.com/bep/godartsass v0.14.0
github.com/bep/golibsass v1.0.0
github.com/bep/golibsass v1.1.0
github.com/bep/gowebp v0.1.0
github.com/bep/overlayfs v0.6.0
github.com/bep/tmc v0.5.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ github.com/bep/godartsass v0.14.0 h1:pPb6XkpyDEppS+wK0veh7OXDQc4xzOJI9Qcjb743UeQ
github.com/bep/godartsass v0.14.0/go.mod h1:6LvK9RftsXMxGfsA0LDV12AGc4Jylnu6NgHL+Q5/pE8=
github.com/bep/golibsass v1.0.0 h1:gNguBMSDi5yZEZzVZP70YpuFQE3qogJIGUlrVILTmOw=
github.com/bep/golibsass v1.0.0/go.mod h1:DL87K8Un/+pWUS75ggYv41bliGiolxzDKWJAq3eJ1MA=
github.com/bep/golibsass v1.1.0 h1:pjtXr00IJZZaOdfryNa9wARTB3Q0BmxC3/V1KNcgyTw=
github.com/bep/golibsass v1.1.0/go.mod h1:DL87K8Un/+pWUS75ggYv41bliGiolxzDKWJAq3eJ1MA=
github.com/bep/gowebp v0.1.0 h1:4/iQpfnxHyXs3x/aTxMMdOpLEQQhFmF6G7EieWPTQyo=
github.com/bep/gowebp v0.1.0/go.mod h1:ZhFodwdiFp8ehGJpF4LdPl6unxZm9lLFjxD3z2h2AgI=
github.com/bep/overlayfs v0.6.0 h1:sgLcq/qtIzbaQNl2TldGXOkHvqeZB025sPvHOQL+DYo=
Expand Down
3 changes: 2 additions & 1 deletion hugolib/integrationtest_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,9 @@ func (s *IntegrationTestBuilder) destinationExists(filename string) bool {
return b
}

func (s *IntegrationTestBuilder) AssertIsFileError(err error) {
func (s *IntegrationTestBuilder) AssertIsFileError(err error) herrors.FileError {
s.Assert(err, qt.ErrorAs, new(herrors.FileError))
return herrors.UnwrapFileError(err)
}

func (s *IntegrationTestBuilder) AssertRenderCountContent(count int) {
Expand Down
2 changes: 1 addition & 1 deletion hugolib/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(err, p.File().Filename()).UpdatePosition(pos)
return herrors.NewFileErrorFromName(err, p.File().Filename()).UpdatePosition(pos)
}

func (p *pageState) pathOrTitle() string {
Expand Down
4 changes: 2 additions & 2 deletions hugolib/shortcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func renderShortcode(
var err error
tmpl, err = s.TextTmpl().Parse(templName, templStr)
if err != nil {
fe := herrors.NewFileError(err, p.File().Filename())
fe := herrors.NewFileErrorFromName(err, p.File().Filename())
pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos)
Expand Down Expand Up @@ -391,7 +391,7 @@ func renderShortcode(
result, err := renderShortcodeWithPage(s.Tmpl(), tmpl, data)

if err != nil && sc.isInline {
fe := herrors.NewFileError(err, p.File().Filename())
fe := herrors.NewFileErrorFromName(err, p.File().Filename())
pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos)
Expand Down
2 changes: 1 addition & 1 deletion langs/i18n/translationProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,6 @@ func errWithFileContext(inerr error, r source.File) error {
}
defer f.Close()

return herrors.NewFileError(inerr, realFilename).UpdateContent(f, nil)
return herrors.NewFileErrorFromName(inerr, realFilename).UpdateContent(f, nil)

}
2 changes: 1 addition & 1 deletion parser/metadecoders/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (d Decoder) unmarshalORG(data []byte, v any) error {
}

func toFileError(f Format, data []byte, err error) error {
return herrors.NewFileError(err, fmt.Sprintf("_stream.%s", f)).UpdateContent(bytes.NewReader(data), nil)
return herrors.NewFileErrorFromName(err, fmt.Sprintf("_stream.%s", f)).UpdateContent(bytes.NewReader(data), nil)
}

// stringifyMapKeys recurses into in and changes all instances of
Expand Down
2 changes: 1 addition & 1 deletion resources/resource_transformers/js/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (t *buildTransformation) Transform(ctx *resources.ResourceTransformationCtx

if err == nil {
fe := herrors.
NewFileError(errors.New(errorMessage), path).
NewFileErrorFromName(errors.New(errorMessage), path).
UpdatePosition(text.Position{Offset: -1, LineNumber: loc.Line, ColumnNumber: loc.Column}).
UpdateContent(f, nil)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,6 @@ func TestTransformPostCSSImporSkipInlineImportsNotFound(t *testing.T) {
TxtarString: files,
}).Build()

s.AssertFileContent("public/css/styles.css", filepath.FromSlash(`@import "components/doesnotexist.css";`))
s.AssertFileContent("public/css/styles.css", `@import "components/doesnotexist.css";`)

}
4 changes: 2 additions & 2 deletions resources/resource_transformers/postcss/postcss.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func (imp *importResolver) importRecursive(
LineNumber: offset + 1,
ColumnNumber: column + 1,
}
return 0, "", herrors.NewFileErrorFromFileInPos(fmt.Errorf("failed to resolve CSS @import %q", filename), pos, imp.fs, nil)
return 0, "", herrors.NewFileErrorFromFileInPos(fmt.Errorf("failed to resolve CSS @import \"%s\"", filename), pos, imp.fs, nil)
}

i--
Expand Down Expand Up @@ -421,7 +421,7 @@ func (imp *importResolver) toFileError(output string) error {
}
defer f.Close()

ferr := herrors.NewFileError(inErr, realFilename)
ferr := herrors.NewFileErrorFromName(inErr, realFilename)
pos := ferr.Position()
pos.LineNumber = file.Offset + 1
return ferr.UpdatePosition(pos).UpdateContent(f, nil)
Expand Down
10 changes: 8 additions & 2 deletions resources/resource_transformers/tocss/dartsass/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"io"
"strings"

"github.com/gohugoio/hugo/common/herrors"
"github.com/gohugoio/hugo/helpers"
"github.com/gohugoio/hugo/hugofs"
"github.com/gohugoio/hugo/hugolib/filesystems"
"github.com/gohugoio/hugo/resources"
"github.com/gohugoio/hugo/resources/resource"
Expand All @@ -33,6 +35,10 @@ import (
// used as part of the cache key.
const transformationName = "tocss-dart"

// See https://github.com/sass/dart-sass-embedded/issues/24
// Note: This prefix must be all lower case.
const dartSassStdinPrefix = "hugostdin:"

func New(fs *filesystems.SourceFilesystem, rs *resources.Spec) (*Client, error) {
if !Supports() {
return &Client{dartSassNotAvailable: true}, nil
Expand All @@ -44,7 +50,7 @@ func New(fs *filesystems.SourceFilesystem, rs *resources.Spec) (*Client, error)

transpiler, err := godartsass.Start(godartsass.Options{
LogEventHandler: func(event godartsass.LogEvent) {
message := strings.ReplaceAll(event.Message, stdinPrefix, "")
message := strings.ReplaceAll(event.Message, dartSassStdinPrefix, "")
switch event.Type {
case godartsass.LogEventTypeDebug:
// Log as Info for now, we may adjust this if it gets too chatty.
Expand Down Expand Up @@ -94,7 +100,7 @@ func (c *Client) toCSS(args godartsass.Args, src io.Reader) (godartsass.Result,
if err.Error() == "unexpected EOF" {
return res, fmt.Errorf("got unexpected EOF when executing %q. The user running hugo must have read and execute permissions on this program. With execute permissions only, this error is thrown.", dartSassEmbeddedBinaryName)
}
return res, err
return res, herrors.NewFileErrorFromFileInErr(err, hugofs.Os, herrors.OffsetMatcher)
}

return res, err
Expand Down
Loading

0 comments on commit 522e44b

Please sign in to comment.