Don't forget to report fileset complete even if there is an exception

Author: BenGalewsky

src/servicex_did_finder_lib/did_finder_app.py

@@ -99,22 +99,23 @@ def do_lookup(self, did: str, dataset_id: int, endpoint: str, user_did_finder: U
             for file_info in user_did_finder(did_info.did, info, self.app.did_finder_args):
                 acc.add(file_info)
 
-        except Exception:
-            if did_info.get_mode == "all":
-                raise
-
-        acc.send_on(did_info.file_count)
-
-        elapsed_time = int((datetime.now() - start_time).total_seconds())
-        servicex.put_fileset_complete(
-            {
-                "files": summary.file_count,
-                "files-skipped": summary.files_skipped,
-                "total-events": summary.total_events,
-                "total-bytes": summary.total_bytes,
-                "elapsed-time": elapsed_time,
-            }
-        )
+            acc.send_on(did_info.file_count)
+        except Exception as e:
+            self.logger.error(
+                f"Error processing DID {did}: {e}",
+                extra={"dataset_id": dataset_id}
+            )
+        finally:
+            elapsed_time = int((datetime.now() - start_time).total_seconds())
+            servicex.put_fileset_complete(
+                {
+                    "files": summary.file_count,
+                    "files-skipped": summary.files_skipped,
+                    "total-events": summary.total_events,
+                    "total-bytes": summary.total_bytes,
+                    "elapsed-time": elapsed_time,
+                }
+            )
 
 
 class DIDFinderApp:

tests/servicex_did_finder_lib_tests/test_did_finder_app.py

@@ -75,6 +75,35 @@ def test_did_finder_task(mocker, servicex, single_file_info):
         )
 
 
+def test_did_finder_task_exception(mocker, servicex, single_file_info):
+    did_finder_task = DIDFinderTask()
+    # did_finder_task.app = mocker.Mock()
+    did_finder_task.app.did_finder_args = {}
+    mock_generator = mocker.Mock(side_effect=Exception("Boom"))
+
+    mock_accumulator = mocker.MagicMock(Accumulator)
+    with patch(
+        "servicex_did_finder_lib.did_finder_app.Accumulator", autospec=True
+    ) as acc:
+        acc.return_value = mock_accumulator
+        did_finder_task.do_lookup('did', 1, 'https://my-servicex', mock_generator)
+        servicex.assert_called_with(dataset_id=1, endpoint="https://my-servicex")
+        acc.assert_called_once()
+
+        mock_accumulator.add.assert_not_called()
+        mock_accumulator.send_on.assert_not_called()
+
+        servicex.return_value.put_fileset_complete.assert_called_with(
+            {
+                "files": 0,  # Aught to have a side effect in mock accumulator that updates this
+                "files-skipped": 0,
+                "total-events": 0,
+                "total-bytes": 0,
+                "elapsed-time": 0,
+            }
+        )
+
+
 def test_did_finder_app(mocker, monkeypatch):
     # Temporarily replace sys.argv with mock_args
     monkeypatch.setattr(sys, 'argv', [