refactor(abstractions)!: remove the 3 never-thrown ABI exception types (rec #3) - #228
Merged
Merged
Conversation
…iled/ThresholdExceeded exceptions BatchFailedException, SourceFailedException and ThresholdExceededException were declared in the frozen Abstractions ABI but never thrown anywhere: the pipeline reports a batch/source/threshold failure through ReportRunResult.Status + its error string and the IFailureStrategy decision, not by throwing. As dead surface in a contract that is meant to be minimal (rule 7), they were a liability locked in by SemVer, and their documented "typed error" contract was a mirage — a consumer's catch for them could never fire. Removed. NeoReportsException (the base, carrying the stable Code) and ConfigurationException are unchanged and still used. A full solution build confirms nothing referenced the removed types. BREAKING (Abstractions): source-breaking for any consumer that referenced those three types; slated for the next major. Recorded in CHANGELOG under Unreleased → Removed. Maintainer decision (2026-07-30) to remove rather than retain-and-document or make-real (throwing would contradict the result-based run contract).
|
|
This was referenced Jul 30, 2026
thiagoluga
added a commit
that referenced
this pull request
Jul 30, 2026
…pers (CA1068) (#234) `AdoSourceHealth.PingAsync`, `AdoSourceHealth.CheckConnectionStringAsync` and `HttpHealthProbe.SendAsync` had the token mid-list, before a trailing optional `pingSql`/`content` argument. Move it to last (defaulted) and update all callers. Positional callers that passed the token before the optional arg are source-breaking, so this is tagged next-major in the CHANGELOG (Changed → breaking, public API) alongside the #228 ABI-exception removal. Resolves the CA1068 backlog item in docs/STATUS-AND-BACKLOG.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third maintainer-decision recommendation. Maintainer chose: remove (breaking, next major).
The finding
BatchFailedException,SourceFailedExceptionandThresholdExceededExceptionare declared in the frozenNeoReports.AbstractionsABI but never thrown anywhere in the codebase. The pipeline reports batch/source/threshold failures throughReportRunResult.Status+ its error string and theIFailureStrategydecision — not by throwing. So they were dead surface in a contract meant to be minimal (rule 7: every unused type is a liability locked in by SemVer), and the "typed error" contract they implied was a mirage: a consumer'scatch (BatchFailedException)could never fire.Decision
Per the maintainer's call, removed — over the alternatives of retaining-and-documenting (leaves the liability) or making-them-real (throwing would contradict the deliberate result-based run contract and break every consumer that checks
result.Status).NeoReportsException(the base, carrying the stableCode) andConfigurationExceptionare unchanged and still used.Breaking-change handling
Source-breaking for any consumer that referenced those three types — though nothing ever threw them, so no
catchfor them could have fired. Slated for the next major; recorded inCHANGELOG.mdunder Unreleased → Removed (which now also summarizes the audit's Added/Changed/Fixed entries).Verification
grepacross src/tests/samples/benchmarks: zero references to the three types.dotnet build --no-incremental(full solution): 0 warnings, 0 errors — confirms nothing referenced them.Rec #1 (streaming XLSX, #226) and #2 (auth warning, #227) merged. This is #3. Next: retry default (#4), whole-job deadline + async error scrub (#5), and the minor items (#6).