eregs / regulations-site

Web interface for viewing U.S. federal regulations and other regulatory information
https://regulations.atf.gov/
Creative Commons Zero v1.0 Universal
20 stars 46 forks source link

Missing data leads to an explosion #485

Open cmc333333 opened 7 years ago

cmc333333 commented 7 years ago

We need a friendlier "there's no data" message. Right now the page just explodes...

ccostino commented 6 years ago

@cmc333333, is this by any chance the same issue as a stack trace being thrown on the server regarding a KeyError when trying to view a regulation? Example:

Traceback (most recent call last):
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/core/handlers/base.py", line 249, in _legacy_get_response
    response = self._get_response(request)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/newrelic/hooks/framework_django.py", line 544, in wrapper
    return wrapped(*args, **kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/utils/decorators.py", line 149, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/newrelic/hooks/framework_django.py", line 940, in wrapper
    return wrapped(*args, **kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/views/generic/base.py", line 88, in dispatch
    return handler(request, *args, **kwargs)
  File "/home/vcap/deps/0/src/regulations/regulations/views/chrome.py", line 64, in get
    return super(ChromeView, self).get(request, *args, **kwargs)
  File "/home/vcap/deps/0/python/lib/python3.6/site-packages/django/views/generic/base.py", line 155, in get
    context = self.get_context_data(**kwargs)
  File "/home/vcap/deps/0/src/regulations/regulations/views/chrome.py", line 132, in get_context_data
    self.set_chrome_context(context, reg_part, version)
  File "/home/vcap/deps/0/src/regulations/regulations/views/chrome.py", line 114, in set_chrome_context
    context['history'], context['meta']['effective_date'])
KeyError: 'effective_date'

If so, we're seeing this some in fec-eregs - if not, I'll file another issue. :-)

cmc333333 commented 6 years ago

Hey @ccostino -- it's certainly possible. Is the database populated? In particular, this is trying to access meta data about a specific regulation.

ccostino commented 6 years ago

The database is populated, but this happens when hitting, e.g., https://www.fec.gov/regulations/104-8/2017-annual-104 or https://www.fec.gov/regulations/110-8/2017-annual-110

ccostino commented 6 years ago

Turns out the database is not populated with 2017 data, hence the reason why. Thanks @cmc333333! FWIW we have an open issue to account for error handling in these situations on the fec-eregs side of things: https://github.com/fecgov/fec-eregs/issues/385 :-)

ccostino commented 6 years ago

And I now realize that the change has to happen here; the only thing we can do on the fec-eregs side is update the regulations-site dependency once a fix is in place to account for this kind of error handling.

ccostino commented 6 years ago

The error is taking place in the ChromeView object, which is a class-based Django view. I'm not sure where specifically we'll want to make the change in there. Based on the stack trace above, I feel like making a fix in the exact location of that particular error (which is in the set_chrome_context method) might be too specific of a spot, although I'm not sure.

If there's a better place for capturing any error that occurs during the view processing and handling it there, that would seem ideal to me. Going over the documentation for the TemplateView may help in figuring out the best spot. Does that makes sense to you, @cmc333333?

cmc333333 commented 6 years ago

@ccostino I think we probably want to raise a 404 as soon as we see the meta data is empty. There's already logic around catching MissingContentExceptions, so it may make sense to raise that.

TemplateView's docs won't help too much: the primary entry is get, which ultimately calls get_context_data. Our implementation calls set_chrome_context, which does most of the work, if you want to trace the data flow.

ccostino commented 6 years ago

Ah good point, thanks for explaining that @cmc333333!

lbeaufort commented 6 years ago

This issue is addressed by https://github.com/eregs/regulations-site/pull/515. Thank you @cmc333333 and @ccostino for your help!