Improved orbit debug logs when response contains a large HTML page. (#33195)
Resolves #33219 Note: this only fixes orbit. The issue remains on osquery: [#33019](https://github.com/fleetdm/fleet/issues/33019) # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## fleetd/orbit/Fleet Desktop - [x] Verified compatibility with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md)) - [x] Verified that fleetd runs on macOS, Linux and Windows - [x] Verified auto-update works from the released version of component to the new version (see [tools/tuf/test](../tools/tuf/test/README.md)) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Improved error messages when servers return HTML instead of JSON. - Truncates oversized responses in logs to prevent overwhelming output while preserving context. - More robust parsing of non-JSON error responses. - Documentation - Added changelog entry noting enhanced debug logging for large HTML responses. - Tests - Added tests covering HTML, plain text, empty, long, and invalid JSON error bodies to validate error message handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Improved orbit debug logs when response contains a large HTML page.
|
||||
@@ -81,7 +81,13 @@ func (bc *baseClient) parseResponse(verb, path string, response *http.Response,
|
||||
return fmt.Errorf("reading response body: %w", err)
|
||||
}
|
||||
if err := json.Unmarshal(b, &responseDest); err != nil {
|
||||
return fmt.Errorf("decode %s %s response: %w, body: %s", verb, path, err, b)
|
||||
const maxBodyLen = 200
|
||||
truncatedBytes, isHTML := truncateAndDetectHTML(b, maxBodyLen)
|
||||
|
||||
if isHTML {
|
||||
return fmt.Errorf("decode %s %s response: %w, (server returned HTML instead of JSON), body: %s", verb, path, err, truncatedBytes)
|
||||
}
|
||||
return fmt.Errorf("decode %s %s response: %w, body: %s", verb, path, err, truncatedBytes)
|
||||
}
|
||||
if e, ok := responseDest.(fleet.Errorer); ok {
|
||||
if e.Error() != nil {
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"strings"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
)
|
||||
@@ -104,15 +106,55 @@ type serverError struct {
|
||||
} `json:"errors"`
|
||||
}
|
||||
|
||||
// truncateAndDetectHTML truncates a response body to a reasonable length and
|
||||
// detects if it's HTML content. Returns the truncated body and whether it's HTML.
|
||||
func truncateAndDetectHTML(body []byte, maxLen int) (truncated []byte, isHTML bool) {
|
||||
if len(body) > maxLen {
|
||||
// Use append which is more idiomatic and efficient
|
||||
truncated = append([]byte(nil), body[:maxLen]...)
|
||||
truncated = append(truncated, "..."...)
|
||||
} else {
|
||||
// For small bodies, we can return the slice directly since it will be
|
||||
// converted to string soon anyway and won't hold a large underlying array
|
||||
truncated = body
|
||||
}
|
||||
lowerPrefix := bytes.ToLower(truncated)
|
||||
isHTML = bytes.Contains(lowerPrefix, []byte("<html")) || bytes.Contains(lowerPrefix, []byte("<!doctype"))
|
||||
|
||||
// Return truncated byte slice
|
||||
return truncated, isHTML
|
||||
}
|
||||
|
||||
func extractServerErrorText(body io.Reader) string {
|
||||
_, reason := extractServerErrorNameReason(body)
|
||||
return reason
|
||||
}
|
||||
|
||||
func extractServerErrorNameReason(body io.Reader) (string, string) {
|
||||
// Read the body first so we can try to parse it as JSON and fallback to text if needed
|
||||
bodyBytes, err := io.ReadAll(body)
|
||||
if err != nil {
|
||||
return "", "failed to read response body"
|
||||
}
|
||||
|
||||
// Try to parse as JSON first
|
||||
var serverErr serverError
|
||||
if err := json.NewDecoder(body).Decode(&serverErr); err != nil {
|
||||
return "", "unknown"
|
||||
if err := json.Unmarshal(bodyBytes, &serverErr); err != nil {
|
||||
// If it's not JSON, it might be HTML or plain text error from a proxy/load balancer
|
||||
const maxLen = 200
|
||||
truncatedBytes, isHTML := truncateAndDetectHTML(bodyBytes, maxLen)
|
||||
|
||||
if isHTML {
|
||||
// Generic HTML response
|
||||
return "", fmt.Sprintf("server returned HTML instead of JSON response, body: %s", truncatedBytes)
|
||||
}
|
||||
|
||||
// Return cleaned up text for non-HTML responses
|
||||
truncated := strings.TrimSpace(string(truncatedBytes))
|
||||
if truncated == "" {
|
||||
return "", "empty response body"
|
||||
}
|
||||
return "", truncated
|
||||
}
|
||||
|
||||
errName := ""
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestExtractServerErrorText(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
body string
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "valid JSON error",
|
||||
body: `{"message": "Something went wrong", "errors": [{"name": "error1", "reason": "invalid input"}]}`,
|
||||
expected: "Something went wrong: invalid input",
|
||||
},
|
||||
{
|
||||
name: "403 Forbidden HTML",
|
||||
body: `<!DOCTYPE html><html><head><title>403 Forbidden</title></head><body><h1>403 Forbidden</h1><p>You don't have permission to access this resource.</p></body></html>`,
|
||||
expected: "server returned HTML instead of JSON response, body: <!DOCTYPE html><html><head><title>403 Forbidden</title></head><body><h1>403 Forbidden</h1><p>You don't have permission to access this resource.</p></body></html>",
|
||||
},
|
||||
{
|
||||
name: "HTML with uppercase tags",
|
||||
body: `<HTML><HEAD><TITLE>Error</TITLE></HEAD><BODY>Server Error</BODY></HTML>`,
|
||||
expected: "server returned HTML instead of JSON response, body: <HTML><HEAD><TITLE>Error</TITLE></HEAD><BODY>Server Error</BODY></HTML>",
|
||||
},
|
||||
{
|
||||
name: "long HTML gets truncated",
|
||||
body: `<!DOCTYPE html><html><head><title>Error Page</title></head><body>` + strings.Repeat("A", 200) + `</body></html>`,
|
||||
expected: "server returned HTML instead of JSON response, body: <!DOCTYPE html><html><head><title>Error Page</title></head><body>" + strings.Repeat("A", 135) + "...",
|
||||
},
|
||||
{
|
||||
name: "plain text error",
|
||||
body: "Connection refused",
|
||||
expected: "Connection refused",
|
||||
},
|
||||
{
|
||||
name: "empty response",
|
||||
body: "",
|
||||
expected: "empty response body",
|
||||
},
|
||||
{
|
||||
name: "long plain text truncated",
|
||||
body: strings.Repeat("a", 250),
|
||||
expected: strings.Repeat("a", 200) + "...",
|
||||
},
|
||||
{
|
||||
name: "invalid JSON",
|
||||
body: `{invalid json}`,
|
||||
expected: "{invalid json}",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
reader := strings.NewReader(tt.body)
|
||||
result := extractServerErrorText(reader)
|
||||
assert.Equal(t, tt.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user