mirror/gohugoio-hugo
1
0
Fork 0
mirror of https://github.com/gohugoio/hugo.git synced 2024-05-11 05:54:58 +00:00
Code Issues Releases Activity

herrors: Improve handling of JSON errors

Browse Source 
`*json.UnmarshalTypeError` and `*json.SyntaxError` has a byte `Offset`, so use that.

This commit also reworks/simplifies the errror line matching logic. This also makes the file reading unbuffered, but that should be fine in this error case.

See #5324
This commit is contained in:
Bjørn Erik Pedersen
2018-10-23 08:54:10 +02:00
parent ed7b3e2619
commit f669ef6bec
10 changed files with 228 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
common/herrors/error_locator.go
View File

@@ -15,9 +15,9 @@
package herrors
import (
"bufio"
"fmt"
"io"
"io/ioutil"
"strings"
"github.com/gohugoio/hugo/helpers"
@@ -27,13 +27,20 @@ import (
var fileErrorFormat = "\"%s:%d:%d\": %s"
// LineMatcher is used to match a line with an error.
type LineMatcher func(le FileError, lineNumber int, line string) bool
// LineMatcher contains the elements used to match an error to a line
type LineMatcher struct {
FileError FileError
LineNumber int
Offset int
Line string
}
// SimpleLineMatcher matches if the current line number matches the line number
// in the error.
var SimpleLineMatcher = func(le FileError, lineNumber int, line string) bool {
return le.LineNumber() == lineNumber
// LineMatcherFn is used to match a line with an error.
type LineMatcherFn func(m LineMatcher) bool
// SimpleLineMatcher simply matches by line number.
var SimpleLineMatcher = func(m LineMatcher) bool {
return m.FileError.LineNumber() == m.LineNumber
}
// ErrorContext contains contextual information about an error. This will
@@ -79,7 +86,7 @@ func (e *ErrorWithFileContext) Cause() error {
// WithFileContextForFile will try to add a file context with lines matching the given matcher.
// If no match could be found, the original error is returned with false as the second return value.
func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs, matcher LineMatcher) (error, bool) {
func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs, matcher LineMatcherFn) (error, bool) {
f, err := fs.Open(filename)
if err != nil {
return e, false
@@ -90,11 +97,12 @@ func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs,
// WithFileContextForFile will try to add a file context with lines matching the given matcher.
// If no match could be found, the original error is returned with false as the second return value.
func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatcher) (error, bool) {
func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatcherFn) (error, bool) {
if e == nil {
panic("error missing")
}
le := UnwrapFileError(e)
if le == nil {
var ok bool
if le, ok = ToFileError("", e).(FileError); !ok {
@@ -102,13 +110,27 @@ func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatc
}
}
errCtx := locateError(r, le, matcher)
errCtx.Filename = realFilename
var errCtx ErrorContext
if le.Offset() != -1 {
errCtx = locateError(r, le, func(m LineMatcher) bool {
if le.Offset() >= m.Offset && le.Offset() < m.Offset+len(m.Line) {
fe := m.FileError
m.FileError = ToFileErrorWithOffset(fe, -fe.LineNumber()+m.LineNumber)
}
return matcher(m)
})
} else {
errCtx = locateError(r, le, matcher)
}
if errCtx.LineNumber == -1 {
return e, false
}
errCtx.Filename = realFilename
if le.Type() != "" {
errCtx.ChromaLexer = chromaLexerFromType(le.Type())
} else {
@@ -151,72 +173,66 @@ func chromaLexerFromFilename(filename string) string {
return chromaLexerFromType(ext)
}
func locateErrorInString(le FileError, src string, matcher LineMatcher) ErrorContext {
return locateError(strings.NewReader(src), nil, matcher)
func locateErrorInString(src string, matcher LineMatcherFn) ErrorContext {
return locateError(strings.NewReader(src), &fileError{}, matcher)
}
func locateError(r io.Reader, le FileError, matches LineMatcher) ErrorContext {
var errCtx ErrorContext
s := bufio.NewScanner(r)
func locateError(r io.Reader, le FileError, matches LineMatcherFn) ErrorContext {
if le == nil {
panic("must provide an error")
}
errCtx.ColumnNumber = 1
if le != nil {
errCtx := ErrorContext{LineNumber: -1, ColumnNumber: 1, Pos: -1}
b, err := ioutil.ReadAll(r)
if err != nil {
return errCtx
}
lines := strings.Split(string(b), "\n")
if le != nil && le.ColumnNumber() >= 0 {
errCtx.ColumnNumber = le.ColumnNumber()
}
lineNo := 0
posBytes := 0
var buff [6]string
i := 0
errCtx.Pos = -1
for s.Scan() {
lineNo++
txt := s.Text()
buff[i] = txt
if errCtx.Pos != -1 && i >= 5 {
for li, line := range lines {
lineNo = li + 1
m := LineMatcher{
FileError: le,
LineNumber: lineNo,
Offset: posBytes,
Line: line,
}
if errCtx.Pos == -1 && matches(m) {
errCtx.LineNumber = lineNo
break
}
if errCtx.Pos == -1 && matches(le, lineNo, txt) {
errCtx.Pos = i
errCtx.LineNumber = lineNo
}
if errCtx.Pos == -1 && i == 2 {
// Shift left
buff[0], buff[1] = buff[i-1], buff[i]
} else {
i++
}
posBytes += len(line)
}
// Go's template parser will typically report "unexpected EOF" errors on the
// empty last line that is supressed by the scanner.
// Do an explicit check for that.
if errCtx.Pos == -1 {
lineNo++
if matches(le, lineNo, "") {
buff[i] = ""
errCtx.Pos = i
errCtx.LineNumber = lineNo
i++
}
}
if errCtx.Pos != -1 {
low := errCtx.Pos - 2
if errCtx.LineNumber != -1 {
low := errCtx.LineNumber - 3
if low < 0 {
low = 0
}
high := i
errCtx.Lines = buff[low:high]
} else {
errCtx.Pos = -1
errCtx.LineNumber = -1
if errCtx.LineNumber > 2 {
errCtx.Pos = 2
} else {
errCtx.Pos = errCtx.LineNumber - 1
}
high := errCtx.LineNumber + 2
if high > len(lines) {
high = len(lines)
}
errCtx.Lines = lines[low:high]
}
return errCtx

54
common/herrors/error_locator_test.go
View File

@@ -24,8 +24,8 @@ import (
func TestErrorLocator(t *testing.T) {
assert := require.New(t)
lineMatcher := func(le FileError, lineno int, line string) bool {
return strings.Contains(line, "THEONE")
lineMatcher := func(m LineMatcher) bool {
return strings.Contains(m.Line, "THEONE")
}
lines := `LINE 1
@@ -38,49 +38,51 @@ LINE 7
LINE 8
`
location := locateErrorInString(nil, lines, lineMatcher)
location := locateErrorInString(lines, lineMatcher)
assert.Equal([]string{"LINE 3", "LINE 4", "This is THEONE", "LINE 6", "LINE 7"}, location.Lines)
assert.Equal(5, location.LineNumber)
assert.Equal(2, location.Pos)
assert.Equal([]string{"This is THEONE"}, locateErrorInString(nil, `This is THEONE`, lineMatcher).Lines)
assert.Equal([]string{"This is THEONE"}, locateErrorInString(`This is THEONE`, lineMatcher).Lines)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
This is THEONE
L2
`, lineMatcher)
assert.Equal(2, location.LineNumber)
assert.Equal(1, location.Pos)
assert.Equal([]string{"L1", "This is THEONE", "L2"}, location.Lines)
assert.Equal([]string{"L1", "This is THEONE", "L2", ""}, location.Lines)
location = locateErrorInString(nil, `This is THEONE
location = locateErrorInString(`This is THEONE
L2
`, lineMatcher)
assert.Equal(0, location.Pos)
assert.Equal([]string{"This is THEONE", "L2"}, location.Lines)
assert.Equal([]string{"This is THEONE", "L2", ""}, location.Lines)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
This THEONE
`, lineMatcher)
assert.Equal([]string{"L1", "This THEONE"}, location.Lines)
assert.Equal([]string{"L1", "This THEONE", ""}, location.Lines)
assert.Equal(1, location.Pos)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
L2
This THEONE
`, lineMatcher)
assert.Equal([]string{"L1", "L2", "This THEONE"}, location.Lines)
assert.Equal([]string{"L1", "L2", "This THEONE", ""}, location.Lines)
assert.Equal(2, location.Pos)
location = locateErrorInString(nil, "NO MATCH", lineMatcher)
location = locateErrorInString("NO MATCH", lineMatcher)
assert.Equal(-1, location.LineNumber)
assert.Equal(-1, location.Pos)
assert.Equal(0, len(location.Lines))
lineMatcher = func(le FileError, lineno int, line string) bool {
return lineno == 6
lineMatcher = func(m LineMatcher) bool {
return m.LineNumber == 6
}
location = locateErrorInString(nil, `A
location = locateErrorInString(`A
B
C
D
@@ -96,11 +98,11 @@ J`, lineMatcher)
assert.Equal(2, location.Pos)
// Test match EOF
lineMatcher = func(le FileError, lineno int, line string) bool {
return lineno == 4
lineMatcher = func(m LineMatcher) bool {
return m.LineNumber == 4
}
location = locateErrorInString(nil, `A
location = locateErrorInString(`A
B
C
`, lineMatcher)
@@ -109,4 +111,18 @@ C
assert.Equal(4, location.LineNumber)
assert.Equal(2, location.Pos)
offsetMatcher := func(m LineMatcher) bool {
return m.Offset == 1
}
location = locateErrorInString(`A
B
C
D
E`, offsetMatcher)
assert.Equal([]string{"A", "B", "C", "D"}, location.Lines)
assert.Equal(2, location.LineNumber)
assert.Equal(1, location.Pos)
}

70
common/herrors/file_error.go
View File

@@ -13,6 +13,12 @@
package herrors
import (
"encoding/json"
"github.com/pkg/errors"
)
var _ causer = (*fileError)(nil)
// FileError represents an error when handling a file: Parsing a config file,
@@ -20,9 +26,14 @@ var _ causer = (*fileError)(nil)
type FileError interface {
error
// Offset gets the error location offset in bytes, starting at 0.
// It will return -1 if not provided.
Offset() int
// LineNumber gets the error location, starting at line 1.
LineNumber() int
// Column number gets the column location, starting at 1.
ColumnNumber() int
// A string identifying the type of file, e.g. JSON, TOML, markdown etc.
@@ -32,6 +43,7 @@ type FileError interface {
var _ FileError = (*fileError)(nil)
type fileError struct {
offset int
lineNumber int
columnNumber int
fileType string
@@ -39,10 +51,23 @@ type fileError struct {
cause error
}
type fileErrorWithLineOffset struct {
FileError
offset int
}
func (e *fileErrorWithLineOffset) LineNumber() int {
return e.FileError.LineNumber() + e.offset
}
func (e *fileError) LineNumber() int {
return e.lineNumber
}
func (e *fileError) Offset() int {
return e.offset
}
func (e *fileError) ColumnNumber() int {
return e.columnNumber
}
@@ -63,8 +88,8 @@ func (f *fileError) Cause() error {
}
// NewFileError creates a new FileError.
func NewFileError(fileType string, lineNumber, columnNumber int, err error) FileError {
return &fileError{cause: err, fileType: fileType, lineNumber: lineNumber, columnNumber: columnNumber}
func NewFileError(fileType string, offset, lineNumber, columnNumber int, err error) FileError {
return &fileError{cause: err, fileType: fileType, offset: offset, lineNumber: lineNumber, columnNumber: columnNumber}
}
// UnwrapFileError tries to unwrap a FileError from err.
@@ -83,24 +108,37 @@ func UnwrapFileError(err error) FileError {
return nil
}
// ToFileError will try to convert the given error to an error supporting
// the FileError interface.
// If will fall back to returning the original error if a line number cannot be extracted.
func ToFileError(fileType string, err error) error {
return ToFileErrorWithOffset(fileType, err, 0)
// ToFileErrorWithOffset will return a new FileError with a line number
// with the given offset from the original.
func ToFileErrorWithOffset(fe FileError, offset int) FileError {
return &fileErrorWithLineOffset{FileError: fe, offset: offset}
}
// ToFileErrorWithOffset will try to convert the given error to an error supporting
// the FileError interface. It will take any line number offset given into account.
// If will fall back to returning the original error if a line number cannot be extracted.
func ToFileErrorWithOffset(fileType string, err error, offset int) error {
// ToFileError will convert the given error to an error supporting
// the FileError interface.
func ToFileError(fileType string, err error) FileError {
for _, handle := range lineNumberExtractors {
lno, col := handle(err)
if lno > 0 {
return NewFileError(fileType, lno+offset, col, err)
offset, typ := extractOffsetAndType(err)
if fileType == "" {
fileType = typ
}
if lno > 0 || offset != -1 {
return NewFileError(fileType, offset, lno, col, err)
}
}
// Fall back to the original.
return err
// Fall back to the pointing to line number 1.
return NewFileError(fileType, -1, 1, 1, err)
}
func extractOffsetAndType(e error) (int, string) {
e = errors.Cause(e)
switch v := e.(type) {
case *json.UnmarshalTypeError:
return int(v.Offset), "json"
case *json.SyntaxError:
return int(v.Offset), "json"
default:
return -1, ""
}
}

25
common/herrors/file_error_test.go
View File

@@ -14,11 +14,11 @@
package herrors
import (
"errors"
"fmt"
"strconv"
"testing"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)
@@ -33,7 +33,7 @@ func TestToLineNumberError(t *testing.T) {
lineNumber int
columnNumber int
}{
{errors.New("no line number for you"), 0, -1, 1},
{errors.New("no line number for you"), 0, 1, 1},
{errors.New(`template: _default/single.html:4:15: executing "_default/single.html" at <.Titles>: can't evaluate field Titles in type *hugolib.PageOutput`), 0, 4, 15},
{errors.New("parse failed: template: _default/bundle-resource-meta.html:11: unexpected in operand"), 0, 11, 1},
{errors.New(`failed:: template: _default/bundle-resource-meta.html:2:7: executing "main" at <.Titles>`), 0, 2, 7},
@@ -41,18 +41,19 @@ func TestToLineNumberError(t *testing.T) {
{errors.New(`failed to load translations: (6, 7): was expecting token =, but got "g" instead`), 0, 6, 7},
} {
got := ToFileErrorWithOffset("template", test.in, test.offset)
got := ToFileError("template", test.in)
if test.offset > 0 {
got = ToFileErrorWithOffset(got.(FileError), test.offset)
}
errMsg := fmt.Sprintf("[%d][%T]", i, got)
le, ok := got.(FileError)
assert.True(ok)
if test.lineNumber > 0 {
assert.True(ok, errMsg)
assert.Equal(test.lineNumber, le.LineNumber(), errMsg)
assert.Equal(test.columnNumber, le.ColumnNumber(), errMsg)
assert.Contains(got.Error(), strconv.Itoa(le.LineNumber()))
} else {
assert.False(ok)
}
assert.True(ok, errMsg)
assert.Equal(test.lineNumber, le.LineNumber(), errMsg)
assert.Equal(test.columnNumber, le.ColumnNumber(), errMsg)
assert.Error(errors.Cause(got))
}
}