Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for bug id 1281459 which propagate errors with status 500 to client #194

Merged
merged 4 commits into from Dec 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 9 additions & 12 deletions auslib/test/web/test_client.py
Expand Up @@ -906,35 +906,32 @@ def testCacheControlIsNotSetFor404(self):
ret = self.client.get('/whizzybang')
self.assertEqual(ret.headers.get("Cache-Control"), None)

def testCacheControlIsNotSetFor500(self):
with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
m.side_effect = Exception('I break!')
ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
self.assertEqual(ret.headers.get("Cache-Control"), None)

def testEmptySnippetOn404(self):
ret = self.client.get('/whizzybang')
self.assertUpdatesAreEmpty(ret)

def testEmptySnippetOn500(self):
with mock.patch('auslib.web.views.client.ClientRequestView.get') as m:
m.side_effect = Exception('I break!')
ret = self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
self.assertUpdatesAreEmpty(ret)
with self.assertRaises(Exception) as exc:
self.client.get('/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml')
self.assertEqual('I break!', str(exc.exception.message))

def testSentryBadDataError(self):
with mock.patch("auslib.web.views.client.ClientRequestView.get") as m:
m.side_effect = BadDataError("exterminate!")
with mock.patch("auslib.web.base.sentry") as sentry:
with mock.patch("auslib.web.base.sentry") as sentry, self.assertRaises(Exception) as exc:
self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
self.assertFalse(sentry.captureException.called)
self.assertFalse(sentry.captureException.called)
self.assertEqual('exterminate!', str(exc.exception.message))

def testSentryRealError(self):
with mock.patch("auslib.web.views.client.ClientRequestView.get") as m:
m.side_effect = Exception("exterminate!")
with mock.patch("auslib.web.base.sentry") as sentry:
with mock.patch("auslib.web.base.sentry") as sentry, self.assertRaises(Exception) as exc:
self.client.get("/update/4/b/1.0/1/p/l/a/a/a/a/1/update.xml")
self.assertTrue(sentry.captureException.called)
self.assertTrue(sentry.captureException.called)
self.assertEqual('exterminate!', str(exc.exception.message))

def testNonSubstitutedUrlVariablesReturnEmptyUpdate(self):
request1 = '/update/1/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/update.xml'
Expand Down
17 changes: 3 additions & 14 deletions auslib/web/base.py
@@ -1,7 +1,7 @@
import logging
log = logging.getLogger(__name__)

from flask import Flask, make_response, send_from_directory, request, abort
from flask import Flask, make_response, send_from_directory, abort

from raven.contrib.flask import Sentry

Expand Down Expand Up @@ -38,24 +38,13 @@ def fourohfour(error):
def generic(error):
"""Deals with any unhandled exceptions. If the exception is not a
BadDataError, it will be sent to Sentry. Regardless of the exception,
a 200 response with no updates is returned, because that's what the client
expects. See bugs 885173 and 1069454 for additional background."""
a 500 response is returned."""

if not isinstance(error, BadDataError):
if sentry.client:
sentry.captureException()

# We don't want to eat exceptions from this special Dockerflow endpoint because
# it's used by CloudOps' infrastructure to see whether or not the app is
# functional. See https://github.com/mozilla-services/Dockerflow for additional
# details.
if request.path == "/__heartbeat__":
return error

log.debug('Hit exception, sending an empty response', exc_info=True)
response = make_response('<?xml version="1.0"?>\n<updates>\n</updates>')
response.mimetype = 'text/xml'
return response
raise error


@app.route('/robots.txt')
Expand Down