Skip to content

Conversation

@alex-hofsteede
Copy link
Contributor

@alex-hofsteede alex-hofsteede commented Dec 15, 2017

Log messages from Django
contain a request object which is not JSON serializable. The regular
Raven Client has a pretty bulletproof JSON encoder which handles these
objects, but SentryInternalClient didn't use it, so there is an
exception thrown further down the stack in insert_data_to_database
when it comes across these objects. This fix forces the Raven JSON
encoding of the event.

This is not a very efficient solution, but it is at least simple.

@alex-hofsteede alex-hofsteede requested review from a team and dcramer December 15, 2017 01:42
@alex-hofsteede
Copy link
Contributor Author

I agree with @dcramer s comment at the top of the function that these messages should just use the regular pipeline, but I'm not 100% sure what's involved in making that change.

@mattrobenolt
Copy link
Contributor

I agree with dcramer s comment at the top of the function that these messages should just use the regular pipeline, but I'm not 100% sure what's involved in making that change.

Also, yeah, I'd be down to explore this after to figure this out. Right now, we have a few downsides with current implementation that going through this would be useful.

@alex-hofsteede
Copy link
Contributor Author

@mattrobenolt thanks. I could ship #6785 instead. LMK if you have any thoughts on that, otherwise I will just merge this in the meantime

[Log messages from Django](https://docs.djangoproject.com/en/2.0/topics/logging/#django-request)
contain a `request` object which is not JSON serializable. The regular
Raven `Client` has a pretty bulletproof JSON encoder which handles these
objects, but SentryInternalClient didn't use it, so there is an
exception thrown further down the stack in `insert_data_to_database`
when it comes across these objects. This fix forces the Raven JSON
encoding of the event.

This is not a very efficient solution, but it is at least simple.

 Untracked files:
@alex-hofsteede alex-hofsteede force-pushed the alex/internal-client-fix branch from 44d424d to 792828f Compare December 19, 2017 00:31
@alex-hofsteede alex-hofsteede merged commit 78243aa into master Dec 19, 2017
@alex-hofsteede alex-hofsteede deleted the alex/internal-client-fix branch January 9, 2018 23:44
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants