Bug 1188132 - Further fixes to make (talos) log parsing more robust

* Simplify logic in talos parser (there was an optimization which didn't
  save anything and just caused confusion before)
* Make it so if log parsing fails for a non-http reason, we don't try
  again

tests/log_parser/test_performance_log_parser.py

@@ -14,9 +14,8 @@ def test_valid_talosdata():
 
 
 # Disabled because of bug 1188132.
-@pytest.mark.xfail
 def test_invalid_talosdata():
-    invalid_line = '10:27:39     INFO -  INFO : TALOSDATA: [ { "json"'
+    invalid_line = '10:27:39     INFO -  INFO : TALOSDATA: [ { "json" ]'
     parser = TalosParser()
     with pytest.raises(ValueError):
         parser.parse_line(invalid_line, 1)

treeherder/log_parser/parsers.py

@@ -339,12 +339,8 @@ class TalosParser(ParserBase):
     def parse_line(self, line, lineno):
         """check each line for TALOSDATA"""
 
-        if "TALOSDATA" in line:
+        match = self.RE_TALOSDATA.match(line)
-            match = self.RE_TALOSDATA.match(line)
+        if match:
-            if match:
+            # this will throw an exception if the json parsing breaks, but
-                # this will throw an exception if the parsing breaks, but
+            # that's the behaviour we want
-                # that's the behaviour we want
+            self.artifact = json.loads(match.group(1))
-                self.artifact = json.loads(match.group(1))
-            else:
-                # probably a line which just happens to contain talosdata, ignore
-                pass

treeherder/log_parser/utils.py

@@ -95,11 +95,24 @@ def post_log_artifacts(project,
                                              job_guid, check_errors)
     except Exception as e:
         client.update_parse_status(project, job_log_url['id'], 'failed')
+        # unrecoverable http error (doesn't exist or permission denied)
+        # (apparently this can happen somewhat often with taskcluster if
+        # the job fails, so just warn about it)
         if isinstance(e, urllib2.HTTPError) and e.code in (403, 404):
-            logger.debug("Unable to retrieve log for %s: %s", log_description, e)
+            logger.warning("Unable to retrieve log for %s: %s",
-            return
+                           log_description, e)
-        logger.error("Failed to download/parse log for %s: %s", log_description, e)
+        # possibly recoverable http error (e.g. problems on our end)
-        _retry(e)
+        elif isinstance(e, urllib2.URLError):
+            logger.error("Failed to download log for %s: %s",
+                         log_description, e)
+            _retry(e)
+        # parse error or other unrecoverable error
+        else:
+            logger.error("Failed to download/parse log for %s: %s",
+                         log_description, e)
+        # re-raise exception if we're not retrying, so new relic sees the
+        # error
+        raise
 
     # store the artifacts generated
     tac = TreeherderArtifactCollection()