Repository navigation
Throw only when a condition cannot be detected any other way #398
Description
Background
Developer rule (2026-09-16, reviewing #370): an exception is thrown only when there is no other way to detect the condition, and it is caught at the first opportunity a response can be established. A condition the code has already checked — a lookup returned nothing, a status is wrong, a set of conflicting fields is non-empty — is reported as an outcome, not thrown.
#370 applies this to unresolved field conflicts and to the decide path. The same shape exists across the rest of the application: 11 other exception types this project defines are thrown at about 43 places, nearly all immediately after a check whose result is already known, and SqliteImportActionService alone throws 14 InvalidOperationExceptions the same way. The exceptions that genuinely have no alternative — invalid JSON from System.Text.Json, file and network I/O, SQLite errors — are caught in 60 catch blocks across 23 files, and not every one of them sits at the first point a response can be formed: two converters catch JsonException only to rethrow it as SourceConversionException. About 20 of those catches log the exception at Warning, and many log nothing.
#397 makes this measurable. It logs every thrown exception at Error with an id, and gives a legitimate catch one shared way to declare itself a response point (LogExceptionHandled). A thrown line with no handled line and no Critical line then marks a catch this body of work still has to fix.
Sub-issues
| # | Scope | Depends on |
|---|---|---|
| #404 | Import batch lifecycle: ImportBatchNotFoundException and ImportBatchStateException on reverse and discard, and ImportActionNotFoundException/ImportActionStateException on undo, become outcomes |
#370, which establishes the outcome shape the coordinator returns |
| #405 | Bulk-decide row mapping: ImportActionUnknownEntityTypeException, ImportActionUnknownFieldException, and BulkDecideAsync's own InvalidOperationExceptions become per-row outcomes |
#370, which converts the DecideAsync call inside the same loop |
| #406 | Import upload validation: QuoteImportValidationException, UnknownConverterException, and the converters' SourceConversionException become outcomes, including the IQuoteSourceConverter contract |
#397, which the red run needs |
| #407 | Backup refusal: DatabaseBackupUnavailableException is thrown from an outcome the code already holds |
#397, which the red run needs |
| #408 | Audit every remaining throw and catch: each throw has no alternative; each catch sits at the first point a response can be formed and declares itself through LogExceptionHandled; the existing LogWarning(ex, …) catch sites move to Error |
Every sub-issue above, so the audit reads the converted code; #397, which provides LogExceptionHandled |
Scope boundary
- Unresolved field conflicts and
DecideAsync's failure modes belong to An expected import conflict is signalled by throwing, once per conflicted row per render #370. - Logging every thrown exception, the
Criticallines, and theLogExceptionHandledmethod itself belong to Exceptions the application catches leave no trace outside Visual Studio #397. - Guards against programming errors stay throws — an
ArgumentException-family guard, or an invariant violation such asImportActionPlannerfinding an unresolved field after supplying every decision itself. No caller can respond to a bug in its own call, so there is no outcome to return. The audit sub-issue confirms each remaining throw is one of these rather than assuming it.
Definition of done
- Every sub-issue listed above is closed
- Findings summarised in a closing comment
Activity
docs [#397][#398]: record when to throw and that changed tests run re…