fix(log): Capture service errors only once#558
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
==========================================
+ Coverage 87.45% 87.51% +0.05%
==========================================
Files 93 93
Lines 14760 14754 -6
==========================================
+ Hits 12909 12912 +3
+ Misses 1851 1842 -9
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db71ca5. Configure here.
| .map_err(|e| ApiError::Service(e.into()))? | ||
| .map_err(|e| { | ||
| objectstore_log::error!(!!&e, "failed to collect payload stream"); | ||
| ApiError::Service(e.into()) |
There was a problem hiding this comment.
If we refactor ApiError similar to #547, we can directly propagate the IO error and no longer have to track it explicitly here.
lcian
left a comment
There was a problem hiding this comment.
All the approaches we discussed seem to have their shortcomings, so I'm happy to go with the one you think is best.
For every service error, we received two issues in Sentry: 1. Captured within the service by the spawned task itself 2. Captured by the web middleware when unwinding the error Since tasks can outlive their requests, we need to keep capturing inside the service. Consequently, we now skip error tracking for service error kinds in the API layer. There are a few exceptions where a service error is constructed outside of the spawned task, which is now tracked explicitly.

For every service error, we received two issues in Sentry:
Since tasks can outlive their requests, we need to keep capturing inside
the service. Consequently, we now skip error tracking for service error
kinds in the API layer. There are a few exceptions where a service error
is constructed outside of the spawned task, which is now tracked
explicitly.
Ref FS-439