diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 72f2633d2f..762aa6512b 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -574,6 +574,7 @@ def rotate( ) except Exception as e: logger.exception(f"Error rotating document {pair.root_doc.id}: {e}") + raise return "OK" @@ -873,6 +874,7 @@ def split( except Exception as e: logger.exception(f"Error splitting document {doc.id}: {e}") + raise return "OK" @@ -918,6 +920,7 @@ def delete_pages( ) except Exception as e: logger.exception(f"Error deleting pages from document {pair.root_doc.id}: {e}") + raise return "OK" diff --git a/src/documents/tasks.py b/src/documents/tasks.py index b4096d2798..193596f635 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -713,8 +713,9 @@ def run_workflows_for_documents( logger.warning( f"Manual workflow run requested for missing workflow {workflow_id}", ) - return + raise + failed_ids = [] for document in Document.objects.filter( id__in=document_ids, root_document__isnull=True, @@ -732,10 +733,16 @@ def run_workflows_for_documents( document=document, ) except Exception: + failed_ids.append(document.pk) logger.exception( f"Error running workflow {workflow} on document {document.pk}", ) + if failed_ids: + raise RuntimeError( + f"Workflow {workflow_id} failed for document(s): {failed_ids}. Check logs for details.", + ) + def update_document_parent_tags(tag: Tag, new_parent: Tag) -> None: """ diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 8e7d2e1546..f9a8f71d2e 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -1038,8 +1038,9 @@ class TestPDFActions(DirectoriesMixin, TestCase): mock_consume_file.assert_not_called() + @mock.patch("documents.bulk_edit.group") @mock.patch("documents.tasks.consume_file.s") - def test_split(self, mock_consume_file) -> None: + def test_split(self, mock_consume_file, mock_group) -> None: """ GIVEN: - Existing documents @@ -1060,6 +1061,7 @@ class TestPDFActions(DirectoriesMixin, TestCase): self.assertIsNone(mock_consume_file.call_args.kwargs["overrides"].asn) self.assertEqual(result, "OK") + mock_group.return_value.delay.assert_called_once() @mock.patch("documents.bulk_edit.group") @mock.patch("documents.pdf_ops.build_pdfs") @@ -1168,8 +1170,8 @@ class TestPDFActions(DirectoriesMixin, TestCase): sig.apply_async.side_effect = Exception("boom") mock_chord.return_value = sig - result = bulk_edit.split(doc_ids, pages, delete_originals=True) - self.assertEqual(result, "OK") + with self.assertRaisesMessage(Exception, "boom"): + bulk_edit.split(doc_ids, pages, delete_originals=True) self.doc2.refresh_from_db() self.assertEqual(self.doc2.archive_serial_number, 222) @@ -1191,7 +1193,8 @@ class TestPDFActions(DirectoriesMixin, TestCase): pages = [[1, 2], [3]] with self.assertLogs("paperless.bulk_edit", level="ERROR") as cm: - bulk_edit.split(doc_ids, pages) + with self.assertRaisesMessage(Exception, "Error building PDFs"): + bulk_edit.split(doc_ids, pages) error_str = cm.output[0] expected_str = "Error splitting document 2" self.assertIn(expected_str, error_str) @@ -1240,7 +1243,8 @@ class TestPDFActions(DirectoriesMixin, TestCase): doc_ids = [self.doc2.id, self.doc3.id] with self.assertLogs("paperless.bulk_edit", level="ERROR") as cm: - bulk_edit.rotate(doc_ids, 90) + with self.assertRaisesMessage(Exception, "Error rotating PDF"): + bulk_edit.rotate(doc_ids, 90) error_str = cm.output[0] expected_str = "Error rotating document" self.assertIn(expected_str, error_str) @@ -1375,7 +1379,8 @@ class TestPDFActions(DirectoriesMixin, TestCase): pages = [1, 3] with self.assertLogs("paperless.bulk_edit", level="ERROR") as cm: - bulk_edit.delete_pages(doc_ids, pages) + with self.assertRaisesMessage(Exception, "Error removing pages"): + bulk_edit.delete_pages(doc_ids, pages) error_str = cm.output[0] expected_str = "Error deleting pages from document" self.assertIn(expected_str, error_str) diff --git a/src/documents/tests/test_bulk_operations.py b/src/documents/tests/test_bulk_operations.py index 2e40a23b1b..be977640c8 100644 --- a/src/documents/tests/test_bulk_operations.py +++ b/src/documents/tests/test_bulk_operations.py @@ -184,6 +184,38 @@ def test_delete_failure_propagates_from_worker(bulk_setup): assert Document.objects.filter(pk=doc.pk).exists() +@pytest.mark.django_db +@pytest.mark.parametrize( + "operation,parameters,preparation", + [ + ("rotate", {"degrees": 90}, "rotate_pdf"), + ("split", {"pages": [[1]]}, "build_pdfs"), + ("delete_pages", {"pages": [1]}, "remove_pages"), + ], +) +def test_preparation_failure_propagates_from_worker( + bulk_setup, + operation, + parameters, + preparation, +): + user, _, doc, _ = bulk_setup + doc.mime_type = "application/pdf" + doc.save() + with ( + mock.patch( + f"documents.bulk_edit.pdf_ops.{preparation}", + side_effect=RuntimeError("preparation failed"), + ), + mock.patch("documents.bulk_edit.consume_file.apply_async") as consume, + mock.patch("documents.bulk_edit.consume_file.s") as consume_signature, + pytest.raises(RuntimeError, match="preparation failed"), + ): + execute_bulk_operation(operation, [doc.pk], parameters, user.pk) + consume.assert_not_called() + consume_signature.assert_not_called() + + @pytest.mark.django_db def test_queue_failure_does_not_change_documents(bulk_setup): _, client, doc, correspondent = bulk_setup diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 78acc7b382..4560ad0360 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -2333,9 +2333,12 @@ class TestWorkflows( self.assertEqual(doc.owner, self.user2) def test_manual_workflow_run_missing_workflow(self) -> None: - """A missing workflow id logs a warning rather than raising.""" + """A workflow removed after queueing fails the task.""" doc = DocumentFactory() - with self.assertLogs("paperless.tasks", level="WARNING"): + with ( + self.assertLogs("paperless.tasks", level="WARNING"), + self.assertRaises(Workflow.DoesNotExist), + ): tasks.run_workflows_for_documents([doc.pk], 9999) @mock.patch("documents.tasks.send_websocket_document_updated") @@ -2356,7 +2359,13 @@ class TestWorkflows( mock_run_workflows.side_effect = side_effect - with self.assertLogs("paperless.tasks", level="ERROR"): + with ( + self.assertLogs("paperless.tasks", level="ERROR"), + self.assertRaisesMessage( + RuntimeError, + f"failed for document(s): [{doc1.pk}]", + ), + ): tasks.run_workflows_for_documents([doc1.pk, doc2.pk], workflow.pk) # Both documents were attempted; the websocket update only fired for the