mirror of
https://github.com/fleetdm/fleet
synced 2026-05-24 09:28:54 +00:00
Resolves #40857. The scheduled CI runs (with -race enabled) were failing due to a data race in ErrorWithUUID.UUID(). The race occurred between: - HTTP response encoding calling UUID() to lazily initialize the uuid field - Error store background goroutine calling Error() via value-receiver methods, which copies the struct (including the uuid field) concurrently - Logging calls Fix: 1. Use sync.Once for thread-safe lazy UUID initialization 2. Change all value-receiver methods on types embedding ErrorWithUUID to pointer receivers to prevent struct copying that triggers the race 3. Add isNotFoundErr() helper to replace broken errors.Is/errors.As patterns that relied on value-type error comparisons From Claude Code Web (ported from my personal fork due to repo access level required). I've read through the code prior to submitting this PR. Prompt: > The scheduled run of .github/workflows/test-go.yaml has had a bunch of errors in integration tests, starting recently. set up and run the tests (including race detection) as if you were running in GotHub Actions, then figure out when the issue was introduced, and what needs to happen to fix the test errors. I expect that smoketests and continued during-dev validation of `main` leading up to 4.83.0 will be sufficient manual testing here. ## Testing - [x] Added/updated automated tests - [ ] QA'd all new/changed functionality manually --------- Co-authored-by: Claude <noreply@anthropic.com>
212 lines
5.2 KiB
Go
212 lines
5.2 KiB
Go
package service
|
|
|
|
import (
|
|
"bytes"
|
|
"database/sql"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"strings"
|
|
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
)
|
|
|
|
var (
|
|
ErrUnauthenticated = errors.New("unauthenticated, or invalid token")
|
|
ErrPasswordResetRequired = errors.New("Password reset required. Please sign into the Fleet UI to update your password, then log in again with: fleetctl login.")
|
|
ErrMissingLicense = errors.New("missing or invalid license")
|
|
// ErrEndUserAuthRequired is returned when an action (such as enrolling a device)
|
|
// requires end user authentication
|
|
ErrEndUserAuthRequired = errors.New("end user authentication required")
|
|
)
|
|
|
|
type SetupAlreadyErr interface {
|
|
SetupAlready() bool
|
|
Error() string
|
|
}
|
|
|
|
type setupAlreadyErr struct{}
|
|
|
|
func (e setupAlreadyErr) Error() string {
|
|
return "Fleet has already been setup"
|
|
}
|
|
|
|
func (e setupAlreadyErr) SetupAlready() bool {
|
|
return true
|
|
}
|
|
|
|
type NotSetupErr interface {
|
|
NotSetup() bool
|
|
Error() string
|
|
}
|
|
|
|
type notSetupErr struct{}
|
|
|
|
func (e notSetupErr) Error() string {
|
|
return "The Fleet instance is not set up yet"
|
|
}
|
|
|
|
func (e notSetupErr) NotSetup() bool {
|
|
return true
|
|
}
|
|
|
|
// TODO: we have a similar but different interface in the fleet package,
|
|
// fleet.NotFoundError - at the very least, the NotFound method should be the
|
|
// same in both (the other is currently IsNotFound), and ideally we'd just have
|
|
// one of those interfaces.
|
|
type NotFoundErr interface {
|
|
NotFound() bool
|
|
Error() string
|
|
}
|
|
|
|
type notFoundErr struct {
|
|
msg string
|
|
|
|
fleet.ErrorWithUUID
|
|
}
|
|
|
|
func (e *notFoundErr) Error() string {
|
|
if e.msg != "" {
|
|
return e.msg
|
|
}
|
|
return "The resource was not found"
|
|
}
|
|
|
|
func (e *notFoundErr) NotFound() bool {
|
|
return true
|
|
}
|
|
|
|
// Implement Is so that errors.Is(err, sql.ErrNoRows) returns true for an
|
|
// error of type *notFoundError, without having to wrap sql.ErrNoRows
|
|
// explicitly. It also matches other *notFoundErr targets so that pointer-based
|
|
// comparison works (pointers to distinct structs are never == even if their
|
|
// contents are identical).
|
|
func (e *notFoundErr) Is(other error) bool {
|
|
if other == sql.ErrNoRows {
|
|
return true
|
|
}
|
|
_, ok := other.(*notFoundErr)
|
|
return ok
|
|
}
|
|
|
|
// isNotFoundErr reports whether err's chain contains a *notFoundErr.
|
|
func isNotFoundErr(err error) bool {
|
|
var nfe *notFoundErr
|
|
return errors.As(err, &nfe)
|
|
}
|
|
|
|
type ConflictErr interface {
|
|
Conflict() bool
|
|
Error() string
|
|
}
|
|
|
|
type conflictErr struct {
|
|
msg string
|
|
}
|
|
|
|
func (e conflictErr) Error() string {
|
|
return e.msg
|
|
}
|
|
|
|
func (e conflictErr) Conflict() bool {
|
|
return true
|
|
}
|
|
|
|
type serverError struct {
|
|
Message string `json:"message"`
|
|
Errors []struct {
|
|
Name string `json:"name"`
|
|
Reason string `json:"reason"`
|
|
} `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.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 := ""
|
|
errReason := serverErr.Message
|
|
if len(serverErr.Errors) > 0 {
|
|
errReason += ": " + serverErr.Errors[0].Reason
|
|
errName = serverErr.Errors[0].Name
|
|
}
|
|
|
|
return errName, errReason
|
|
}
|
|
|
|
func extractServerErrorNameReasons(body io.Reader) ([]string, []string) {
|
|
var serverErr serverError
|
|
if err := json.NewDecoder(body).Decode(&serverErr); err != nil {
|
|
return []string{""}, []string{"unknown"}
|
|
}
|
|
|
|
var errName []string
|
|
var errReason []string
|
|
for _, err := range serverErr.Errors {
|
|
errName = append(errName, err.Name)
|
|
errReason = append(errReason, err.Reason)
|
|
}
|
|
|
|
return errName, errReason
|
|
}
|
|
|
|
type statusCodeErr struct {
|
|
code int
|
|
body string
|
|
}
|
|
|
|
func (e *statusCodeErr) Error() string {
|
|
return fmt.Sprintf("%d %s", e.code, e.body)
|
|
}
|
|
|
|
func (e *statusCodeErr) StatusCode() int {
|
|
return e.code
|
|
}
|