Repository: dmwm/wmcore
Issue: CMSSW seg fault showing up as BadFWJRXML error in WMStats

Provided that jobs can pile many different exceptions and exit codes within the same step, I'd expect the very first exception to be reported in WMStats.

Which is not the case for (in testbed)
amaltaro_TaskChain_NanoAOD_Agent124_Val_190706_191332_1418

there is a job that seg faulted with an exit code 139, followed then by a failure to read the XML job report with an exit code 50115 (of course, the report wasn't completely created). What is getting reported in the WMStats table is actually the 50115 first, followed by the other 2 exit codes added to the job.
```
Job Name: 304435bf-e897-410b-bc6e-a8a7e6771653-0
WMBS job id: 39
Workflow: amaltaro_TaskChain_NanoAOD_Agent124_Val_190706_191332_1418
Task: /amaltaro_TaskChain_NanoAOD_Agent124_Val_190706_191332_1418/RunIIFall17NanoAOD
Status: jobfailed
Input dataset: /WJetsToLNu_HT-200To400_TuneCUETP8M1_13TeV-madgraphMLM-pythia8/RunIISummer16MiniAODv2-PUMoriond17_80X_mcRun2_asymptotic_2016_TrancheIV_v6_ext1-v1/MINIAODSIM
Site: T2_CH_CERN
Agent: vocms0193.cern.ch
ACDC URL: https://cmsweb-testbed.cern.ch/couchdb/acdcserver
State Transition
Exit code: 50115
Retry count: 0
Errors:

    logArch1

    cmsRun1
        BadFWJRXML (Exit Code: 50115)

            Error reading XML job report file, possibly corrupt XML File:
            Details: no element found: line 1, column 0

        CMSSWStepFailure (Exit Code: 139)

             Adding last 25 lines of CMSSW stdout:
            #3  <signal handler called>
```

Given that 139 was the first error, it should be the first error to be added to that step. As well as the first error to get reported in the WMStats table.

So we need to investigate:
* whether the order of errors are correct in the job report
* and why wmstats is changing that order (or which order/logic it's using).

Job log can be found here: http://amaltaro.web.cern.ch/amaltaro/forWMCore/Issue_9289/

Changed files:
test/python/WMCore_t/FwkJobReport_t/Report_t.py, src/python/WMCore/WMSpec/Steps/Diagnostic.py, src/python/WMCore/WMSpec/Steps/Diagnostics/CMSSW.py, test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/CMSSW_t.py, src/python/WMCore/WMSpec/Steps/Diagnostics/DeleteFiles.py, src/python/WMCore/WMSpec/Steps/Executors/CMSSW.py, src/python/WMCore/WMSpec/Steps/Diagnostics/AlcaHarvest.py, src/python/WMCore/FwkJobReport/Report.py, src/python/WMCore/WMSpec/Steps/Diagnostics/DQMUpload.py, src/python/WMCore/WMSpec/Steps/Diagnostics/LogArchive.py

diff --git a/src/python/WMCore/FwkJobReport/Report.py b/src/python/WMCore/FwkJobReport/Report.py
index 694747ce6..79b3ed119 100644
--- a/src/python/WMCore/FwkJobReport/Report.py
+++ b/src/python/WMCore/FwkJobReport/Report.py
@@ -152,7 +152,6 @@ class Report(object):
             for stackFrame in stackTrace:
                 crashMessage += stackFrame
 
-            self.addError(stepName, 50115, "BadFWJRXML", msg)
             logging.debug(crashMessage)
             raise FwkJobReportException(msg)
 
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostic.py b/src/python/WMCore/WMSpec/Steps/Diagnostic.py
index 530123a25..d55faf34a 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostic.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostic.py
@@ -8,7 +8,7 @@ Interface and base class for step specific diagnostic handlers
 
 """
 
-
+from WMCore.FwkJobReport.Report import FwkJobReportException
 
 
 class DiagnosticHandler(object):
@@ -34,6 +34,19 @@ class DiagnosticHandler(object):
         msg += "implemented for class %s" % self.__class__.__name__
         raise NotImplementedError(msg)
 
+    def parse(self, executorInstance, jobRepXml):
+        """
+        Add an error to report if parsing the xml fails.
+        """
+        try:
+            executorInstance.report.parse(jobRepXml, executorInstance.stepName)
+        except FwkJobReportException as ex:
+            # Job report is bad, the parse already puts a 50115 in the file
+            msg = "Error reading XML job report file, possibly corrupt XML File:\\n"
+            msg += "Details: %s" % str(ex)
+            executorInstance.report.addError(executorInstance.stepName, 50115, "BadFWJRXML", msg)
+            raise
+
 
 class DefaultDiagnosticHandler(DiagnosticHandler):
     """
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostics/AlcaHarvest.py b/src/python/WMCore/WMSpec/Steps/Diagnostics/AlcaHarvest.py
index bba478057..7973f2683 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostics/AlcaHarvest.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostics/AlcaHarvest.py
@@ -45,7 +45,7 @@ class AHExceptionHandler(DiagnosticHandler):
             return
 
         # job report XML exists, load the exception information from it
-        executor.report.parse(jobRepXml)
+        self.parse(executor, jobRepXml)
 
         # make sure the report has the error in it
         errSection = getattr(executor.report.report, "errors", None)
@@ -66,4 +66,6 @@ class AlcaHarvest(Diagnostic):
         self.handlers[60319] = Exit60319()
 
         catchAll = AHExceptionHandler()
-        [self.handlers.__setitem__(x, catchAll) for x in range(0, 255) if x not in self.handlers]
+        for x in range(0, 255):
+            if x not in self.handlers:
+                self.handlers.__setitem__(x, catchAll)
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostics/CMSSW.py b/src/python/WMCore/WMSpec/Steps/Diagnostics/CMSSW.py
index 1a9eeee42..f3b86c80e 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostics/CMSSW.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostics/CMSSW.py
@@ -99,10 +99,11 @@ class CMSDefaultHandler(DiagnosticHandler):
         if os.path.exists(jobRepXml):
             # job report XML exists, load the exception information from it
             try:
-                executor.report.parse(jobRepXml)
-            except FwkJobReportException:
-                # Job report is bad, the parse already puts a 50115 in the file
+                self.parse(executor, jobRepXml)
+            except FwkJobReportException:    
+                # Job report is bad, the parse already puts a 50115 in the file    
                 pass
+
             reportStep = executor.report.retrieveStep(executor.stepName)
             reportStep.status = errCode
 
@@ -149,10 +150,11 @@ class CMSRunHandler(DiagnosticHandler):
         if os.path.exists(jobRepXml):
             # job report XML exists, load the exception information from it
             try:
-                executor.report.parse(jobRepXml)
-            except FwkJobReportException:
-                # Job report is bad, the parse already puts a 50115 in the file
+                self.parse(executor, jobRepXml)
+            except FwkJobReportException:    
+                # Job report is bad, the parse already puts a 50115 in the file    
                 pass
+
             reportStep = executor.report.retrieveStep(executor.stepName)
             reportStep.status = self.code
 
@@ -238,10 +240,9 @@ class EDMExceptionHandler(DiagnosticHandler):
 
         # job report XML exists, load the exception information from it
         try:
-            executor.report.parse(jobRepXml)
-        except FwkJobReportException:
-            # Job report is bad, the parse already puts a 50115 in the file
-            # just go on
+            self.parse(executor, jobRepXml)
+        except FwkJobReportException:    
+            # Job report is bad, the parse already puts a 50115 in the file    
             pass
 
         # make sure the report has the error in it
@@ -294,4 +295,6 @@ class CMSSW(Diagnostic):
         # for all the exception codes between 1 and 225, use a default that attempts to read the code
         # from the job report
         catchAll = EDMExceptionHandler()
-        [self.handlers.__setitem__(x, catchAll) for x in range(0, 255) if x not in self.handlers]
+        for x in range(0, 255):
+            if x not in self.handlers:
+                self.handlers.__setitem__(x, catchAll)
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostics/DQMUpload.py b/src/python/WMCore/WMSpec/Steps/Diagnostics/DQMUpload.py
index 8b1d5efb9..d5745f560 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostics/DQMUpload.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostics/DQMUpload.py
@@ -44,7 +44,7 @@ class DUExceptionHandler(DiagnosticHandler):
             return
 
         # job report XML exists, load the exception information from it
-        executor.report.parse(jobRepXml)
+        self.parse(executor, jobRepXml)
 
         # make sure the report has the error in it
         errSection = getattr(executor.report.report, "errors", None)
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostics/DeleteFiles.py b/src/python/WMCore/WMSpec/Steps/Diagnostics/DeleteFiles.py
index 3c0f21362..bd063fb13 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostics/DeleteFiles.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostics/DeleteFiles.py
@@ -40,8 +40,7 @@ class DFExceptionHandler(DiagnosticHandler):
             return
 
         # job report XML exists, load the exception information from it
-        executor.report.parse(jobRepXml)
-
+        self.parse(executor, jobRepXml)
 
         # make sure the report has the error in it
         errSection = getattr(executor.report.report, "errors", None)
@@ -64,4 +63,6 @@ class DeleteFiles(Diagnostic):
 
 
         catchAll = DFExceptionHandler()
-        [ self.handlers.__setitem__(x, catchAll) for x in range(0, 255) if x not in self.handlers ]
+        for x in range(0, 255):
+            if x not in self.handlers:
+                self.handlers.__setitem__(x, catchAll)
diff --git a/src/python/WMCore/WMSpec/Steps/Diagnostics/LogArchive.py b/src/python/WMCore/WMSpec/Steps/Diagnostics/LogArchive.py
index aee763945..7dd3fca8d 100644
--- a/src/python/WMCore/WMSpec/Steps/Diagnostics/LogArchive.py
+++ b/src/python/WMCore/WMSpec/Steps/Diagnostics/LogArchive.py
@@ -42,8 +42,7 @@ class LAExceptionHandler(DiagnosticHandler):
             return
 
         # job report XML exists, load the exception information from it
-        executor.report.parse(jobRepXml)
-
+        self.parse(executor, jobRepXml)
 
         # make sure the report has the error in it
         errSection = getattr(executor.report.report, "errors", None)
@@ -71,5 +70,6 @@ class LogArchive(Diagnostic):
         # Setup a default handler
         catchAll            = LAExceptionHandler()
         self.defaultHandler = catchAll
-
-        [ self.handlers.__setitem__(x, catchAll) for x in range(0, 255) if x not in self.handlers ]
+        for x in range(0, 255):
+            if x not in self.handlers:
+                self.handlers.__setitem__(x, catchAll)
diff --git a/src/python/WMCore/WMSpec/Steps/Executors/CMSSW.py b/src/python/WMCore/WMSpec/Steps/Executors/CMSSW.py
index 9b31aaaa3..bd3725993 100644
--- a/src/python/WMCore/WMSpec/Steps/Executors/CMSSW.py
+++ b/src/python/WMCore/WMSpec/Steps/Executors/CMSSW.py
@@ -43,6 +43,10 @@ class CMSSW(Executor):
 
     """
 
+    def __init__(self):
+        super(CMSSW, self).__init__()
+        self.failedPreviousStep = None
+
     def _setStatus(self, returnCode, returnMessage):
         """
         Set return code.
diff --git a/test/python/WMCore_t/FwkJobReport_t/Report_t.py b/test/python/WMCore_t/FwkJobReport_t/Report_t.py
index 9137e9c85..047de3375 100644
--- a/test/python/WMCore_t/FwkJobReport_t/Report_t.py
+++ b/test/python/WMCore_t/FwkJobReport_t/Report_t.py
@@ -253,8 +253,6 @@ class ReportTest(unittest.TestCase):
         myReport = Report("cmsRun1")
         from WMCore.FwkJobReport.Report import FwkJobReportException
         self.assertRaises(FwkJobReportException, myReport.parse, self.badxmlPath)
-        self.assertEqual(myReport.getStepErrors("cmsRun1")['error0'].type, 'BadFWJRXML')
-        self.assertEqual(myReport.getStepErrors("cmsRun1")['error0'].exitCode, 50115)
         return
 
     def testErrorReporting(self):
diff --git a/test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/CMSSW_t.py b/test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/CMSSW_t.py
index fedd1589b..5ddac7e97 100644
--- a/test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/CMSSW_t.py
+++ b/test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/CMSSW_t.py
@@ -195,10 +195,11 @@ class CMSSW_t(unittest.TestCase):
                 self.fail("An exception should have been raised")
             except WMExecutionFailure as ex:
                 executor.diagnostic(ex.code, executor, ExceptionInstance=ex)
-                self.assertEqual(50115, executor.report.getExitCode())
+                #self.assertEqual(50115, executor.report.getExitCode())
+                self.assertEqual(134, executor.report.getExitCode())
                 report = Report()
                 report.load("Report.pkl")
-                self.assertEqual(50115, report.getExitCode())
+                self.assertEqual(134, report.getExitCode())
         except Exception as ex:
             self.fail("Failure encountered, %s" % str(ex))
         finally: