From 2b3532d2482209491c20fcc1ec75ffeace38fb96 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Thu, 7 Sep 2023 07:33:50 +0930 Subject: [PATCH] x-pack/filebeat/input/httpjson: improve error logging * make errors relate to behaviour rather than implementation * remove duplicated wrapping * make use of textContextError for JSON decoding failures * improve consistency of error formatting and typography --- CHANGELOG.next.asciidoc | 1 + x-pack/filebeat/input/httpjson/request.go | 39 ++++++++++--------- .../filebeat/input/httpjson/request_test.go | 6 +-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 8f016aff7079..4a31d76c29fa 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -216,6 +216,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] - Allow fine-grained control of entity analytics API requests for Okta provider. {issue}36440[36440] {pull}36492[36492] - Add support for expanding `journald.process.capabilities` into the human-readable effective capabilities in the ECS `process.thread.capabilities.effective` field. {issue}36454[36454] {pull}36470[36470] - Allow fine-grained control of entity analytics API requests for AzureAD provider. {issue}36440[36440] {pull}36441[36441] +- Improve error logging in HTTPJSON input. {pull}[] *Auditbeat* diff --git a/x-pack/filebeat/input/httpjson/request.go b/x-pack/filebeat/input/httpjson/request.go index 6a1d926ab40c..5dc49560313d 100644 --- a/x-pack/filebeat/input/httpjson/request.go +++ b/x-pack/filebeat/input/httpjson/request.go @@ -57,7 +57,7 @@ func (r *requester) doRequest(stdCtx context.Context, trCtx *transformContext, p // perform and store regular call responses httpResp, err = rf.collectResponse(stdCtx, trCtx, r) if err != nil { - return fmt.Errorf("failed to execute rf.collectResponse: %w", err) + return fmt.Errorf("failed to collect first response: %w", err) } if rf.saveFirstResponse { @@ -70,7 +70,7 @@ func (r *requester) doRequest(stdCtx context.Context, trCtx *transformContext, p httpResp.Body = io.NopCloser(bytes.NewReader(body)) err = json.Unmarshal(body, &bodyMap) if err != nil { - r.log.Errorf("unable to unmarshal first_response.body: %v", err) + r.log.Errorf("unable to unmarshal first_response.body: %v", textContextError{error: err, body: body}) } firstResponse := response{ url: *httpResp.Request.URL, @@ -165,7 +165,7 @@ func (r *requester) doRequest(stdCtx context.Context, trCtx *transformContext, p // collect data from new urls httpResp, err = rf.collectResponse(stdCtx, chainTrCtx, r) if err != nil { - return fmt.Errorf("failed to execute rf.collectResponse: %w", err) + return fmt.Errorf("failed to collect tail response %d: %w", i, err) } // store data according to response type if i == len(r.requestFactories)-1 && len(ids) != 0 { @@ -222,12 +222,12 @@ func (rf *requestFactory) collectResponse(stdCtx context.Context, trCtx *transfo if rf.isChain && rf.chainHTTPClient != nil { httpResp, err = rf.chainHTTPClient.do(stdCtx, req) if err != nil { - return nil, fmt.Errorf("failed to execute chain http client.Do: %w", err) + return nil, fmt.Errorf("failed to execute chain http %s: %w", req.Method, err) } } else { httpResp, err = r.client.do(stdCtx, req) if err != nil { - return nil, fmt.Errorf("failed to execute http client.Do: %w", err) + return nil, fmt.Errorf("failed to execute http %s: %w", req.Method, err) } } @@ -239,7 +239,7 @@ func (c *httpClient) do(stdCtx context.Context, req *http.Request) (*http.Respon return c.client.Do(req) }) if err != nil { - return nil, fmt.Errorf("failed to execute http client.Do: %w", err) + return nil, err } defer resp.Body.Close() @@ -250,9 +250,12 @@ func (c *httpClient) do(stdCtx context.Context, req *http.Request) (*http.Respon return nil, fmt.Errorf("failed to read response body: %w", err) } - if resp.StatusCode > 399 { + if resp.StatusCode >= http.StatusBadRequest { body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("server responded with status code %d: %s", resp.StatusCode, string(body)) + if len(body) == 0 { + return nil, fmt.Errorf("server responded with status code %d", resp.StatusCode) + } + return nil, fmt.Errorf("server responded with status code %d: %s", resp.StatusCode, body) } return resp, nil } @@ -311,7 +314,7 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met ch.Step.Auth = tryAssignAuth(config.Auth, ch.Step.Auth) httpClient, err := newChainHTTPClient(ctx, ch.Step.Auth, ch.Step.Request, log, reg) if err != nil { - return nil, fmt.Errorf("failed in creating chain http client with error : %w", err) + return nil, fmt.Errorf("failed in creating chain http client with error: %w", err) } if ch.Step.Auth != nil && ch.Step.Auth.Basic.isEnabled() { rf.user = ch.Step.Auth.Basic.User @@ -339,7 +342,7 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met ch.While.Auth = tryAssignAuth(config.Auth, ch.While.Auth) httpClient, err := newChainHTTPClient(ctx, ch.While.Auth, ch.While.Request, log, reg, policy) if err != nil { - return nil, fmt.Errorf("failed in creating chain http client with error : %w", err) + return nil, fmt.Errorf("failed in creating chain http client with error: %w", err) } if ch.While.Auth != nil && ch.While.Auth.Basic.isEnabled() { rf.user = ch.While.Auth.Basic.User @@ -372,7 +375,7 @@ func evaluateResponse(expression *valueTpl, data []byte, log *logp.Logger) (bool err := json.Unmarshal(data, &dataMap) if err != nil { - return false, fmt.Errorf("error while unmarshalling data : %w", err) + return false, fmt.Errorf("error while unmarshalling data: %w", textContextError{error: err, body: data}) } tr := transformable{} paramCtx := &transformContext{ @@ -384,11 +387,11 @@ func evaluateResponse(expression *valueTpl, data []byte, log *logp.Logger) (bool val, err := expression.Execute(paramCtx, tr, "", nil, log) if err != nil { - return false, fmt.Errorf("error while evaluating expression : %w", err) + return false, fmt.Errorf("error while evaluating expression: %w", err) } result, err := strconv.ParseBool(val) if err != nil { - return false, fmt.Errorf("error while parsing boolean value of string : %w", err) + return false, fmt.Errorf("error while parsing boolean value of string: %w", err) } return result, nil @@ -513,7 +516,7 @@ func (r *requester) getIdsFromResponses(intermediateResps []*http.Response, repl // get replace values from collected json var v interface{} if err := json.Unmarshal(b, &v); err != nil { - return nil, fmt.Errorf("cannot unmarshal data: %w", err) + return nil, fmt.Errorf("cannot unmarshal data: %w", textContextError{error: err, body: b}) } values, err := jsonpath.Get(replace, v) if err != nil { @@ -534,7 +537,7 @@ func (r *requester) getIdsFromResponses(intermediateResps []*http.Response, repl case float64, string: ids = append(ids, fmt.Sprintf("%v", tresp)) default: - r.log.Errorf("cannot collect IDs from type '%T' : '%v'", values, values) + r.log.Errorf("cannot collect IDs from type %T: %v", values, values) } } return ids, nil @@ -662,21 +665,21 @@ func (r *requester) processChainPaginationEvents(stdCtx context.Context, trCtx * // reformat urls of requestFactory using ids rf.url, err = generateNewUrl(rf.replace, urlString, id) if err != nil { - return -1, fmt.Errorf("failed to generate new URL: %w", err) + return -1, fmt.Errorf("failed to generate new url: %w", err) } // reformat url accordingly if replaceWith clause exists if doReplaceWith { rf.url, err = generateNewUrl(strings.TrimSpace(replaceArr[0]), rf.url.String(), val) if err != nil { - return n, fmt.Errorf("failed to generate new URL: %w", err) + return n, fmt.Errorf("failed to generate new url: %w", err) } } // collect data from new urls httpResp, err = rf.collectResponse(stdCtx, chainTrCtx, r) if err != nil { - return -1, fmt.Errorf("failed to execute rf.collectResponse: %w", err) + return -1, fmt.Errorf("failed to collect response: %w", err) } // store data according to response type if i == len(r.requestFactories)-1 && len(ids) != 0 { diff --git a/x-pack/filebeat/input/httpjson/request_test.go b/x-pack/filebeat/input/httpjson/request_test.go index 315a3d4864bb..05fefebef5c9 100644 --- a/x-pack/filebeat/input/httpjson/request_test.go +++ b/x-pack/filebeat/input/httpjson/request_test.go @@ -216,7 +216,7 @@ func Test_evaluateResponse(t *testing.T) { log: log, }, want: false, - expectedError: "error while parsing boolean value of string : strconv.ParseBool: parsing \"eq .last_response.body.status \\\"completed\\\" ]]\": invalid syntax", + expectedError: "error while parsing boolean value of string: strconv.ParseBool: parsing \"eq .last_response.body.status \\\"completed\\\" ]]\": invalid syntax", }, { name: "newEvaluateResponse_emptyExpressionError", @@ -226,7 +226,7 @@ func Test_evaluateResponse(t *testing.T) { log: log, }, want: false, - expectedError: "error while evaluating expression : the template result is empty", + expectedError: "error while evaluating expression: the template result is empty", }, { name: "newEvaluateResponse_incompleteExpressionError", @@ -236,7 +236,7 @@ func Test_evaluateResponse(t *testing.T) { log: log, }, want: false, - expectedError: "error while parsing boolean value of string : strconv.ParseBool: parsing \"initiated\": invalid syntax", + expectedError: "error while parsing boolean value of string: strconv.ParseBool: parsing \"initiated\": invalid syntax", }, } for _, tt := range tests {