Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Falcon integration does not respect custom exception handlers #1362

Closed
alexmic opened this issue Mar 7, 2022 · 4 comments · Fixed by #2465
Closed

Falcon integration does not respect custom exception handlers #1362

alexmic opened this issue Mar 7, 2022 · 4 comments · Fixed by #2465
Assignees
Labels
Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! Integration: Falcon Triaged Has been looked at recently during old issue triage

Comments

@alexmic
Copy link

alexmic commented Mar 7, 2022

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.5.6

Steps to Reproduce

  1. Attach a custom exception handler:
import marshmallow

def marshmallow_validation_error_handler(req, res, exc, params):
    """Transforms a marshmallow validation error into the appropriate HTTP response."""
    raise falcon.HTTPError(400)

api.add_error_handler(marshmallow.ValidationError, marshmallow_validation_error_handler)
  1. Do an API request that leads to a validation error

Expected Result

The marshmallow exception should not have been reported to Sentry.

Actual Result

The custom exception leads to a 4xx, not a 5xx, yet it is still reported as an unhandled error on Sentry.

@sl0thentr0py
Copy link
Member

thx for reporting @alexmic, if you want it fixed sooner, PRs always welcome!

@sentrivana sentrivana added Triaged Has been looked at recently during old issue triage Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! labels Oct 23, 2023
@khuongduy354
Copy link

I have a question,
image
In the docs, Sentry detect if Falcon is in dependencies, I've been looking for the code doing that but haven't found it yet.
At first, I guess the dependencies detection code is inside the init method
image
But after reading class Client and Hub, these just takes integration options parameters, and doesn't do dependencies scan.
image
Can you show me where that part is ?

@sentrivana
Copy link
Contributor

sentrivana commented Oct 25, 2023

Hey @khuongduy354, you're looking for the setup_integrations function which is called at client setup here:

self.integrations = setup_integrations(
These are the specific parts responsible for the detection:
for import_string in all_import_strings:
try:
module, cls = import_string.rsplit(".", 1)
yield getattr(import_module(module), cls)
except (DidNotEnable, SyntaxError) as e:
logger.debug(
"Did not import default integration %s: %s", import_string, e
)
and
except DidNotEnable as e:
(An integration raises a special DidNotEnable exception if it fails to import the instrumented library and that tells this code that the library is not installed and the integration for it shouldn't be enabled.)

But please note that someone's already working on this issue.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 Oct 25, 2023
szokeasaurusrex added a commit that referenced this issue Oct 25, 2023
…eporting error (#2465)

* Falcon checks actual HTTP status before reporting error

* Only support custom error handlers on Falcon 3+

* Add Falcon 3.1 to tox.ini

This change fixes an issue where the Falcon integration would report an error occurring in a Falcon request handler to Sentry, even though a Falcon custom event handler was handling the exception, causing an HTTP status other than 5xx to be returned. From now on, Falcon will inspect the HTTP status on the response before sending the associated error event to Sentry, and the error will only be reported if the response status is a 5xx status.

Fixes GH-#1362
@szokeasaurusrex
Copy link
Member

We have fixed this issue for Falcon versions 3.0 and above only. In case anyone encounters this same bug in an older Falcon version, please feel free to submit a new issue, referencing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! Integration: Falcon Triaged Has been looked at recently during old issue triage
Projects
Archived in project
8 participants