getsentry / raven-python

Raven is the legacy Python client for Sentry (getsentry.com) — replaced by sentry-python
https://sentry.io
BSD 3-Clause "New" or "Revised" License
1.68k stars 657 forks source link

user_context is overwritten when Sentry(app) called after @before_request #842

Open iffy opened 8 years ago

iffy commented 8 years ago

If you initialize raven.contrib.flask.Sentry after a Flask before_request has been defined (in which sentry.user_context is used), the user context is destroyed when sending errors.

For instance, in the code below the user context sent to Sentry only has an ip address. But if you uncomment "Option A" and comment out the "Option B", the user context contains email=bob@bob.com (and the other items).

from flask import Flask
from raven.contrib.flask import Sentry

app = Flask(__name__)

# Option A
# sentry = Sentry(app)
# message = 'Option A'

@app.before_request
def before_request():
    sentry.user_context({
        'ip_address': '1.2.3.4',
        'email': 'bob@bob.com',
        'username': 'someguy',
        'id': '1238923',
    })
    sentry.extra_context({
        'goo': 'bar',
        'ip_address': '1.2.3.4',
        'email': 'bob@bob.com',
        'username': 'someguy',
        'id': '1238923',
    })

# Option B
sentry = Sentry(app)
message = 'Option B'

@app.route('/')
def index():
    raise Exception(message)

app.run(debug=True)

I think it's because get_user_info in this spot returns None and merging None overwrites existing values:

>>> from raven.context import Context
>>> c = Context()
>>> c.merge({'user': {'foo': 'bar'}})
>>> c
<Context: {'user': {'foo': 'bar'}}>
>>> c.merge({'user': None})
>>> c
<Context: {'user': None}>

So maybe the fix is to have Context.merge treat Nones as {}. Or maybe get_user_info should return {} instead of None.

Or maybe this:

diff --git a/raven/contrib/flask.py b/raven/contrib/flask.py
index a973344..6014c2c 100644
--- a/raven/contrib/flask.py
+++ b/raven/contrib/flask.py
@@ -221,7 +221,7 @@ class Sentry(object):
         except Exception as e:
             self.client.logger.exception(to_unicode(e))
         try:
-            self.client.user_context(self.get_user_info(request))
+            self.client.user_context(self.get_user_info(request) or {})
         except Exception as e:
             self.client.logger.exception(to_unicode(e))
iffy commented 8 years ago

Any update on this?

iffy commented 7 years ago

Is my suggested fix not a good one? Or are you waiting for me to file a PR?

mitsuhiko commented 7 years ago

I am not sure if that is actually the correct fix here. For the moment I think it makes sense to put this into the docs (that you need to initialize this early together with the rest of the app).