Browse Source

Fix #7060: Return 400 InvalidRequest instead of 500 for context canceled errors (#7309)

When a client cancels an HTTP request (e.g., connection timeout, client
disconnect), the context gets canceled and propagates through the system
as "context canceled" or "code = Canceled" errors. These errors were
being treated as internal server errors (500) when they should be treated
as client errors (400).

Problem:
- Client cancels request or connection times out
- Filer fails to assign file ID with "context canceled"
- S3 API returns HTTP 500 Internal Server Error
- This is incorrect - it's a client issue, not a server issue

Solution:
Added detection for context canceled errors in filerErrorToS3Error():
- Detects "context canceled" and "code = Canceled" in error strings
- Returns ErrInvalidRequest (HTTP 400) instead of ErrInternalError (500)
- Properly attributes the error to the client, not the server

Changes:
- Updated filerErrorToS3Error() to detect context cancellation
- Added test cases for both gRPC and simple context canceled errors
- Maintains existing error handling for other error types

This ensures:
- Clients get appropriate 4xx error codes for their canceled requests
- Server metrics correctly reflect that these are client issues
- Monitoring/alerting won't trigger false positives for client timeouts

Fixes #7060
pull/4956/merge
Chris Lu 2 days ago
committed by GitHub
parent
commit
8bf727d225
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 3
      weed/s3api/s3api_object_handlers_put.go
  2. 10
      weed/s3api/s3api_object_handlers_put_test.go

3
weed/s3api/s3api_object_handlers_put.go

@ -383,6 +383,9 @@ func filerErrorToS3Error(errString string) s3err.ErrorCode {
switch { switch {
case errString == constants.ErrMsgBadDigest: case errString == constants.ErrMsgBadDigest:
return s3err.ErrBadDigest return s3err.ErrBadDigest
case strings.Contains(errString, "context canceled") || strings.Contains(errString, "code = Canceled"):
// Client canceled the request, return client error not server error
return s3err.ErrInvalidRequest
case strings.HasPrefix(errString, "existing ") && strings.HasSuffix(errString, "is a directory"): case strings.HasPrefix(errString, "existing ") && strings.HasSuffix(errString, "is a directory"):
return s3err.ErrExistingObjectIsDirectory return s3err.ErrExistingObjectIsDirectory
case strings.HasSuffix(errString, "is a file"): case strings.HasSuffix(errString, "is a file"):

10
weed/s3api/s3api_object_handlers_put_test.go

@ -18,6 +18,16 @@ func TestFilerErrorToS3Error(t *testing.T) {
errString: constants.ErrMsgBadDigest, errString: constants.ErrMsgBadDigest,
expectedErr: s3err.ErrBadDigest, expectedErr: s3err.ErrBadDigest,
}, },
{
name: "Context canceled error",
errString: "rpc error: code = Canceled desc = context canceled",
expectedErr: s3err.ErrInvalidRequest,
},
{
name: "Context canceled error (simple)",
errString: "context canceled",
expectedErr: s3err.ErrInvalidRequest,
},
{ {
name: "Directory exists error", name: "Directory exists error",
errString: "existing /path/to/file is a directory", errString: "existing /path/to/file is a directory",

Loading…
Cancel
Save