Skip to content

Commit

Permalink
x-pack/filebeat/input/httpjson: make response body decoding errors mo…
Browse files Browse the repository at this point in the history
…re informative

The default rendering of errors from the json, csv and xml decoders can
be a little terse, but the error values themselves contain information
that allows the text context of the error to be constructed and
returned. It is a common problem that users are unable to decipher the
error messages, so add this text context to help.
  • Loading branch information
efd6 committed Aug 31, 2023
1 parent ac5f806 commit 3917f92
Show file tree
Hide file tree
Showing 3 changed files with 281 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- [Azure] Add input metrics to the azure-eventhub input. {pull}35739[35739]
- Reduce HTTPJSON metrics allocations. {pull}36282[36282]
- Add support for a simplified input configuraton when running under Elastic-Agent {pull}36390[36390]
- Make HTTPJSON response body decoding errors more informative. {pull}36481[36481]

*Auditbeat*

Expand Down
136 changes: 131 additions & 5 deletions x-pack/filebeat/input/httpjson/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import (
"bytes"
"encoding/csv"
"encoding/json"
stdxml "encoding/xml"
"errors"
"fmt"
"io"
"net/http"
"unicode"

"github.com/elastic/mito/lib/xml"
)
Expand Down Expand Up @@ -72,7 +75,11 @@ func encodeAsJSON(trReq transformable) ([]byte, error) {

// decodeAsJSON decodes the JSON message in p into dst.
func decodeAsJSON(p []byte, dst *response) error {
return json.Unmarshal(p, &dst.body)
err := json.Unmarshal(p, &dst.body)
if err != nil {
return jsonError{error: err, body: p}
}
return nil
}

// encodeAsForm encodes trReq as a URL encoded form.
Expand All @@ -95,14 +102,36 @@ func decodeAsNdjson(p []byte, dst *response) error {
for dec.More() {
var o interface{}
if err := dec.Decode(&o); err != nil {
return err
return jsonError{error: err, body: p}
}
results = append(results, o)
}
dst.body = results
return nil
}

type jsonError struct {
error
body []byte
}

func (e jsonError) Error() string {
switch err := e.error.(type) {

Check failure on line 119 in x-pack/filebeat/input/httpjson/encoding.go

View workflow job for this annotation

GitHub Actions / lint (linux)

type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
case nil:
return "<nil>"
case *json.SyntaxError:
return fmt.Sprintf("%v: text context %q", err, textContext(e.body, err.Offset))
case *json.UnmarshalTypeError:
return fmt.Sprintf("%v: text context %q", err, textContext(e.body, err.Offset))
default:
return err.Error()
}
}

func (e jsonError) Unwrap() error {
return e.error
}

// decodeAsCSV decodes p as a headed CSV document into dst.
func decodeAsCSV(p []byte, dst *response) error {
var results []interface{}

Check failure on line 137 in x-pack/filebeat/input/httpjson/encoding.go

View workflow job for this annotation

GitHub Actions / lint (linux)

Consider pre-allocating `results` (prealloc)
Expand Down Expand Up @@ -135,7 +164,7 @@ func decodeAsCSV(p []byte, dst *response) error {

if err != nil {
if err != io.EOF { //nolint:errorlint // csv.Reader never wraps io.EOF.
return err
return csvError{error: err, body: p}
}
}

Expand All @@ -144,6 +173,31 @@ func decodeAsCSV(p []byte, dst *response) error {
return nil
}

type csvError struct {
error
body []byte
}

func (e csvError) Error() string {
switch err := e.error.(type) {

Check failure on line 182 in x-pack/filebeat/input/httpjson/encoding.go

View workflow job for this annotation

GitHub Actions / lint (linux)

type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
case nil:
return "<nil>"
case *csv.ParseError:
lines := bytes.Split(e.body, []byte{'\n'})
l := err.Line - 1 // Lines are 1-based.
if uint(l) >= uint(len(lines)) {
return err.Error()
}
return fmt.Sprintf("%v: text context %q", err, textContext(lines[l], int64(err.Column)))
default:
return err.Error()
}
}

func (e csvError) Unwrap() error {
return e.error
}

// decodeAsZip decodes p as a ZIP archive into dst.
func decodeAsZip(p []byte, dst *response) error {
var results []interface{}
Expand All @@ -165,7 +219,7 @@ func decodeAsZip(p []byte, dst *response) error {
var o interface{}
if err := dec.Decode(&o); err != nil {
rc.Close()
return err
return jsonError{error: err, body: p}
}
results = append(results, o)
}
Expand All @@ -185,9 +239,81 @@ func decodeAsZip(p []byte, dst *response) error {
func decodeAsXML(p []byte, dst *response) error {
cdata, body, err := xml.Unmarshal(bytes.NewReader(p), dst.xmlDetails)
if err != nil {
return err
return xmlError{error: err, body: p}
}
dst.body = body
dst.header["XML-CDATA"] = []string{cdata}
return nil
}

type xmlError struct {
error
body []byte
}

func (e xmlError) Error() string {
switch err := e.error.(type) {

Check failure on line 255 in x-pack/filebeat/input/httpjson/encoding.go

View workflow job for this annotation

GitHub Actions / lint (linux)

type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
case nil:
return "<nil>"
case *stdxml.SyntaxError:
lines := bytes.Split(e.body, []byte{'\n'})
l := err.Line - 1 // Lines are 1-based.
if uint(l) >= uint(len(lines)) {
return err.Error()
}
// The xml package does not provide column-level context,
// so just point to first non-whitespace character of the
// line. This doesn't make a great deal of difference
// except in deeply indented XML documents.
pos := bytes.IndexFunc(lines[l], func(r rune) bool {
return !unicode.IsSpace(r)
})
if pos < 0 {
pos = 0
}
return fmt.Sprintf("%v: text context %q", err, textContext(lines[l], int64(pos)))
default:
return err.Error()
}
}

func (e xmlError) Unwrap() error {
return e.error
}

// textContext returns the context of text around the provided position starting
// five bytes before pos and extending ten bytes, dependent on the length of the
// text and the value of pos relative to bounds. If a text truncation is made,
// an ellipsis is added to indicate this. The returned []byte should not be mutated
// as it may be shared with the caller.
func textContext(text []byte, pos int64) []byte {
left := maxInt64(0, pos-5)
text = text[left:]
var pad int64
if left != 0 {
pad = 3
text = append([]byte("..."), text...)
}
right := minInt(pos+10+pad, int64(len(text)))
if right != int64(len(text)) {
// Ensure we don't clobber the body's bytes.
text = append(text[:right:right], []byte("...")...)
} else {
text = text[:right]
}
return text
}

func minInt(a, b int64) int64 {
if a < b {
return a
}
return b
}

func maxInt64(a, b int64) int64 {
if a > b {
return a
}
return b
}
Loading

0 comments on commit 3917f92

Please sign in to comment.