We're getting some valid looking requests from 2.0.0.20 (!) users on Windows 98 (!!!). Eg: https://aus3.mozilla.org/update/2/Firefox/2.0.0.20/2008121709/WINNT_x86-msvc/de/beta/Windows_98%204.10/update.xml And they cause tracebacks like: ValueError: invalid version number '2.0.0.20' Stacktrace (most recent call last): File "flask/app.py", line 1060, in full_dispatch_request rv = self.dispatch_request() File "flask/app.py", line 1047, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "flask/views.py", line 56, in view return self.dispatch_request(*args, **kwargs) File "flask/views.py", line 112, in dispatch_request return meth(*args, **kwargs) File "auslib/web/views/client.py", line 35, in get release, update_type = AUS.evaluateRules(query) File "auslib/AUS.py", line 53, in evaluateRules fallbackChannel=getFallbackChannel(updateQuery['channel']) File "auslib/db.py", line 744, in getRulesMatchingQuery if not self._versionMatchesRule(rule['version'], updateQuery['version']): File "auslib/db.py", line 671, in _versionMatchesRule return version_compare(queryVersion, ruleVersion) File "auslib/util/comparison.py", line 38, in version_compare value = MozillaVersion(value) File "distutils/version.py", line 40, in __init__ self.parse(vstring) File "distutils/version.py", line 107, in parse raise ValueError, "invalid version number '%s'" % vstring

Hopefully just updating https://github.com/mozilla/balrog/blob/master/auslib/util/versions.py#L9

StrictVersion doesn't support versions longer than x.y.z, so this is a vaguely hacky workaround for our historic use cases. ie update paths of 1.5.0.x --> 1.5.0.12 ---> 2.0.0.6 2.0.0.x --> 2.0.0.20 ---> 3.0.15 We'd have a rule with version of 1.5.12 mapped to Firefox-2.0.0.6, and 2.0.20 to Firefox-3.0.15. We could duplicate & modify StrictVersion.parse() too, if you prefer.

Comment on attachment 8506807 [details] [diff] [review] [balrog] Squash middle .0 in x.y.0.z Review of attachment 8506807 [details] [diff] [review]: ----------------------------------------------------------------- The bulk of StrictVersion is parse(), so if we end up duplicating it we're almost maintaining our own version class entirely. It sucks a bit that we need to look for "0" instead of a more generic number there, but I would still call this solution elegant rather than hacky =). I can't think of any 4 part version number where the 3rd part has been significant, so ignoring that part seems like the right thing.

We could write version_re = re.compile(r'^(\d+) \. (\d+) ((?:\. \d)? \. (\d+ ))? ([a-zA-Z]+(\d+))?$', re.VERBOSE) to modify w.x.y.z to w.x.z. That seems like a bigger footgun for the future if we don't need to cover any cases which aren't w.x.0.z right now.

Rather than write one regexp to handle all cases, this leaves the existing one alone except for renaming the class. If we fail to parse the version with that, then try the old style with any digit in third position, otherwise call for mama.

As for v2, but trims the regexp down for clarity.

Comment on attachment 8507576 [details] [diff] [review] [balrog] Handle old version separately, with clearer regexp Review of attachment 8507576 [details] [diff] [review]: ----------------------------------------------------------------- Ah, very good idea.

Commit pushed to master at https://github.com/mozilla/balrog https://github.com/mozilla/balrog/commit/3f8776be8ca4e372075ed89be8e872d3569e6682 Bug 1069512, deal with tracebacks for old style w.x.y.z versions, r=bhearsum

Verified in errormill that we've squashed all the errors for 2.0.0.20 errors. Still getting one for 3.7a4webm.

Ben, I'm not sure it's worth handling this one old, anomalous, version, but I don't really want to leave exceptions in errormill either. Any thoughts ?

(In reply to Nick Thomas [:nthomas] from comment #10) > Ben, I'm not sure it's worth handling this one old, anomalous, version, but > I don't really want to leave exceptions in errormill either. Any thoughts ? It's not the end of the world if we have just this one sitting in there. Perhaps we could add a finally "except" block in the MozillaVersion function that raises a BadDataError instead of ValueError? Or would that mask other potentially useful failures to parse version? I also wonder if getting a newer version of Sentry would give us more ways to filter or sort data on the interface in general. It has tags already, but no way of adding custom ones.

Been thinking about this more and it seems like we shouldn't be optimizing around zero exceptions on Sentry. I agree that it's not worth fixing the version regex for this edge case. If Sentry (or some better system) was able to let us tag or otherwise mark exceptions as "acceptable" (aka "stfu about this forever"), we'd be done here. Perhaps we should close this bug and live with these appearing on Sentry until we switch to something more flexible?

Fine by me.