Static pages generator script throws error during browser version retrieval [adblockplus.org]

[saroyanm]

Environment

Python: 2.7
Ubuntu: 14.04

How to reproduce

1. Download ​CMS
2. Download ​web.adblockplus.org repository
3. ​Generate static content
4. Observe

Observed behaviour

Current error is being thrown:

Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/opt/cms/cms/bin/generate_static_pages.py", line 155, in <module>
    generate_pages(args.source, args.output, args.rev)
  File "/opt/cms/cms/bin/generate_static_pages.py", line 97, in generate_pages
    params = get_page_params(source, locale, page, format)
  File "/opt/cms/cms/utils.py", line 74, in get_page_params
    params['head'], params['body'] = converter()
  File "/opt/cms/cms/converters.py", line 303, in __call__
    result = self.get_html(*self._params[self._key])
  File "/opt/cms/cms/converters.py", line 418, in get_html
    env.handle_exception()
  File "/usr/local/lib/python2.7/dist-packages/jinja2/environment.py", line 754, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/jinja2/environment.py", line 1033, in make_module
    return TemplateModule(self, self.new_context(vars, shared, locals))
  File "/usr/local/lib/python2.7/dist-packages/jinja2/environment.py", line 1090, in __init__
    self._body_stream = list(template.root_render_func(context))
  File "web.adblockplus.org!pages/requirements.tmpl", line 50, in top-level template code
  File "web.adblockplus.org!pages/requirements.tmpl", line 6, in template
  File "web.adblockplus.org!globals/get_browser_versions.py", line 237, in get_browser_versions
  File "web.adblockplus.org!globals/get_browser_versions.py", line 141, in key_by_version
ValueError: invalid literal for int() with base 10: ''

Expected behaviour

No error is being thrown during the static content generation

Note

In #4087 we have changed the retrieval URLs to make it more reliable, apparently they are down again.

[saroyanm]

@trev: Do you know if it's a regular issue (or the service URL has been changed) ? According to the Fred the server were reporting errors for last few hours.

@kvas, @John: maybe guys you can help us here as well to deal with this issue ?

We are using 3-rd party sources to generate our requirements page and when the sources are being down we are getting current errors, we use cache AFAIK, but we are still getting the errors when the services are not being available for a while and generating errors, in general I'm not sure if we should generate errors, but rather warnings ?

[jsonesen]

This is because the browser version detection is configured on the server and is done via nginx as far as i know. I think it would be possible to change the CMS to not break when testing locally.

[saroyanm]

Replying to jsonesen:

This is because the browser version detection is configured on the server and is done via nginx as far as i know. I think it would be possible to change the CMS to not break when testing locally.

The user browser detectaion yes, but I'm reffering to generation of ​Requirements page, which is using ​current script, to generate the page, see #4087.

The Issue currently is accruing on server side and was reported by @Fred, I just provided local reproduction steps.

[jsonesen]

Oh i see, my apologies!

Yes, it seems that we should have better handling for times when the servers are down. Thanks for opening the ticket and reporting this.

[trev]

For reference, Mozilla is phasing out Firefox Aurora builds. Could it be that this version number is missing now and the script complaining because of that?

[trev]

Never mind, Firefox Aurora version is still in - this is something we will have to address later. However, ​https://www.seamonkey-project.org/seamonkey_versions.json lists an empty string for LATEST_SEAMONKEY_TESTING_VERSION which is apparently causing the issue at hand. I wonder whether this is intended.

[kvas]

Sorry, I'm a bit late to this party.

I'm wondering if while we are waiting for the response from Seamonkey guys we could already change our code to not fail in such situation but replace missing LATEST_SEAMONKEY_TESTING_VERSION with the content of some other key in that JSON file.

Would this be better than what we have now or is it kind of wrong to make such substitutions? What do you think?

[trev]

Given that the point is compiling a list of versions, we can treat this key as optional. I'd rather not ignore all errors silently but we can just accept that this one can be empty.

[jsonesen]

As of this morning it looks like they have removed LATEST_SEAMONKEY_TESTING_VERISION from the response. So perhaps we should just remove it, unless it makes more sense to make it optional since it is possible that this is temporary.

[kvas]

So I'd say we can already make this optional (if this key is not there or empty, we don't include that version in the output).

Then it would be good to check in a couple of months and if it doesn't reappear, we could drop it completely.

[jsonesen]

Replying to kvas:

So I'd say we can already make this optional (if this key is not there or empty, we don't include that version in the output).

Then it would be good to check in a couple of months and if it doesn't reappear, we could drop it completely.

Got a reply on IRC from the SeaMonkey guys. Looks like they removed the aurora channel from the website and just forgot to alsodrop the key. So it seems we are good to just remove it. Although maybe it is a good idea to add some more to the exception? I am not sure about this personally. I think it is simple enough code to find out what would blow up in these globals, but as far as nice to haves go I think this one wouldn't be the worst change.

What do you guys think?

[trev]

First thing first. For now, removing this key is sufficient. While at it, we should remove FIREFOX_AURORA as well - it is already listing the same version as the current Beta, so only a matter of time until it gets removed.

[kvas]

I agree with Wladimir about first just removing the key so the script stops failing.

As for better error messages, I'm not sure it's really worth it: we seem to have been able to figure this one out pretty quickly and then what took time is understanding if the absence of the key was permanent or not. If we start developing a mechanism that would figure out what's wrong with the response that we got from the server and produce a nice message for every case of wrongness, this would make this script somewhat over-engineered.

We could think about replacing the values for missing keys with some defaults, maybe taken from other keys. This way the generation would not fail. However, we still need to know that this is happening, especially if it's happening repeatedly. In this particular case the format of the response changed so our expectation is no longer correct. It's good that we identified this and thought about the right way to handle it. Whatever we do, we should make sure we don't let such changes just silently slip through or we might eventually end up with wrong information being displayed.

Anyway, let's just remove the code that expects LATEST_SEAMONKEY_TESTING_VERISION (and FIREFOX_AURORA, if that makes sense), close this issue and handle the meta-issue separately.

[abpbot]

A commit referencing this issue has landed:
​Issue 5188 - Drops Aurora builds from get_browser_versions.py