Skip to content

Commit 65a2143

Browse files
virag-compbioVirag SharmaVirag SharmajbleschHoeze
authored
feat(grz-tools): add structured failure reason tracking for submissions (#544)
Addresses #481 and supersedes #542 --------- Co-authored-by: Virag Sharma <viragsharma@w620-mb006-wl.local> Co-authored-by: Virag Sharma <viragsharma@w620-mb006-wl.speedport.ip> Co-authored-by: Jo Chen <122810232+jblesch@users.noreply.github.com> Co-authored-by: Florian R. Hölzlwimmer <Hoeze@users.noreply.github.com>
1 parent b0929cf commit 65a2143

11 files changed

Lines changed: 575 additions & 37 deletions

File tree

packages/grz-common/src/grz_common/exceptions.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,23 @@ class VersionFileAccessError(VersionFileError):
1212

1313
class VersionFileValidationError(VersionFileError):
1414
"""Raised when the version file content is invalid or cannot be parsed."""
15+
16+
17+
class DecryptionError(Exception):
18+
"""Raised when decryption of a submission file fails."""
19+
20+
21+
class EncryptionError(Exception):
22+
"""Raised when encryption of a submission file fails."""
23+
24+
25+
class NetworkError(Exception):
26+
"""Raised when a network-related operation fails."""
27+
28+
29+
class UploadError(Exception):
30+
"""Raised when uploading a submission file fails."""
31+
32+
33+
class IncompleteSubmissionError(Exception):
34+
"""Raised when a submission is missing required files or metadata."""

packages/grz-common/src/grz_common/workers/worker.py

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
from os import PathLike
77
from pathlib import Path
88

9+
from grz_common.exceptions import (
10+
DecryptionError,
11+
EncryptionError,
12+
IncompleteSubmissionError,
13+
)
914
from grz_db.models.submission import (
1015
DonorsDiffCollection,
1116
SubmissionDb,
@@ -19,7 +24,7 @@
1924
from ..transfer import init_s3_client
2025
from .download import S3BotoDownloadWorker
2126
from .submission import EncryptedSubmission, Submission, SubmissionValidationError
22-
from .upload import S3BotoUploadWorker
27+
from .upload import S3BotoUploadWorker, UploadError
2328

2429
log = logging.getLogger(__name__)
2530

@@ -192,21 +197,24 @@ def encrypt(
192197
"Please re-run the 'validate' command and try again."
193198
)
194199
self.__log.error(error_msg)
195-
raise SubmissionValidationError(error_msg)
200+
raise IncompleteSubmissionError(error_msg)
196201

197202
self.__log.info("All files verified as successfully validated.")
198203

199204
if force:
200205
# delete the log file if it exists
201206
self.progress_file_encrypt.unlink(missing_ok=True)
202207

203-
encrypted_submission = submission.encrypt(
204-
encrypted_files_dir=str(self.encrypted_files_dir),
205-
progress_log_file=self.progress_file_encrypt,
206-
recipient_public_key_path=recipient_public_key_path,
207-
submitter_private_key_path=submitter_private_key_path,
208-
force=force,
209-
)
208+
try:
209+
encrypted_submission = submission.encrypt(
210+
encrypted_files_dir=str(self.encrypted_files_dir),
211+
progress_log_file=self.progress_file_encrypt,
212+
recipient_public_key_path=recipient_public_key_path,
213+
submitter_private_key_path=submitter_private_key_path,
214+
force=force,
215+
)
216+
except Exception as e:
217+
raise EncryptionError(str(e)) from e
210218

211219
return encrypted_submission
212220

@@ -223,11 +231,14 @@ def decrypt(self, recipient_private_key_path: str | PathLike, force: bool = Fals
223231
# delete the log file if it exists
224232
self.progress_file_decrypt.unlink(missing_ok=True)
225233

226-
submission = encrypted_submission.decrypt(
227-
files_dir=self.files_dir,
228-
progress_log_file=self.progress_file_decrypt,
229-
recipient_private_key_path=recipient_private_key_path,
230-
)
234+
try:
235+
submission = encrypted_submission.decrypt(
236+
files_dir=self.files_dir,
237+
progress_log_file=self.progress_file_decrypt,
238+
recipient_private_key_path=recipient_private_key_path,
239+
)
240+
except Exception as e:
241+
raise DecryptionError(str(e)) from e
231242

232243
return submission
233244

@@ -256,7 +267,7 @@ def upload(self, s3_options: S3Options) -> str:
256267
"Please re-run the 'encrypt' command and try again."
257268
)
258269
self.__log.error(error_msg)
259-
raise SubmissionValidationError(error_msg)
270+
raise IncompleteSubmissionError(error_msg)
260271

261272
self.__log.info("All files verified as successfully encrypted.")
262273

@@ -266,7 +277,10 @@ def upload(self, s3_options: S3Options) -> str:
266277

267278
encrypted_submission = self.parse_encrypted_submission()
268279

269-
upload_worker.upload(encrypted_submission)
280+
try:
281+
upload_worker.upload(encrypted_submission)
282+
except Exception as e:
283+
raise UploadError(str(e)) from e
270284

271285
return encrypted_submission.submission_id
272286

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
"""add failure_reason to submission_states
2+
3+
Revision ID: 09602efd9105
4+
Revises: 834bd50b8734
5+
Create Date: 2026-04-29 09:35:54.981907+00:00
6+
"""
7+
8+
from collections.abc import Sequence
9+
10+
import sqlalchemy as sa
11+
from alembic import op
12+
13+
revision: str = "09602efd9105"
14+
down_revision: str | Sequence[str] | None = "c5f6a8b9d2e1"
15+
branch_labels: str | Sequence[str] | None = None
16+
depends_on: str | Sequence[str] | None = None
17+
18+
ENUM_NAME = "failurereasonenum"
19+
FAILURE_REASONS = [
20+
"duplicate_tang",
21+
"incomplete_submission",
22+
"decryption_error",
23+
"network_error",
24+
"validation_error",
25+
"file_not_found",
26+
"encryption_error",
27+
"upload_error",
28+
"unknown",
29+
]
30+
31+
32+
def upgrade() -> None:
33+
"""Upgrade schema."""
34+
context = op.get_context()
35+
dialect_name = context.bind.dialect.name if context.bind else "sqlite"
36+
37+
if dialect_name == "postgresql":
38+
failure_reason_enum = sa.Enum(*FAILURE_REASONS, name=ENUM_NAME)
39+
failure_reason_enum.create(op.get_bind(), checkfirst=True)
40+
op.add_column(
41+
"submission_states",
42+
sa.Column("failure_reason", sa.Enum(*FAILURE_REASONS, name=ENUM_NAME), nullable=True),
43+
)
44+
else:
45+
with op.batch_alter_table("submission_states", schema=None) as batch_op:
46+
batch_op.add_column(sa.Column("failure_reason", sa.Enum(*FAILURE_REASONS, name=ENUM_NAME), nullable=True))
47+
48+
49+
def downgrade() -> None:
50+
"""Downgrade schema."""
51+
raise RuntimeError("Downgrades not supported.")

packages/grz-db/src/grz_db/models/submission/__init__.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,20 @@ def from_metadata(
247247
)
248248

249249

250+
class FailureReasonEnum(CaseInsensitiveStrEnum, ListableEnum): # type: ignore[misc]
251+
"""Failure reason enum for submissions in ERROR state."""
252+
253+
DUPLICATE_TANG = "duplicate_tang"
254+
INCOMPLETE_SUBMISSION = "incomplete_submission"
255+
DECRYPTION_ERROR = "decryption_error"
256+
NETWORK_ERROR = "network_error"
257+
VALIDATION_ERROR = "validation_error"
258+
FILE_NOT_FOUND = "file_not_found"
259+
ENCRYPTION_ERROR = "encryption_error"
260+
UPLOAD_ERROR = "upload_error"
261+
UNKNOWN = "unknown"
262+
263+
250264
class SubmissionStateLogBase(SQLModel):
251265
"""
252266
Submission state log base model.
@@ -262,6 +276,13 @@ class SubmissionStateLogBase(SQLModel):
262276
description="grzctl versions that created this state log (nullable for backward compatibility with old state logs)",
263277
sa_column=Column(JSON),
264278
)
279+
failure_reason: FailureReasonEnum | None = Field(
280+
default=None,
281+
sa_column=Column(
282+
Enum(FailureReasonEnum, values_callable=lambda e: [x.value for x in e]),
283+
nullable=True,
284+
),
285+
)
265286
timestamp: datetime.datetime = Field(
266287
default_factory=lambda: datetime.datetime.now(datetime.UTC),
267288
sa_column=Column(DateTime(timezone=True), nullable=False),
@@ -748,6 +769,7 @@ def update_submission_state(
748769
state: SubmissionStateEnum,
749770
data: dict | None = None,
750771
grzctl_versions: dict[str, str] | None = None,
772+
failure_reason: FailureReasonEnum | None = None,
751773
) -> SubmissionStateLog:
752774
"""
753775
Updates a submission's state to the specified state.
@@ -774,6 +796,7 @@ def update_submission_state(
774796
state=state,
775797
data=data,
776798
grzctl_versions=grzctl_versions,
799+
failure_reason=failure_reason,
777800
)
778801
signature = state_log_payload.sign(self._author.private_key())
779802

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import pytest
2+
from cryptography.hazmat.primitives import serialization
3+
from cryptography.hazmat.primitives.asymmetric import ed25519
4+
from grz_db.models.author import Author
5+
from grz_db.models.submission import FailureReasonEnum, SubmissionDb, SubmissionStateEnum
6+
7+
SUBMITTER_ID = "123456789"
8+
9+
10+
@pytest.fixture(scope="function")
11+
def test_author() -> Author:
12+
key = ed25519.Ed25519PrivateKey.generate()
13+
private_key_bytes = key.private_bytes(
14+
encoding=serialization.Encoding.PEM,
15+
format=serialization.PrivateFormat.OpenSSH,
16+
encryption_algorithm=serialization.NoEncryption(),
17+
)
18+
return Author(
19+
name="alice",
20+
private_key_bytes=private_key_bytes,
21+
private_key_passphrase="",
22+
)
23+
24+
25+
@pytest.fixture(scope="function")
26+
def db(tmp_path, test_author: Author) -> SubmissionDb:
27+
db_path = tmp_path / "submissions.sqlite"
28+
db_url = f"sqlite:///{db_path.resolve()}"
29+
submission_db = SubmissionDb(db_url=db_url, author=test_author, debug=False)
30+
submission_db.initialize_schema()
31+
return submission_db
32+
33+
34+
@pytest.fixture(scope="function")
35+
def submission_id(db: SubmissionDb) -> str:
36+
sid = f"{SUBMITTER_ID}_2025-01-01_00000000"
37+
db.add_submission(sid)
38+
return sid
39+
40+
41+
class TestFailureReasonEnum:
42+
def test_list_returns_all_values(self):
43+
values = FailureReasonEnum.list()
44+
assert set(values) == {e.value for e in FailureReasonEnum}
45+
46+
def test_case_insensitive_lookup(self):
47+
assert FailureReasonEnum("decryption_error") == FailureReasonEnum.DECRYPTION_ERROR
48+
assert FailureReasonEnum("DECRYPTION_ERROR") == FailureReasonEnum.DECRYPTION_ERROR
49+
assert FailureReasonEnum("Decryption_Error") == FailureReasonEnum.DECRYPTION_ERROR
50+
51+
def test_invalid_value_raises(self):
52+
with pytest.raises(ValueError):
53+
FailureReasonEnum("notavalidreason")
54+
55+
56+
class TestFailureReasonPersistence:
57+
@pytest.mark.parametrize("failure_reason", list(FailureReasonEnum))
58+
def test_roundtrip(self, db: SubmissionDb, submission_id: str, failure_reason: FailureReasonEnum):
59+
"""Each FailureReasonEnum value should persist and be retrieved correctly."""
60+
db.update_submission_state(
61+
submission_id,
62+
SubmissionStateEnum.ERROR,
63+
failure_reason=failure_reason,
64+
)
65+
submission = db.get_submission(submission_id)
66+
latest_state = submission.get_latest_state()
67+
assert latest_state.state == SubmissionStateEnum.ERROR
68+
assert latest_state.failure_reason == failure_reason
69+
70+
def test_failure_reason_none_by_default(self, db: SubmissionDb, submission_id: str):
71+
"""failure_reason should be None when not provided."""
72+
db.update_submission_state(submission_id, SubmissionStateEnum.UPLOADED)
73+
submission = db.get_submission(submission_id)
74+
latest_state = submission.get_latest_state()
75+
assert latest_state.failure_reason is None
76+
77+
def test_failure_reason_only_on_latest_state(self, db: SubmissionDb, submission_id: str):
78+
"""failure_reason should only be set on the Error state, not bleed into subsequent states."""
79+
db.update_submission_state(
80+
submission_id,
81+
SubmissionStateEnum.ERROR,
82+
failure_reason=FailureReasonEnum.DECRYPTION_ERROR,
83+
)
84+
db.update_submission_state(submission_id, SubmissionStateEnum.UPLOADED)
85+
submission = db.get_submission(submission_id)
86+
states = sorted(submission.states, key=lambda s: s.timestamp)
87+
assert states[-1].state == SubmissionStateEnum.UPLOADED
88+
assert states[-1].failure_reason is None
89+
assert states[-2].failure_reason == FailureReasonEnum.DECRYPTION_ERROR
90+
91+
def test_failure_reason_persists_across_sessions(self, tmp_path, test_author: Author):
92+
"""failure_reason should survive closing and reopening the DB connection."""
93+
db_path = tmp_path / "submissions.sqlite"
94+
db_url = f"sqlite:///{db_path.resolve()}"
95+
96+
sid = f"{SUBMITTER_ID}_2025-01-01_00000000"
97+
98+
db1 = SubmissionDb(db_url=db_url, author=test_author, debug=False)
99+
db1.initialize_schema()
100+
db1.add_submission(sid)
101+
db1.update_submission_state(sid, SubmissionStateEnum.ERROR, failure_reason=FailureReasonEnum.NETWORK_ERROR)
102+
103+
db2 = SubmissionDb(db_url=db_url, author=test_author, debug=False)
104+
submission = db2.get_submission(sid)
105+
assert submission.get_latest_state().failure_reason == FailureReasonEnum.NETWORK_ERROR
106+
107+
def test_all_enum_values_are_snake_case(self):
108+
"""Ensures no typos like 'duplicated_tang' slip in — all values should be snake_case."""
109+
for member in FailureReasonEnum:
110+
assert member.value == member.value.lower()
111+
assert " " not in member.value

0 commit comments

Comments
 (0)