Skip to content

fix(log): Capture service errors only once#558

Merged
jan-auer merged 4 commits into
mainfrom
fix/service-error-double-capture
Jul 16, 2026
Merged

fix(log): Capture service errors only once#558
jan-auer merged 4 commits into
mainfrom
fix/service-error-double-capture

Conversation

@jan-auer

@jan-auer jan-auer commented Jul 15, 2026

Copy link
Copy Markdown
Member

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.

Ref FS-439

@jan-auer
jan-auer requested a review from a team as a code owner July 15, 2026 13:29
@linear-code

linear-code Bot commented Jul 15, 2026

Copy link
Copy Markdown

FS-439

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.51%. Comparing base (e254a12) to head (9e131f3).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
objectstore-server/src/endpoints/common.rs 63.63% 4 Missing ⚠️
objectstore-server/src/endpoints/batch.rs 40.00% 3 Missing ⚠️
objectstore-service/src/concurrency.rs 25.00% 3 Missing ⚠️
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     
Components Coverage Δ
Rust Backend 92.22% <50.00%> (+0.02%) ⬆️
Rust Client 79.89% <ø> (ø)
Python Client 90.20% <ø> (+0.82%) ⬆️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread objectstore-server/src/endpoints/common.rs
@jan-auer
jan-auer marked this pull request as draft July 15, 2026 13:45
@jan-auer
jan-auer marked this pull request as ready for review July 15, 2026 16:02

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ 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.

Comment thread objectstore-service/src/error.rs Outdated
Comment thread objectstore-service/src/error.rs Outdated
.map_err(|e| ApiError::Service(e.into()))?
.map_err(|e| {
objectstore_log::error!(!!&e, "failed to collect payload stream");
ApiError::Service(e.into())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we refactor ApiError similar to #547, we can directly propagate the IO error and no longer have to track it explicitly here.

@lcian lcian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the approaches we discussed seem to have their shortcomings, so I'm happy to go with the one you think is best.

@jan-auer
jan-auer merged commit 34dc02b into main Jul 16, 2026
26 checks passed
@jan-auer
jan-auer deleted the fix/service-error-double-capture branch July 16, 2026 11:19
lcian pushed a commit that referenced this pull request Jul 16, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants