Skip to content

Commit a2faa67

Browse files
eatmoreappleeatmoreapple
authored
feat: enhance XML parsing error with namespace and XML content context (#549)
* feat: enhance XML parsing error with namespace and XML content context - Add XMLParseError type with Namespace and XMLContent fields - Wrap parsing errors with detailed context information - Track current namespace during mapper parsing - Build readable XML element representation for error messages - Cover all XML parsing scenarios (select, if, foreach, when, bind, etc.) - Add comprehensive unit and integration tests Error message format: XML parse error in namespace '<namespace>': <xml_content>: <error_message> This improves debugging experience by showing exactly which XML element caused the error and in which mapper namespace it occurred. * wip
1 parent 74fbc24 commit a2faa67

File tree

3 files changed

+401
-35
lines changed

3 files changed

+401
-35
lines changed

‎errors.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package juice
1919
import (
2020
"errors"
2121
"fmt"
22+
"strings"
2223

2324
"github.com/go-juicedev/juice/sql"
2425
)
@@ -73,6 +74,41 @@ func (e *nodeAttributeConflictError) Error() string {
7374
return fmt.Sprintf("node %s has conflicting attribute %s", e.nodeName, e.attrName)
7475
}
7576

77+
// XMLParseError represents an error occurred during XML parsing with detailed context.
78+
type XMLParseError struct {
79+
// Namespace is the namespace of the mapper being parsed
80+
Namespace string
81+
// XMLContent is the XML element content that caused the error
82+
XMLContent string
83+
// Err is the underlying error
84+
Err error
85+
}
86+
87+
// Error returns the error message.
88+
func (e *XMLParseError) Error() string {
89+
var builder strings.Builder
90+
builder.WriteString("XML parse error")
91+
if e.Namespace != "" {
92+
builder.WriteString(" in namespace '")
93+
builder.WriteString(e.Namespace)
94+
builder.WriteString("'")
95+
}
96+
if e.XMLContent != "" {
97+
builder.WriteString(": ")
98+
builder.WriteString(e.XMLContent)
99+
}
100+
if e.Err != nil {
101+
builder.WriteString(": ")
102+
builder.WriteString(e.Err.Error())
103+
}
104+
return builder.String()
105+
}
106+
107+
// Unwrap returns the underlying error.
108+
func (e *XMLParseError) Unwrap() error {
109+
return e.Err
110+
}
111+
76112
// unreachable is a function that is used to mark unreachable code.
77113
// nolint:deadcode,unused
78114
func unreachable() error {

‎parser.go‎

Lines changed: 76 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,8 @@ func (p *XMLSettingsElementParser) parseSettings(decoder *xml.Decoder) (keyValue
284284
}
285285

286286
type XMLMappersElementParser struct {
287-
parser *XMLParser
287+
parser *XMLParser
288+
currentNamespace string // tracks the current mapper namespace being parsed
288289
}
289290

290291
func (p *XMLMappersElementParser) MatchElement(token xml.StartElement) bool {
@@ -384,6 +385,9 @@ func (p *XMLMappersElementParser) parseMapper(decoder *xml.Decoder, token xml.St
384385
return nil, &nodeAttributeRequiredError{nodeName: "mapper", attrName: "namespace"}
385386
}
386387

388+
// Set current namespace for error reporting
389+
p.currentNamespace = namespace
390+
387391
mapper.namespace = namespace
388392
mapper.statements = make(map[string]*xmlSQLStatement)
389393

@@ -542,7 +546,7 @@ func (p *XMLMappersElementParser) parseStatement(stmt *xmlSQLStatement, decoder
542546
stmt.setAttribute(attr.Name.Local, attr.Value)
543547
}
544548
if stmt.id = stmt.Attribute("id"); stmt.id == "" {
545-
return fmt.Errorf("%s xmlSQLStatement id is required", stmt.Action())
549+
return p.wrapParseError(decoder, token, fmt.Errorf("%s xmlSQLStatement id is required", stmt.Action()))
546550
}
547551

548552
for {
@@ -556,14 +560,14 @@ func (p *XMLMappersElementParser) parseStatement(stmt *xmlSQLStatement, decoder
556560
switch token := token.(type) {
557561
case xml.StartElement:
558562
if handled, err := p.parseBindAndAppend(decoder, token, &stmt.bindNodes); err != nil {
559-
return err
563+
return p.wrapParseError(decoder, token, err)
560564
} else if handled {
561565
continue
562566
}
563567

564568
n, err := p.parseTags(stmt.mapper, decoder, token)
565569
if err != nil {
566-
return err
570+
return p.wrapParseError(decoder, token, err)
567571
}
568572
stmt.Nodes = append(stmt.Nodes, n)
569573
case xml.CharData:
@@ -608,7 +612,7 @@ func (p *XMLMappersElementParser) parseTags(mapper *Mapper, decoder *xml.Decoder
608612
func (p *XMLMappersElementParser) parseInclude(mapper *Mapper, decoder *xml.Decoder, token xml.StartElement) (node.Node, error) {
609613
ref := getXMLAttr("refid", token.Attr)
610614
if ref == "" {
611-
return nil, &nodeAttributeRequiredError{nodeName: "include", attrName: "refid"}
615+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "include", attrName: "refid"})
612616
}
613617

614618
// try to find sql xmlSQLStatement by refid
@@ -653,14 +657,14 @@ func (p *XMLMappersElementParser) parseSet(mapper *Mapper, decoder *xml.Decoder)
653657
switch token := token.(type) {
654658
case xml.StartElement:
655659
if handled, err := p.parseBindAndAppend(decoder, token, &setNode.BindNodes); err != nil {
656-
return nil, err
660+
return nil, p.wrapParseError(decoder, token, err)
657661
} else if handled {
658662
continue
659663
}
660664

661665
n, err := p.parseTags(mapper, decoder, token)
662666
if err != nil {
663-
return nil, err
667+
return nil, p.wrapParseError(decoder, token, err)
664668
}
665669
setNode.Nodes = append(setNode.Nodes, n)
666670
case xml.CharData:
@@ -682,12 +686,12 @@ func (p *XMLMappersElementParser) parseIf(mapper *Mapper, decoder *xml.Decoder,
682686

683687
test := getXMLAttr("test", token.Attr)
684688
if test == "" {
685-
return nil, &nodeAttributeRequiredError{nodeName: "if", attrName: "test"}
689+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "if", attrName: "test"})
686690
}
687691

688692
// parse condition expression
689693
if err := ifNode.Parse(test); err != nil {
690-
return nil, err
694+
return nil, p.wrapParseError(decoder, token, err)
691695
}
692696
for {
693697
token, err := decoder.Token()
@@ -700,13 +704,13 @@ func (p *XMLMappersElementParser) parseIf(mapper *Mapper, decoder *xml.Decoder,
700704
switch token := token.(type) {
701705
case xml.StartElement:
702706
if handled, err := p.parseBindAndAppend(decoder, token, &ifNode.BindNodes); err != nil {
703-
return nil, err
707+
return nil, p.wrapParseError(decoder, token, err)
704708
} else if handled {
705709
continue
706710
}
707711
n, err := p.parseTags(mapper, decoder, token)
708712
if err != nil {
709-
return nil, err
713+
return nil, p.wrapParseError(decoder, token, err)
710714
}
711715
ifNode.Nodes = append(ifNode.Nodes, n)
712716
case xml.CharData:
@@ -736,13 +740,13 @@ func (p *XMLMappersElementParser) parseWhere(mapper *Mapper, decoder *xml.Decode
736740
switch token := token.(type) {
737741
case xml.StartElement:
738742
if handled, err := p.parseBindAndAppend(decoder, token, &whereNode.BindNodes); err != nil {
739-
return nil, err
743+
return nil, p.wrapParseError(decoder, token, err)
740744
} else if handled {
741745
continue
742746
}
743747
n, err := p.parseTags(mapper, decoder, token)
744748
if err != nil {
745-
return nil, err
749+
return nil, p.wrapParseError(decoder, token, err)
746750
}
747751
whereNode.Nodes = append(whereNode.Nodes, n)
748752
case xml.CharData:
@@ -792,13 +796,13 @@ func (p *XMLMappersElementParser) parseTrim(mapper *Mapper, decoder *xml.Decoder
792796
switch token := token.(type) {
793797
case xml.StartElement:
794798
if handled, err := p.parseBindAndAppend(decoder, token, &trimNode.BindNodes); err != nil {
795-
return nil, err
799+
return nil, p.wrapParseError(decoder, token, err)
796800
} else if handled {
797801
continue
798802
}
799803
n, err := p.parseTags(mapper, decoder, token)
800804
if err != nil {
801-
return nil, err
805+
return nil, p.wrapParseError(decoder, token, err)
802806
}
803807
trimNode.Nodes = append(trimNode.Nodes, n)
804808
case xml.EndElement:
@@ -834,7 +838,7 @@ func (p *XMLMappersElementParser) parseForeach(mapper *Mapper, decoder *xml.Deco
834838
foreachNode.Collection = eval.DefaultParamKey()
835839
}
836840
if foreachNode.Item == "" {
837-
return nil, &nodeAttributeRequiredError{nodeName: "foreach", attrName: "item"}
841+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "foreach", attrName: "item"})
838842
}
839843
for {
840844
token, err := decoder.Token()
@@ -847,13 +851,13 @@ func (p *XMLMappersElementParser) parseForeach(mapper *Mapper, decoder *xml.Deco
847851
switch token := token.(type) {
848852
case xml.StartElement:
849853
if handled, err := p.parseBindAndAppend(decoder, token, &foreachNode.BindNodes); err != nil {
850-
return nil, err
854+
return nil, p.wrapParseError(decoder, token, err)
851855
} else if handled {
852856
continue
853857
}
854858
n, err := p.parseTags(mapper, decoder, token)
855859
if err != nil {
856-
return nil, err
860+
return nil, p.wrapParseError(decoder, token, err)
857861
}
858862
foreachNode.Nodes = append(foreachNode.Nodes, n)
859863
case xml.CharData:
@@ -883,7 +887,7 @@ func (p *XMLMappersElementParser) parseChoose(mapper *Mapper, decoder *xml.Decod
883887
switch token := token.(type) {
884888
case xml.StartElement:
885889
if handled, err := p.parseBindAndAppend(decoder, token, &chooseNode.BindNodes); err != nil {
886-
return nil, err
890+
return nil, p.wrapParseError(decoder, token, err)
887891
} else if handled {
888892
continue
889893
}
@@ -892,16 +896,16 @@ func (p *XMLMappersElementParser) parseChoose(mapper *Mapper, decoder *xml.Decod
892896
case "when":
893897
n, err := p.parseWhen(mapper, decoder, token)
894898
if err != nil {
895-
return nil, err
899+
return nil, p.wrapParseError(decoder, token, err)
896900
}
897901
chooseNode.WhenNodes = append(chooseNode.WhenNodes, n)
898902
case "otherwise":
899903
if chooseNode.OtherwiseNode != nil {
900-
return nil, errors.New("otherwise is only once")
904+
return nil, p.wrapParseError(decoder, token, errors.New("otherwise is only once"))
901905
}
902906
n, err := p.parseOtherwise(mapper, decoder)
903907
if err != nil {
904-
return nil, err
908+
return nil, p.wrapParseError(decoder, token, err)
905909
}
906910
chooseNode.OtherwiseNode = n
907911
}
@@ -920,10 +924,10 @@ func (p *XMLMappersElementParser) parseSQLNode(mapper *Mapper, decoder *xml.Deco
920924

921925
sqlNode.ID = getXMLAttr("id", token.Attr)
922926
if sqlNode.ID == "" {
923-
return nil, &nodeAttributeRequiredError{nodeName: "sql", attrName: "id"}
927+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "sql", attrName: "id"})
924928
}
925929
if strings.Contains(sqlNode.ID, ".") {
926-
return nil, fmt.Errorf("sql id can not contain '.' %s", sqlNode.ID)
930+
return nil, p.wrapParseError(decoder, token, fmt.Errorf("sql id can not contain '.' %s", sqlNode.ID))
927931
}
928932

929933
for {
@@ -937,13 +941,13 @@ func (p *XMLMappersElementParser) parseSQLNode(mapper *Mapper, decoder *xml.Deco
937941
switch token := token.(type) {
938942
case xml.StartElement:
939943
if handled, err := p.parseBindAndAppend(decoder, token, &sqlNode.BindNodes); err != nil {
940-
return nil, err
944+
return nil, p.wrapParseError(decoder, token, err)
941945
} else if handled {
942946
continue
943947
}
944948
tags, err := p.parseTags(mapper, decoder, token)
945949
if err != nil {
946-
return nil, err
950+
return nil, p.wrapParseError(decoder, token, err)
947951
}
948952
sqlNode.Nodes = append(sqlNode.Nodes, tags)
949953
case xml.CharData:
@@ -965,12 +969,12 @@ func (p *XMLMappersElementParser) parseWhen(mapper *Mapper, decoder *xml.Decoder
965969

966970
test := getXMLAttr("test", token.Attr)
967971
if test == "" {
968-
return nil, &nodeAttributeRequiredError{nodeName: "when", attrName: "test"}
972+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "when", attrName: "test"})
969973
}
970974

971975
// parse condition expression
972976
if err := whenNode.Parse(test); err != nil {
973-
return nil, err
977+
return nil, p.wrapParseError(decoder, token, err)
974978
}
975979
for {
976980
token, err := decoder.Token()
@@ -983,13 +987,13 @@ func (p *XMLMappersElementParser) parseWhen(mapper *Mapper, decoder *xml.Decoder
983987
switch token := token.(type) {
984988
case xml.StartElement:
985989
if handled, err := p.parseBindAndAppend(decoder, token, &whenNode.BindNodes); err != nil {
986-
return nil, err
990+
return nil, p.wrapParseError(decoder, token, err)
987991
} else if handled {
988992
continue
989993
}
990994
n, err := p.parseTags(mapper, decoder, token)
991995
if err != nil {
992-
return nil, err
996+
return nil, p.wrapParseError(decoder, token, err)
993997
}
994998
whenNode.Nodes = append(whenNode.Nodes, n)
995999
case xml.CharData:
@@ -1019,14 +1023,14 @@ func (p *XMLMappersElementParser) parseOtherwise(mapper *Mapper, decoder *xml.De
10191023
switch token := token.(type) {
10201024
case xml.StartElement:
10211025
if handled, err := p.parseBindAndAppend(decoder, token, &otherwiseNode.BindNodes); err != nil {
1022-
return nil, err
1026+
return nil, p.wrapParseError(decoder, token, err)
10231027
} else if handled {
10241028
continue
10251029
}
10261030

10271031
tags, err := p.parseTags(mapper, decoder, token)
10281032
if err != nil {
1029-
return nil, err
1033+
return nil, p.wrapParseError(decoder, token, err)
10301034
}
10311035
otherwiseNode.Nodes = append(otherwiseNode.Nodes, tags)
10321036
case xml.CharData:
@@ -1058,16 +1062,16 @@ func (p *XMLMappersElementParser) parseBind(decoder *xml.Decoder, token xml.Star
10581062
}
10591063

10601064
if bindNode.Name == "" {
1061-
return nil, &nodeAttributeRequiredError{nodeName: "bind", attrName: "name"}
1065+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "bind", attrName: "name"})
10621066
}
10631067

10641068
if value == "" {
1065-
return nil, &nodeAttributeRequiredError{nodeName: "bind", attrName: "value"}
1069+
return nil, p.wrapParseError(decoder, token, &nodeAttributeRequiredError{nodeName: "bind", attrName: "value"})
10661070
}
10671071

10681072
// parse expression
10691073
if err := bindNode.Parse(value); err != nil {
1070-
return nil, err
1074+
return nil, p.wrapParseError(decoder, token, err)
10711075
}
10721076

10731077
for {
@@ -1165,3 +1169,40 @@ func getXMLAttr(key string, attrs []xml.Attr) string {
11651169
}
11661170
return ""
11671171
}
1172+
1173+
// wrapParseError wraps an error with XML parsing context including namespace and XML content.
1174+
func (p *XMLMappersElementParser) wrapParseError(decoder *xml.Decoder, token xml.StartElement, err error) error {
1175+
if err == nil {
1176+
return nil
1177+
}
1178+
1179+
// If it's already an XMLParseError, return as is
1180+
if _, ok := err.(*XMLParseError); ok {
1181+
return err
1182+
}
1183+
1184+
// Build XML content string
1185+
xmlContent := buildXMLContent(token)
1186+
1187+
return &XMLParseError{
1188+
Namespace: p.currentNamespace,
1189+
XMLContent: xmlContent,
1190+
Err: err,
1191+
}
1192+
}
1193+
1194+
// buildXMLContent constructs a string representation of the XML element.
1195+
func buildXMLContent(token xml.StartElement) string {
1196+
var builder strings.Builder
1197+
builder.WriteString("<")
1198+
builder.WriteString(token.Name.Local)
1199+
for _, attr := range token.Attr {
1200+
builder.WriteString(" ")
1201+
builder.WriteString(attr.Name.Local)
1202+
builder.WriteString("=\"")
1203+
builder.WriteString(attr.Value)
1204+
builder.WriteString("\"")
1205+
}
1206+
builder.WriteString(">")
1207+
return builder.String()
1208+
}

0 commit comments

Comments
 (0)