Skip to content

Commit

Permalink
fixes bug 1272050 - Report index breaks on failing counting correlati…
Browse files Browse the repository at this point in the history
…ons (mozilla-services#3345)

r=adngdb
  • Loading branch information
Peter Bengtsson authored and adngdb committed May 18, 2016
1 parent 0680f31 commit c03b544
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
data-correlations-url="{{ url('crashstats:correlations_json') }}"
data-product="{{ report.product }}"
data-version="{{ report.version }}"
data-platform="{{ report.os_name }}"
data-platform="{{ report.os_name or '' }}"
data-signature="{{ report.signature }}"
data-total-correlations="{{ total_correlations }}"
data-correlations-signature-count-url="{{ url('crashstats:correlations_count_json') }}?{{ make_query_string(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ var SignatureCorrelations = (function() {
var container = $('#mainbody');
return {
showSignatureCorrelationsTab: function() {
if (!container.data('platform')) {
// Some strange crashes don't have a platform (aka OS).
// Then they definitely won't have correlations.
return;
}
// If that number is -1, we don't know if there are correlations.
// Find out, by ajax, and if the count is >0, then make the
// "Correlations" tab visible.
Expand Down
60 changes: 60 additions & 0 deletions webapp-django/crashstats/crashstats/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2949,6 +2949,66 @@ def mocked_get(url, params, **options):
for node in doc('#mainbody'):
eq_(node.attrib['data-total-correlations'], '123')

@mock.patch('crashstats.crashstats.models.Bugs.get')
@mock.patch('requests.get')
def test_report_index_empty_os_name(self, rget, rpost):

rpost.side_effect = mocked_post_123

def mocked_get(url, params, **options):
if (
'/crash_data' in url and
'datatype' in params and
params['datatype'] == 'meta'
):
return Response({
'InstallTime': 'Not a number',
'Version': '5.0a1',
'Email': '',
'URL': None,
})
if 'crashes/comments' in url:
return Response({
'hits': [],
'total': 0,
})
if 'correlations/signatures' in url:
return Response({
'hits': [],
'total': 0
})

if (
'/crash_data' in url and
'datatype' in params and
params['datatype'] == 'unredacted'
):
return Response({
'dump': 'some dump',
'signature': 'FakeSignature1',
'uuid': '11cb72f5-eb28-41e1-a8e4-849982120611',
'process_type': None,
'os_name': None,
'product': 'WaterWolf',
'version': '1.0',
'cpu_name': 'amd64',
})
raise NotImplementedError(url)

rget.side_effect = mocked_get

url = reverse(
'crashstats:report_index',
args=['11cb72f5-eb28-41e1-a8e4-849982120611']
)
response = self.client.get(url)
# Despite the `os_name` being null, it should work to render
# this page.
eq_(response.status_code, 200)
doc = pyquery.PyQuery(response.content)
for node in doc('#mainbody'):
eq_(node.attrib['data-platform'], '')

@mock.patch('crashstats.crashstats.models.Bugs.get')
@mock.patch('requests.get')
def test_report_index_with_invalid_parsed_dump(self, rget, rpost):
Expand Down
37 changes: 24 additions & 13 deletions webapp-django/crashstats/crashstats/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1460,19 +1460,30 @@ def handle_middleware_404(crash_id, error_code):
# correlations, the tab becomes visible.
# If that AJAX query has been done before, let's find out early
# and use that fact to immediately display the "Correlations" tab.
total_correlations_cache_key = make_correlations_count_cache_key(
context['report']['product'],
context['report']['version'],
context['report']['os_name'],
context['report']['signature'],
)
# Because it's hard to do something like `{% if foo is None %}` in Jinja
# we instead make the default -1. That means it's not confused with 0
# it basically means "We don't know".
context['total_correlations'] = cache.get(
total_correlations_cache_key,
-1
)
if (
context['report']['product'] and
context['report']['version'] and
context['report']['os_name'] and
context['report']['signature']
):
total_correlations_cache_key = make_correlations_count_cache_key(
context['report']['product'],
context['report']['version'],
context['report']['os_name'],
context['report']['signature'],
)
# Because it's hard to do something like `{% if foo is None %}` in
# Jinja we instead make the default -1. That means it's not
# confused with 0 it basically means "We don't know".
context['total_correlations'] = cache.get(
total_correlations_cache_key,
-1
)
else:
# Some crashes might potentially miss this. For example,
# some crashes unfortunately don't have a platform (aka os_name)
# so finding correlations on those will never work.
context['total_correlations'] = 0

context['BUG_PRODUCT_MAP'] = settings.BUG_PRODUCT_MAP
context['CRASH_ANALYSIS_URL'] = settings.CRASH_ANALYSIS_URL
Expand Down

0 comments on commit c03b544

Please sign in to comment.