Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(starlette): Fix failed_request_status_codes=[] #3561

Merged

Conversation

szokeasaurusrex
Copy link
Member

Passing an empty list for failed_request_status_codes should result in no status codes resulting in a Sentry error. However, right now, setting failed_request_status_codes=[] instead yields the default failed_request_status_codes of range(500, 599). This change fixes the incorrect behavior and adds tests to verify the fix.

Depends on #3560

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.31%. Comparing base (5c6c778) to head (09c6f2a).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3561   +/-   ##
=======================================
  Coverage   84.30%   84.31%           
=======================================
  Files         133      133           
  Lines       13890    13890           
  Branches     2930     2930           
=======================================
+ Hits        11710    11711    +1     
- Misses       1440     1442    +2     
+ Partials      740      737    -3     
Files with missing lines Coverage Δ
sentry_sdk/integrations/starlette.py 84.47% <100.00%> (ø)

... and 3 files with indirect coverage changes

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/remove-invalid-tests branch from 8402895 to 2afe13a Compare September 24, 2024 11:28
…3560)

The Starlette integration tests (as well as the FastAPI integration tests, which hit the same code path as the Starlette integration) include a test where the integrations' `failed_request_status_codes` parameter is set to `[None]`. However, since the parameter is typed as `Optional[list[HttpStatusCodeRange]]`, where `HttpStatusCodeRange = Union[int, Container[int]]`, passing `[None]` for this parameter should not be allowed, per the type hint. Thus, we should not test this input, since the behavior of passing `[None]` is not, and should not be, defined by the API.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/remove-invalid-tests branch from 2afe13a to ccdbffb Compare September 24, 2024 11:31
szokeasaurusrex added a commit that referenced this pull request Sep 24, 2024
Passing an empty list for `failed_request_status_codes` should result in no status codes resulting in a Sentry error. However, right now, setting `failed_request_status_codes=[]` instead yields the default `failed_request_status_codes` of `range(500, 599)`. This change fixes the incorrect behavior and adds tests to verify the fix.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix-failed_request_status_codes branch from ef76732 to 7ddada4 Compare September 24, 2024 11:31
Passing an empty list for `failed_request_status_codes` should result in no status codes resulting in a Sentry error. However, right now, setting `failed_request_status_codes=[]` instead yields the default `failed_request_status_codes` of `range(500, 599)`. This change fixes the incorrect behavior and adds tests to verify the fix.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix-failed_request_status_codes branch from 7ddada4 to 09c6f2a Compare September 24, 2024 11:39
@szokeasaurusrex szokeasaurusrex changed the base branch from szokeasaurusrex/remove-invalid-tests to master September 24, 2024 12:12
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix-failed_request_status_codes branch from 2352b53 to 09c6f2a Compare September 24, 2024 12:13
@szokeasaurusrex szokeasaurusrex merged commit 09c6f2a into master Sep 24, 2024
244 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/fix-failed_request_status_codes branch September 24, 2024 12:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants