Skip to content
This repository has been archived by the owner on Jan 31, 2018. It is now read-only.

[bug 1136840] Fix error handling for better debugging: WIP #509

Merged
merged 3 commits into from
Mar 4, 2015
Merged

[bug 1136840] Fix error handling for better debugging: WIP #509

merged 3 commits into from
Mar 4, 2015

Conversation

willkg
Copy link
Member

@willkg willkg commented Mar 4, 2015

  • creates DebuggableWSGIHandler which should send more useful error
    emails so I can debug API-related problems, plus in DEBUG=True
    mode, it'll return text/plain content rather than text/html content
    which is a lot easier to read in a terminal when debugging API-related
    problems
  • created fjord/wsgi.py which is where the wsgi stuff should be--we'll
    have to get the servers fixed to use this instead of wsgi/playdoh.wsgi
    if this works
  • for now, wsgi/playdoh.wsgi and fjord/wsgi.py should be identical--the
    former is used in production and the latter is used in development,
    but should be used in production

r?

@willkg willkg changed the title [bug 1136840] Fix error handling for better debugging [bug 1136840] Fix error handling for better debugging: WIP Mar 4, 2015
@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

Since I added WSGI_APPLICATION to settings, doing ./manage.py runserver and ./manage.py test will use fjord/wsgi.py to build the WSGI application. So running the tests will give us some confidence that the wsgi.py file is at least "mostly correct".

However wsgi/playdoh.wsgi which runs on stage/prod does some things that will mess up the contributor environment. I still need to figure out how to deal with that. Maybe I'll do something goofy like have wsgi/playdoh.wsgi do the os.environ stuff and then call out to fjord/wsgi.py to do the rest.

The tests don't kick up any exceptions, so there's nothing going through that new section of code at the moment. I think I can fix that by creating a view that errors out and then triggering it and then testing the Accept logic.

Anyhow, this is a WIP. I'm curious to see if the approach is daft or not. I also want to see if it passes Travis tests.

@mythmon @rlr Does this seem daft?

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

^^^ That fixes wsgi/playdoh.wsgi and fjord/wsgi.py so we keep them both and wsgi/playdoh.wsgi deals with the server-environment-specific things. This shouldn't hose contributor environments plus it doesn't require us to do anything to stage/prod environments.

I tried to add a test that verifies the Accept handling, but it appears the tests are using a different WSGI mechanism. I might be able to do this with a LiveServerTestCase, but that seems like a lot of trouble.

It's easy to test manually.

  1. Run ./manage.py runserver
  2. Go to http://localhost:8000/services/throw-error and see glorious HTML
  3. Run this shell script and get text:
#!/bin/bash
HOST='http://localhost:8000'

curl -v -XGET "$HOST/services/throw-error" \
     -H 'Accept: application/json'

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

^^^ That cleans some things up and reduces some of the differences.

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

I spent some time trying to get LiveServerTestCase working, but it refuses to be ok with DEBUG=True, so I can't test the error handling automatedly.

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

For the record, I don't like parts of this PR, but can't figure out better ways to do them and I need this (or something equivalent that lets me see POST payloads) asap so I can fix HB issues.


if newrelic:
application = newrelic.agent.wsgi_application()(application)

# vim: ft=python
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you hating on vim? </troll>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been removing these emacs/vim metadata things where I see them.

@rlr
Copy link
Contributor

rlr commented Mar 4, 2015

Slightly related, it would be cool if that error page was CSRF exempt... So I could do this $ curl -v -XPOST "http://0.0.0.0:8000/services/throw-error" -H 'Accept: application/json' -d "this is the body"

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

I can make that view csrf-exempt. That's a good idea.

@rlr
Copy link
Contributor

rlr commented Mar 4, 2015

cool. Time to try it on stage?

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

^^^ Added the csrf_exempt thing.

@rlr
Copy link
Contributor

rlr commented Mar 4, 2015

I see HTTP_X_POST_BODY = 'this is the body' now. YAY!

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

Ok. After Travis gives me the green light, I'll squish everything down, double-check the code, push it to upstream in a branch and test it out on stage.

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

^ Squashed commits and redid the commit message.

I'm pushing this to stage to test it out now.

@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

I pushed it to stage and tested it out. I tried a few "bad post data" attempts, too, and they all seem fine.

I mostly just moved some things around so they're in places that are
more logical.
* creates DebuggableWSGIHandler which should send more useful error
  emails so I can debug API-related problems, plus in DEBUG=True
  mode, it'll return plain formatted content rather than html formatted
  content; plain text is a lot easier to read in a terminal when
  debugging API-related problems
* created fjord/wsgi.py which gets used by "./manage.py runserver"
  and holds common WSGI setup for fjord
* adjusts wsgi/playdoh.wsgi which gets used by stage/prod environments
  to do stage/prod specific things and then use fjord/wsgi.py for
  the rest
willkg added a commit that referenced this pull request Mar 4, 2015
[bug 1136840] Fix error handling for better debugging: WIP
@willkg willkg merged commit 55bbcea into mozilla:master Mar 4, 2015
@willkg
Copy link
Member Author

willkg commented Mar 4, 2015

Landed in ceb53eb [bug 1136840] Fix error handling for better debugging

Thank you!

@willkg willkg deleted the 1136840-error-emails branch March 5, 2015 22:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants