fix(objectstore): Do not log errors for client aborts#6064
Conversation
| if self.kind.is_client_error() { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is the code change that suppresses error logs for client errors.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| ErrorKind::LoadShed => false, | ||
| ErrorKind::UploadFailed(objectstore_client::Error::Reqwest(error)) => { | ||
| find_error_source(error, is_hyper_user_error).is_some() | ||
| } | ||
| ErrorKind::UploadFailed(_) => false, | ||
| ErrorKind::Uuid(_) => false, | ||
| } |
There was a problem hiding this comment.
Is there a rule on when to do this vs?
ErrorKind::InvalidScoping
| ErrorKind::Timeout(_)
| ErrorKind::LoadShed
| ErrorKind::Uuid(_) => false,
There was a problem hiding this comment.
Simplified the expression now.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6324cba. Configure here.
| find_error_source(error, is_user_error).is_some() | ||
| } | ||
| _ => false, | ||
| } |
There was a problem hiding this comment.
Client abort detection too narrow
Medium Severity
is_client_error only treats UploadFailed reqwest errors as client faults when the source chain contains an io::Error with kind UnexpectedEof or FileTooLarge. The upload endpoint already classifies the same objectstore UploadFailed reqwest failures using is_hyper_user_error, and PATCH bodies map inbound failures to io::Error::other, so many disconnects never match and still emit relay_log::error!.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6324cba. Configure here.
There was a problem hiding this comment.
That's OK, I want to start conservatively and relax the condition if necessary.
2 participants
Most of the "upload failed" errors we see in the objectstore service are broken client streams (unexpected EOF).
In both cases we do not need to log an error in the objectstore service.
Fixes INGEST-884