Skip to content

Commit

Permalink
refactor(parser): improve error handling and reporting
Browse files Browse the repository at this point in the history 
make sure that all subsequent steps of the pipeline are bypassed
when a DocumentFragment carries and error. Also, include the
`start` and `end` position (as offsets from the beginning of the doc)
for each fragment (with and without errors)

fixes bytesparadise#879

Signed-off-by: Xavier Coulon <[email protected]>
  • Loading branch information
@xcoulon
xcoulon committed Dec 6, 2021
1 parent c23737c commit e931e05
Show file tree
Hide file tree
Showing 27 changed files with 14,354 additions and 14,044 deletions.
59 changes: 59 additions & 0 deletions pkg/parser/attributes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -40,6 +44,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 17,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -58,6 +66,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ , , ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 21,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -76,6 +88,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ , 200, ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 24,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -97,6 +113,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 30,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -118,6 +138,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["The Foo\"Bar\" here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 37,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -139,6 +163,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png['The Foo\'Bar\' here']`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 37,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -160,6 +188,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["The Foo\Bar here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 34,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -181,6 +213,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png['The Foo\Bar here']`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 34,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -202,6 +238,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", height=100]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 42,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -226,6 +266,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", 1, 2, height=100]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 48,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -249,6 +293,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", 1, 2, height=100, test1=123 ,test2 = second test ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 81,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -274,6 +322,10 @@ var _ = Describe("attributes", func() {
source := "image::foo.png[\"Quoted, Here\", 1, 2, height=100, test1=123 ,test2 = second \"test\" ]"
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 83,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand Down Expand Up @@ -305,6 +357,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ "This \Backslash 2Spaced End Space " ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 55,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand Down Expand Up @@ -689,6 +745,9 @@ var _ = DescribeTable("invalid block attributes",

func(source string) {
// given
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
content := strings.NewReader(source + "\n")
// when parsing only (ie, no substitution applied)
_, err := parser.ParseReader("", content, parser.Entrypoint("BlockAttributes"))
Expand Down
39 changes: 36 additions & 3 deletions pkg/parser/blank_line_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ var _ = Describe("blank lines", func() {
second paragraph`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -27,11 +31,19 @@ second paragraph`
},
},
{
Position: types.Position{
Start: 16,
End: 18,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 18,
End: 34,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -53,6 +65,10 @@ second paragraph
`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -62,21 +78,37 @@ second paragraph
},
},
{
Position: types.Position{
Start: 16,
End: 20,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 20,
End: 21,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 21,
End: 24,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 24,
End: 41,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -95,10 +127,11 @@ second paragraph
`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 11,
},
Elements: []interface{}{
// types.Attributes{
// types.AttrTitle: "ignored",
// },
&types.BlankLine{},
},
},
Expand Down
4 changes: 4 additions & 0 deletions pkg/parser/delimited_block_example_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -51,6 +52,9 @@ some *example* content
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `====
.foo
====`
Expand Down
15 changes: 13 additions & 2 deletions pkg/parser/delimited_block_listing_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -1161,9 +1162,19 @@ and <more text> on the +
})

It("should fail when substitution is unknown", func() {
logs, reset := ConfigureLogger(log.DebugLevel) //, DiscardStdout)
defer reset()
s := strings.ReplaceAll(source, "$SUBS", "unknown")
_, err := ParseDocument(s)
Expect(err).To(HaveOccurred())
expected := &types.Document{
Elements: []interface{}{
&types.AttributeDeclaration{
Name: "github-url",
Value: string("https://github.com"),
},
},
}
Expect(ParseDocument(s)).To(MatchDocument(expected))
Expect(logs).To(ContainJSONLog(log.ErrorLevel, 33, 183, "unsupported kind of substitution: 'unknown'"))
})
})
})
Expand Down
12 changes: 12 additions & 0 deletions pkg/parser/delimited_block_passthrough_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ _foo_
another paragraph`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 19,
},
Elements: []interface{}{
&types.Paragraph{
Attributes: types.Attributes{
Expand All @@ -35,11 +39,19 @@ another paragraph`
},
},
{
Position: types.Position{
Start: 19,
End: 20,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 20,
End: 37,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand Down
4 changes: 4 additions & 0 deletions pkg/parser/delimited_block_quote_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser_test
import (
"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -154,6 +155,9 @@ ____
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `[quote]
____
.foo
Expand Down
7 changes: 7 additions & 0 deletions pkg/parser/delimited_block_sidebar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser_test
import (
"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -98,6 +99,9 @@ bar
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `
****
.foo
Expand Down Expand Up @@ -198,6 +202,9 @@ bar
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `
****
.foo
Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/document_processing_aggregate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func aggregate(ctx *ParseContext, fragmentStream <-chan types.DocumentFragment)
var toc *types.TableOfContents
for f := range fragmentStream {
if f.Error != nil {
log.Debugf("skipping aggregation because of fragment with error: %v", f.Error)
return nil, nil, f.Error
log.WithField("start_offset", f.Position.Start).WithField("end_offset", f.Position.End).Error(f.Error)
continue
}
for _, element := range f.Elements {
switch e := element.(type) {
Expand Down
Loading

0 comments on commit e931e05

Please sign in to comment.