Closed Bug 1136840 Opened 10 years ago Closed 8 years ago

error emails from API posts with JSON content get "could not parse"

Categories

(Input Graveyard :: Backend, defect, P2)

Product:
Input Graveyard
Component:
Backend
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: willkg, Unassigned)

Details

(Whiteboard: u=dev c=api p= s=)

Error emails from HTTP 500 problems with API views have this in them: POST:<could not parse>, That's not helpful. I'm pretty sure that it's when the API post has JSON formatted payload. This bug covers verifying that and then figuring out what to do about it. Maybe we should write up a different HTTP 500 handler that parses the POST data more intelligently? Maybe DRF has a handler that's better for these situations?
Mike: Do you have any ideas on this?
Flags: needinfo?(mcooper)
I don't think I have any ideas here. I haven't noticed this on SUMO before, but I poked around in Errormill and maybe SUMO has the same issue. I'm curious what you come up with, but I don't know what might be up.
Flags: needinfo?(mcooper)
Working on this now. I'm updating Harold and will test it out on Heroku to figure out what's going on and then figure out what I want to do to fix it.
Assignee: nobody → willkg
Status: NEW → ASSIGNED
I set up Harold similarly to my theoretical situation and end up with this: <WSGIRequest path:/api/v1/feedback/, GET:<QueryDict: {}>, POST:<QueryDict: {}>, COOKIES:{}, META:{'CONTENT_LENGTH': '46', 'CONTENT_TYPE': 'application/json', 'HTTP_ACCEPT': 'application/json; indent=4', The POST is an empty querydict, but the content_length is 46. That's different than the problem scenario here. However, it'd be nice to have the JSON data for figuring out API problems, so I'm going to lump fixing this in with this bug, too.
More headers from the problem situation in the description: <WSGIRequest path:/api/v1/feedback/, GET:<QueryDict: {}>, POST:<could not parse>, COOKIES:{}, META:{'CONTENT_LENGTH': '330', 'CONTENT_TYPE': 'application/json; charset=UTF-8', ... 'HTTP_USER_AGENT': 'Mozilla/5.0 (Android; Mobile; rv:35.0) Gecko/35.0 Firefox/35.0', Maybe all the problems are coming from the Android in-product feedback form? Maybe there are unicode characters in the data?
WOW! This is trickier than I thought it would be. Let's break down the issue into several specific requirements (with some additional related ones): 1. I want to see the raw HTTP POST payload in the error emails 2. When doing development with DEBUG=True, I want to see debugging information in text rather than HTML when debugging API endpoints 3. When triggering an HTTP 404, 500, etc with "Accept: application/json" header, the response should be in JSON I can fix items 1 and 2 by creating a Handler mixin that overrides handle_uncaught_exception, but it's kind of hacky and it's mostly a copy/paste of Django code with some tweaks to make things "work better". Further the fix for item 1 involves adding the raw POST payload data to the HTTP_X_POSTDATA META because otherwise I have to rewrite/override a lot of Django code so that either it shows another part of the request or it shows more of the extra data. Very painful--I'm game for other ideas. Item 3 can probably be fixed by overriding the various handle* views in the URL conf file: https://docs.djangoproject.com/en/1.7/topics/http/views/#customizing-error-views I'm tossing around writing a library for this.
I threw together a prototype library for this: https://github.com/willkg/django-godfrey I'm asking around to see if there are better ways to do it. I really hope so.
Redid the WSGIHandler mixin code that I wrote in django-godfrey and added it to fjord. This is a little less "override all the django things" and more "tweak the django things" which is probably good. However, it still does the "let's shove something in META since we know that gets tossed in the repr of WSGIRequest" thing which is goofy. In a PR: https://github.com/mozilla/fjord/pull/509
cc:ing Gregg because we need to fix this bug before I can look into his hb woes.
Landed in https://github.com/mozilla/fjord/commit/ceb53eb348c775b3b996dc0e2514c6bb23011eaa I did some extensive testing on stage and everything looks ok (which is somewhat of a relief). I tested sending "bad data" payloads, too, in utf-8 and utf-16 and the postbody handling does what I expected with both. So I think we're good to push to prod. This is easy to back out, so if there are problems I'll know fast and can back it out quickly.
Pushed it to prod. Monitoring. I'll mark this FIXED once I get some helpful data from HB errors. Otherwise I'll try another way to fix this.
I can see the changes the new WSGIHandler mixin implemented, but I stopped getting HB errors. Either that's a fluke or I screwed something up. I'm going to back out the changes for now and see if I start getting HB errors again. Backing out the new handler in PR: https://github.com/mozilla/fjord/pull/519
Just got an HB error email. So I'll fully back out that WSGIHandler stuff and try another approach. :(
Backed out the rest of the WSGIHandler mixing. In a PR: https://github.com/mozilla/fjord/pull/521
Landed in master: https://github.com/mozilla/fjord/commit/d51873135a52edbbf55b384322209400778da6cf Will deploy soon. After that, I have to mull over this bug, so I'm probably going to put it on the back burner for a while.
Pushed to prod just now. Going to table this for another day when I have time to think about it more.
Bumping this out of the quarter. I'll look into it if it becomes an issue again.
Assignee: willkg → nobody
Whiteboard: u=dev c=api p= s=input.2015q1 → u=dev c=api p= s=
It's possible this becomes moot if we switch to Sentry. That's covered in bug bug #1200370.
The Input service has been decommissioned (see bug 1315316) and has been replaced by a redirect to an external vendor (SurveyGizmo). I'm bulk WONTFIXing Input bugs that do not appear to be relevant anymore.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Product: Input → Input Graveyard
You need to log in before you can comment on or make changes to this bug.