launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/404-bug-api-generates-oops-701246 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #701246 in Launchpad itself: "missing bug dereference generates OOPS"
  https://bugs.launchpad.net/launchpad/+bug/701246

For more details, see:
https://code.launchpad.net/~deryck/launchpad/404-bug-api-generates-oops-701246/+merge/61407

This branch ensures that the webservice doesn't generate an OOPS when a bug is not found.  Actually, it ensures that we don't generate an OOPS when any resource is not found.  We report a 404, but not an OOPS.  Using launchpadlib, the user will get a KeyError.

I need a bit more test coverage, so marking this WIP for now, and will updathe MP and this description soon.
-- 
https://code.launchpad.net/~deryck/launchpad/404-bug-api-generates-oops-701246/+merge/61407
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/404-bug-api-generates-oops-701246 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2010-12-13 18:04:24 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2011-05-18 13:51:08 +0000
@@ -410,7 +410,7 @@
 
             if WebServiceLayer.providedBy(request):
                 webservice_error = getattr(
-                    info[0], '__lazr_webservice_error__', 500)
+                    info[1], '__lazr_webservice_error__', 500)
                 if webservice_error / 100 != 5:
                     request.oopsid = None
                     # Return so the OOPS is not generated.

=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py	2011-04-21 01:54:33 +0000
+++ lib/canonical/launchpad/webapp/publisher.py	2011-05-18 13:51:08 +0000
@@ -51,6 +51,8 @@
     )
 from zope.traversing.browser.interfaces import IAbsoluteURL
 
+from lazr.restful.error import expose
+
 from canonical.launchpad.layers import (
     LaunchpadLayer,
     setFirstLayer,
@@ -691,7 +693,7 @@
         """
         # Avoid circular imports.
         if nextobj is None:
-            raise NotFound(self.context, name)
+            raise expose(NotFound(self.context, name), 404)
         elif isinstance(nextobj, redirection):
             return RedirectionView(
                 nextobj.toname, request, status=nextobj.status)

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt	2011-04-12 11:36:11 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt	2011-05-18 13:51:08 +0000
@@ -1195,14 +1195,16 @@
   Location: http://.../bugs/bugtrackers/bob
   ...
 
-Note the 301 response. We changed the name, so the API URL at which
+Note the 301 response above. We changed the name, so the API URL at which
 the bug tracker can be found has changed.
 
+Now notice that bug trackers (and bugs too) that are not found generate
+a 404 error, but do not generate an OOPS.
+
   >>> print webservice.get(bug_tracker['self_link'])
   HTTP/1.1 404 Not Found...
   Content-Length: ...
   ...
-  X-Lazr-Oopsid: OOPS-...
   <BLANKLINE>
   Object: <...BugTrackerSet object at ...>, name: u'mozilla.org'