Skip to content

chore(issues): Log warning instead of exception#101368

Merged
mrduncan merged 1 commit into
masterfrom
mrduncan/logger-warning
Oct 14, 2025
Merged

chore(issues): Log warning instead of exception#101368
mrduncan merged 1 commit into
masterfrom
mrduncan/logger-warning

Conversation

@mrduncan

Copy link
Copy Markdown
Member

We raise an exception when failing to parse a Snuba response, there is no need to also log an exception here since it otherwise results in 2 separate errors being logged for the same problem.

Fixes SENTRY-5AGR

See also SENTRY-5AGQ

We raise an exception when failing to parse a Snuba response, there is
no need to also log an exception here since it otherwise results in 2
separate errors being logged for the same problem.

Fixes SENTRY-5AGR
@sentry

sentry Bot commented Oct 10, 2025

Copy link
Copy Markdown
Contributor

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: src/sentry/utils/snuba.py

Function Unhandled Issue
_bulk_snuba_query SnubaError: HTTPConnectionPool(host='snuba-api', port=80): Read timed out. (read timeout=30) ...
Event Count: 3

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 10, 2025
@mrduncan mrduncan marked this pull request as ready for review October 10, 2025 19:38
@mrduncan mrduncan requested a review from a team as a code owner October 10, 2025 19:38
@mrduncan mrduncan requested review from a team October 10, 2025 19:38
Comment thread src/sentry/utils/snuba.py
except ValueError:
if response.status != 200:
logger.exception(
logger.warning(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Log Level Downgrade Masks Parsing Errors

Changing logger.exception() to logger.warning() for Snuba JSON parsing ValueErrors loses the original traceback and error details. This also downgrades the log level from ERROR to WARNING, potentially breaking monitoring and making these parsing failures much harder to debug.

Fix in Cursor Fix in Web

@mrduncan mrduncan enabled auto-merge (squash) October 10, 2025 19:58
@mrduncan mrduncan merged commit 9bc688b into master Oct 14, 2025
94 of 96 checks passed
@mrduncan mrduncan deleted the mrduncan/logger-warning branch October 14, 2025 22:54
@codecov

codecov Bot commented Oct 14, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/utils/snuba.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #101368      +/-   ##
===========================================
+ Coverage   80.86%    81.01%   +0.15%     
===========================================
  Files        8696      8696              
  Lines      385725    385736      +11     
  Branches    24381     24381              
===========================================
+ Hits       311898    312509     +611     
+ Misses      73459     72859     -600     
  Partials      368       368              

chromy pushed a commit that referenced this pull request Oct 17, 2025
We raise an exception when failing to parse a Snuba response, there is
no need to also log an exception here since it otherwise results in 2
separate errors being logged for the same problem.

Fixes SENTRY-5AGR

See also SENTRY-5AGQ
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants