Fix data race in ErrorWithUUID.UUID() causing CI test failures (#40961)
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>
This commit is contained in:
@@ -9,6 +9,7 @@ import (
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/docker/go-units"
|
||||
platform_errors "github.com/fleetdm/fleet/v4/server/platform/errors"
|
||||
@@ -39,21 +40,23 @@ type ErrorUUIDer interface {
|
||||
}
|
||||
|
||||
// ErrorWithUUID can be embedded in error types to implement ErrorUUIDer.
|
||||
// The UUID is lazily generated on first access and is safe for concurrent use.
|
||||
type ErrorWithUUID struct {
|
||||
uuid string
|
||||
uuidOnce sync.Once
|
||||
uuid string
|
||||
}
|
||||
|
||||
var _ ErrorUUIDer = (*ErrorWithUUID)(nil)
|
||||
|
||||
// UUID implements the ErrorUUIDer interface.
|
||||
func (e *ErrorWithUUID) UUID() string {
|
||||
if e.uuid == "" {
|
||||
e.uuidOnce.Do(func() {
|
||||
u, err := uuid.NewRandom()
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
e.uuid = u.String()
|
||||
}
|
||||
})
|
||||
return e.uuid
|
||||
}
|
||||
|
||||
@@ -77,7 +80,7 @@ func (e *BadRequestError) BadRequestError() []map[string]string {
|
||||
}
|
||||
|
||||
// Internal implements the ErrWithInternal interface.
|
||||
func (e BadRequestError) Internal() string {
|
||||
func (e *BadRequestError) Internal() string {
|
||||
if e.InternalErr != nil {
|
||||
return e.InternalErr.Error()
|
||||
}
|
||||
@@ -86,12 +89,12 @@ func (e BadRequestError) Internal() string {
|
||||
|
||||
// We implement the second type of Unwrap that returns an error array, which still works for errors.Is/As, but is not supported in errors.Unwrap
|
||||
// This allows us to check the error chain, but not log the most inner error in the HTTP response.
|
||||
func (e BadRequestError) Unwrap() []error {
|
||||
func (e *BadRequestError) Unwrap() []error {
|
||||
return []error{e.InternalErr}
|
||||
}
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
func (e BadRequestError) IsClientError() bool {
|
||||
func (e *BadRequestError) IsClientError() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -150,7 +153,7 @@ func NewUserMessageError(err error, statusCode int) *UserMessageError {
|
||||
}
|
||||
|
||||
// StatusCode returns the HTTP status code for this error.
|
||||
func (e UserMessageError) StatusCode() int {
|
||||
func (e *UserMessageError) StatusCode() int {
|
||||
if e.statusCode > 0 {
|
||||
return e.statusCode
|
||||
}
|
||||
@@ -159,7 +162,7 @@ func (e UserMessageError) StatusCode() int {
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
// Returns true for 4xx status codes, false for 5xx.
|
||||
func (e UserMessageError) IsClientError() bool {
|
||||
func (e *UserMessageError) IsClientError() bool {
|
||||
code := e.StatusCode()
|
||||
return code >= 400 && code < 500
|
||||
}
|
||||
@@ -186,7 +189,7 @@ func GetJSONUnknownField(err error) *string {
|
||||
// UserMessage implements the user-friendly translation of the error if its
|
||||
// root cause is one of the supported types, otherwise it returns the error
|
||||
// message.
|
||||
func (e UserMessageError) UserMessage() string {
|
||||
func (e *UserMessageError) UserMessage() string {
|
||||
cause := platform_errors.Cause(e.error)
|
||||
switch cause := cause.(type) {
|
||||
case *json.UnmarshalTypeError:
|
||||
@@ -271,22 +274,22 @@ func NewAuthFailedError(internal string) *AuthFailedError {
|
||||
}
|
||||
|
||||
// Error implements the error interface.
|
||||
func (e AuthFailedError) Error() string {
|
||||
func (e *AuthFailedError) Error() string {
|
||||
return "Authentication failed"
|
||||
}
|
||||
|
||||
// Internal implements ErrWithInternal.
|
||||
func (e AuthFailedError) Internal() string {
|
||||
func (e *AuthFailedError) Internal() string {
|
||||
return e.internal
|
||||
}
|
||||
|
||||
// StatusCode implements kithttp.StatusCoder.
|
||||
func (e AuthFailedError) StatusCode() int {
|
||||
func (e *AuthFailedError) StatusCode() int {
|
||||
return http.StatusUnauthorized
|
||||
}
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
func (e AuthFailedError) IsClientError() bool {
|
||||
func (e *AuthFailedError) IsClientError() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -304,22 +307,22 @@ func NewAuthRequiredError(internal string) *AuthRequiredError {
|
||||
}
|
||||
|
||||
// Error implements the error interface.
|
||||
func (e AuthRequiredError) Error() string {
|
||||
func (e *AuthRequiredError) Error() string {
|
||||
return "Authentication required"
|
||||
}
|
||||
|
||||
// Internal implements ErrWithInternal.
|
||||
func (e AuthRequiredError) Internal() string {
|
||||
func (e *AuthRequiredError) Internal() string {
|
||||
return e.internal
|
||||
}
|
||||
|
||||
// StatusCode implements kithttp.StatusCoder.
|
||||
func (e AuthRequiredError) StatusCode() int {
|
||||
func (e *AuthRequiredError) StatusCode() int {
|
||||
return http.StatusUnauthorized
|
||||
}
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
func (e AuthRequiredError) IsClientError() bool {
|
||||
func (e *AuthRequiredError) IsClientError() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -339,22 +342,22 @@ func NewAuthHeaderRequiredError(internal string) *AuthHeaderRequiredError {
|
||||
}
|
||||
|
||||
// Error implements the error interface.
|
||||
func (e AuthHeaderRequiredError) Error() string {
|
||||
func (e *AuthHeaderRequiredError) Error() string {
|
||||
return "Authorization header required"
|
||||
}
|
||||
|
||||
// Internal implements ErrWithInternal.
|
||||
func (e AuthHeaderRequiredError) Internal() string {
|
||||
func (e *AuthHeaderRequiredError) Internal() string {
|
||||
return e.internal
|
||||
}
|
||||
|
||||
// StatusCode implements kithttp.StatusCoder.
|
||||
func (e AuthHeaderRequiredError) StatusCode() int {
|
||||
func (e *AuthHeaderRequiredError) StatusCode() int {
|
||||
return http.StatusUnauthorized
|
||||
}
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
func (e AuthHeaderRequiredError) IsClientError() bool {
|
||||
func (e *AuthHeaderRequiredError) IsClientError() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -366,17 +369,17 @@ type passwordResetRequiredError struct {
|
||||
}
|
||||
|
||||
// Error implements the error interface.
|
||||
func (e passwordResetRequiredError) Error() string {
|
||||
func (e *passwordResetRequiredError) Error() string {
|
||||
return "password reset required"
|
||||
}
|
||||
|
||||
// StatusCode implements kithttp.StatusCoder.
|
||||
func (e passwordResetRequiredError) StatusCode() int {
|
||||
func (e *passwordResetRequiredError) StatusCode() int {
|
||||
return http.StatusUnauthorized
|
||||
}
|
||||
|
||||
// IsClientError implements ErrWithIsClientError.
|
||||
func (e passwordResetRequiredError) IsClientError() bool {
|
||||
func (e *passwordResetRequiredError) IsClientError() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user