Merge lp:~elachuni/ubuntu-webcatalog/amqp-oopses into lp:ubuntu-webcatalog

[Anthony Lenton]

Display the oops id on every error page, and generate oops reports in dev.

This branch includes several improvements related to ampq oops publishing:
 - A custom wsgi handler was added that generates an oops id for every exception.
 - Switched to oops-dictconfig 0.0.5 to use the 'inherit_id' and 'new_only' settings.
 - A custom runserver command was added that uses webcatalog's wsgi stack in dev (to generate oopses in dev)

[Łukasz Czyżykowski (lukasz-czyzykowski)]

Why there's render_html_error but no render_json_error (which should be extracted from __call__)?

[Łukasz Czyżykowski (lukasz-czyzykowski)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~elachuni/ubuntu-webcatalog/amqp-oopses into lp:ubuntu-webcatalog failed. Below is the output from the failed tests.

[localhost] local: /usr/bin/python /usr/bin/virtualenv --no-site-packages virtualenv
The --no-site-packages flag is deprecated; it is now the default behavior.
New python executable in virtualenv/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: virtualenv/bin/pip install -r test_requirements.txt
Downloading/unpacking django<1.4 (from -r test_requirements.txt (line 6))
  Running setup.py egg_info for package django

Downloading/unpacking coverage (from -r test_requirements.txt (line 7))
  Running setup.py egg_info for package coverage

    warning: no previously-included files matching '*.pyc' found anywhere in distribution
Downloading/unpacking mock (from -r test_requirements.txt (line 8))
  Running setup.py egg_info for package mock

    warning: no files found matching '*.png' under directory 'docs'
    warning: no files found matching '*.css' under directory 'docs'
    warning: no files found matching '*.html' under directory 'docs'
    warning: no files found matching '*.js' under directory 'docs'
Downloading/unpacking piston-mini-client (from -r test_requirements.txt (line 9))
  Downloading piston-mini-client-0.7.5.tar.gz
  Running setup.py egg_info for package piston-mini-client

Downloading/unpacking oauthlib (from piston-mini-client->-r test_requirements.txt (line 9))
  Running setup.py egg_info for package oauthlib

Downloading/unpacking httplib2 (from piston-mini-client->-r test_requirements.txt (line 9))
  Running setup.py egg_info for package httplib2

Installing collected packages: django, coverage, mock, piston-mini-client, oauthlib, httplib2
  Running setup.py install for django
    changing mode of build/scripts-2.7/django-admin.py from 644 to 755

    changing mode of /mnt/tarmac/cache/ubuntu-webcatalog/trunk/virtualenv/bin/django-admin.py to 755
  Running setup.py install for coverage
    building 'coverage.tracer' extension
    gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.7 -c coverage/tracer.c -o build/temp.linux-x86_64-2.7/coverage/tracer.o
    gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro build/temp.linux-x86_64-2.7/coverage/tracer.o -o build/lib.linux-x86_64-2.7/coverage/tracer.so

    warning: no previously-included files matching '*.pyc' found anywhere in distribution
    Installing coverage2 script to /mnt/tarmac/cache/ubuntu-webcatalog/trunk/virtualenv/bin
    Installing coverage-2.7 script to /mnt/tarmac/cache/ubuntu-webcatalog/trunk/virtualenv/bin
    Installing coverage script to /mnt/tarmac/cache/ubuntu-webcatalog/trunk/virtualenv/bin
  Running setup.py install for mock

    warning: no files found matching '*.png' under directory 'docs'
    warning: no files found matching '*.css' under directory ...

=== modified file '.bzrignore'
--- .bzrignore	2012-04-18 09:37:51 +0000
+++ .bzrignore	2013-03-01 12:52:22 +0000
@@ -15,3 +15,4 @@
 django_project/adminaudit
 django_project/local.cfg
 django_project/sreclient.py
+oopses/
 
=== modified file 'django_project/config/main.cfg'
--- django_project/config/main.cfg	2012-08-23 15:12:54 +0000
+++ django_project/config/main.cfg	2013-03-01 12:52:22 +0000
@@ -11,14 +11,14 @@
 debug = true
 media_root = django_project/media_root_dev/
 
-installed_apps = webcatalog
-                 django.contrib.contenttypes
+installed_apps = django.contrib.contenttypes
                  django.contrib.sessions
                  django.contrib.sites
                  django.contrib.messages
                  django.contrib.staticfiles
                  django.contrib.markup
                  django.contrib.admin
+                 webcatalog
                  django_openid_auth
                  django_configglue
                  django.contrib.auth
@@ -106,16 +106,6 @@
 canonical-losas = admin
 canonical-ca-hackers = developers
 
-[oops_wsgi]
-oopses = oops_publishers
-
-[oops_publishers] 
-publishers = oops_datedir_publisher
-
-[oops_datedir_publisher]
-type = datedir
-error_dir = oopses
-
 [webcatalog]
 serve_site_media = True
 sca_api_url = https://sc.staging.ubuntu.com/api/2.0/
 
=== modified file 'django_project/urls.py'
--- django_project/urls.py	2012-06-21 20:18:44 +0000
+++ django_project/urls.py	2013-03-01 12:52:22 +0000
@@ -25,6 +25,10 @@
 admin.autodiscover()
 preflight.autodiscover()
 
+handler500 = 'webcatalog.views.server_error'
+handler404 = 'webcatalog.views.page_not_found'
+
+
 urlpatterns = patterns('',
     url(r'^cat/', include('webcatalog.urls')),
     url(r'^admin/', include(admin.site.urls)),
 
=== modified file 'setup.py'
--- setup.py	2012-09-28 15:02:24 +0000
+++ setup.py	2013-03-01 12:52:22 +0000
@@ -53,7 +53,7 @@
         'celery==2.5.0',
         'django-celery==2.5.0',
         'oops-wsgi==0.0.10',
-        'oops-dictconfig==0.0.2',
+        'oops-dictconfig==0.0.5',
         'oops-datedir_repo==0.0.17',
         ],
     package_data = find_packages_data('src'),
 
=== added file 'src/webcatalog/management/commands/runserver.py'
--- src/webcatalog/management/commands/runserver.py	1970-01-01 00:00:00 +0000
+++ src/webcatalog/management/commands/runserver.py	2013-03-01 12:52:22 +0000
@@ -0,0 +1,36 @@
+#   -*- coding: utf-8 -*-
+#   This file is part of the Apps Directory
+#   Copyright (C) 2011-2013 Canonical Ltd.
+#
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU Affero General Public License as
+#   published by the Free Software Foundation, either version 3 of the
+#   License, or (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU Affero General Public License for more details.
+#
+#   You should have received a copy of the GNU Affero General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Customized runserver command that runs ubuntu-webcatalog's wsgi stack."""
+
+from optparse import make_option
+from django.core.management.commands.runserver import BaseRunserverCommand
+from django.core.servers.basehttp import AdminMediaHandler
+
+
+class Command(BaseRunserverCommand):
+    option_list = BaseRunserverCommand.option_list + (
+        make_option(
+            '--adminmedia', dest='admin_media_path', default='',
+            help='Specifies the directory from which to serve admin media.'),
+    )
+
+    def get_handler(self, *args, **options):
+        """Serves admin media like old-school (deprecation pending)."""
+        from webcatalog.wsgi import make_app
+        handler = make_app()
+        return AdminMediaHandler(handler, options.get('admin_media_path', ''))
 
=== modified file 'src/webcatalog/schema.py'
--- src/webcatalog/schema.py	2013-02-22 21:02:05 +0000
+++ src/webcatalog/schema.py	2013-03-01 12:52:22 +0000
@@ -38,6 +38,8 @@
             'publishers': [{
                 'type': 'datedir',
                 'error_dir': 'oopses',
+                'inherit_id': True,
+                'new_only': False,
             }],
         })
 
 
=== modified file 'src/webcatalog/templates/500.html'
--- src/webcatalog/templates/500.html	2012-08-23 14:59:18 +0000
+++ src/webcatalog/templates/500.html	2013-03-01 12:52:22 +0000
@@ -1,10 +1,11 @@
-{% extends "light/index.1col.html" %}
+{% extends "webcatalog/base.html" %}
 {% load i18n %}
 
 {% block title %}{% trans 'Oops!' %}{% endblock %}
 
 {% block content %}
-<h1>{% trans 'Server Error <em>(500)</em>' %}</h1>
+<h1>{% trans 'Server Error' %}</h1>
 {% if errormsg %}<p>{{ errormsg }}</p>{% endif %}
 <p>{% blocktrans %}An error occurred which prevented the page you requested from loading. If this problem continues, please consider reporting this problem by sending an e-mail to <a href="mailto:[e-mail]">[e-mail]</a>{% endblocktrans %}</p>
+<p id="oops-id">{{ oops_id }}</p>
 {% endblock %}
 
=== modified file 'src/webcatalog/tests/test_oops_wsgi_config.py'
--- src/webcatalog/tests/test_oops_wsgi_config.py	2012-08-23 14:59:18 +0000
+++ src/webcatalog/tests/test_oops_wsgi_config.py	2013-03-01 12:52:22 +0000
@@ -26,6 +26,7 @@
 
 import bson
 import os
+import re
 from shutil import rmtree
 from tempfile import mkdtemp
 
@@ -67,13 +68,15 @@
 
     def request_wsgi_response(self, environ):
         start_response = Mock()
-        with patch_settings(OOPSES={
+        oops_config = {
             'publishers': [{
                 'type': 'datedir',
                 'error_dir': self.oops_dir,
-                'instance_id': 'dev',
+                'inherit_id': True,
+                'new_only': False,
             }],
-        }):
+        }
+        with patch_settings(OOPSES=oops_config):
             app = make_app()
             response = app(environ, start_response).next()
         return response
@@ -105,6 +108,11 @@
     def test_oops_on_disk_on_error(self):
         response = self.request_with_exception()
 
-        self.assertIn('Server Error <em>(500)</em>', response)
+        self.assertIn('Server Error', response)
+        oops_id_re = re.compile('<p id="oops-id">(OOPS-[0-9a-f]{32})</p>')
+        match = oops_id_re.search(response)
+        self.assertTrue(match)
+        oops_id = match.groups()[0]
         oops_data = self.read_oops_from_disk()
         self.assertEqual('ZeroDivisionError', oops_data['type'])
+        self.assertEqual(oops_id, oops_data['id'])
 
=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py	2012-07-02 21:25:30 +0000
+++ src/webcatalog/urls.py	2013-03-01 12:52:22 +0000
@@ -63,4 +63,5 @@
         name='wc-task-status'),
 
     (r'^api/', include('webcatalog.api.urls')),
+    url(r'^die/$', 'die', name='debug-die'),
 )
 
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2012-09-06 10:38:18 +0000
+++ src/webcatalog/views.py	2013-03-01 12:52:22 +0000
@@ -34,7 +34,6 @@
 from django.http import (
     HttpResponse,
     HttpResponseForbidden,
-    HttpResponseNotFound,
     HttpResponseRedirect,
 )
 from django.shortcuts import (
@@ -42,7 +41,10 @@
     render_to_response,
 )
 from django.template import RequestContext
-from django.template.loader import render_to_string
+from django.template.loader import (
+    render_to_string,
+    get_template,
+)
 from django.utils.translation import ugettext as _
 from django.views.decorators.cache import never_cache
 from django.views.decorators.http import require_GET
@@ -346,3 +348,57 @@
 def forbidden(request):
     return HttpResponseForbidden(
         render_to_string('forbidden.html', RequestContext(request)))
+
+
+class ErrorPage(object):
+    def __init__(self, status, errormsg=None):
+        self.template_name = '%s.html' % status
+        self.status_code = status
+        self.errormsg = errormsg
+
+    def __call__(self, request):
+        accept = request.META.get('ACCEPT', '')
+        if 'application/json' in accept and not 'text/html' in accept:
+            return self.render_json_error(request)
+        else:
+            return self.render_html_error(request)
+
+    def render_html_error(self, request):
+        oops_id = self.get_oops_id(request)
+        template = get_template(self.template_name)
+        atts = {
+            'oops_id': oops_id,
+            'errormsg': self.errormsg,
+        }
+        context = RequestContext(request, atts)
+        return HttpResponse(template.render(context), status=self.status_code)
+
+    def render_json_error(self, request):
+        oops_id = self.get_oops_id(request)
+        oops_context = request.META.get('oops.context', {})
+        exc_info = oops_context.get('exc_info', (None, None, None))
+        exception_type = unicode(exc_info[0])
+        response = json.dumps({
+            'oops_id': oops_id,
+            'error_type': exception_type,
+            'error_msg': "An error occurred and has been logged.",
+        })
+        return HttpResponse(response, status=self.status_code,
+                            content_type='application/json')
+
+    def get_oops_id(self, request):
+        oops_report = request.environ.get('oops.report', {})
+        return oops_report.get('id', 'OOPS ID not available.')
+
+
+server_error = ErrorPage(500)
+page_not_found = ErrorPage(404)
+
+
+class ArtificialOopsException(Exception):
+    pass
+
+
+def die(request):
+    """Just trigger an oops report"""
+    raise ArtificialOopsException()
 
=== modified file 'src/webcatalog/wsgi.py'
--- src/webcatalog/wsgi.py	2012-08-23 14:59:18 +0000
+++ src/webcatalog/wsgi.py	2013-03-01 12:52:22 +0000
@@ -23,8 +23,40 @@
 import oops_wsgi.django
 import os
 import platform
+import uuid
 
 from django.conf import settings
+from django.core.handlers import wsgi
+
+
+class EagerOOPSWSGIHandler(wsgi.WSGIHandler):
+    """Pre-generates an oops id on exception."""
+    def handle_uncaught_exception(self, request, resolver, exc_info):
+        # This will pass the exception information back to oops.  It
+        # should not be necessary once
+        # https://code.djangoproject.com/ticket/16674 has been merged.
+        if 'oops.context' in request.environ:
+            request.environ['oops.context']['exc_info'] = exc_info
+        # Generate OOPS id early so that we can render the error page right
+        # away and not depend on passing thread locals around.
+        if 'oops.report' in request.environ:
+            unique_id = uuid.uuid4().hex
+            request.environ['oops.report']['id'] = "OOPS-%s" % unique_id
+
+        return super(EagerOOPSWSGIHandler, self).handle_uncaught_exception(
+            request, resolver, exc_info)
+
+    def __call__(self, environ, start_response):
+        def start_response_with_exc_info(status, headers, exc_info=None):
+            """Custom start_response callback for wsgi."""
+            # This will pass the exception information back to oops_wgi.  It
+            # should not be necessary once
+            # https://code.djangoproject.com/ticket/16674 has been merged.
+            if exc_info is None:
+                exc_info = environ['oops.context'].get('exc_info', None)
+            return start_response(status, headers, exc_info)
+        return super(EagerOOPSWSGIHandler, self).__call__(
+            environ, start_response_with_exc_info)
 
 
 def make_app():
@@ -38,9 +70,7 @@
     if logging_config:
         logging.config.fileConfig(logging_config)
 
-    # This is needed to workaround a bug in Django, see the file it is
-    # implemented in for more details.
-    non_oops_app = oops_wsgi.django.OOPSWSGIHandler()
+    non_oops_app = EagerOOPSWSGIHandler()
 
     config = oops_dictconfig.config_from_dict(settings.OOPSES)
     oops_wsgi.install_hooks(config)