Rename TryOutcome to Outcome and refactor error factory pattern - #1
Conversation
The recent refactors (TryOutcome->Outcome, Adapter->Port, ErrorCauseType->ErrorOrigin, exception factories->Error factories) had left the documentation out of sync. - Replace all references to the removed TryOutcome<T> with Outcome / Outcome<T>. - Replace ErrorCauseType.System/SystemOrInput with ErrorOrigin.Internal/InternalOrExternal. - Rewrite the README/GettingStarted/BestPractices/ErrorContext examples around the Error-factory model: static [ProvidesErrorsFor(nameof(...))] classes returning Error subtypes (DomainError/PrimaryPortError), thrown via .ToException(). - Fix API names in examples: GetResultOrThrow (not GetOrThrow), result.Error (not result.Exception), .Then (not .Bind), nameof (not typeof) in [ProvidesErrorsFor]. - Document previously-undocumented surface: the Outcome pipeline (Then/To/Recover/Finally) and async OutcomeTaskExtensions, the Port error hierarchy, and the Transience / InteractionDirection / ErrorOrigin enums. - Correct the documentation-pipeline architecture: [ProvidesErrorsFor] is the extraction anchor (not derivation from DiagnosableException); mark the CLI and exporter layers as planned rather than shipped. Clarify that inner errors live on Error.InnerErrors. - Fix EN/FR parity gaps (WritingErrorsGuide, OperationalIntegration HelpLink) and the 'regle'->'regle' accent typo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0193U5BYKQDbnJq7EbKMbBfw
Bugs: - GenDoc: honor an explicit GenerateErrorDocumentation=false opt-out even when IncludeProjectsWithoutOptIn is true (tri-state instead of bool). - AssemblyErrorDocumentationReader: tolerate ReflectionTypeLoadException by extracting the loadable types instead of aborting the whole run; make duplicate-Code resolution deterministic by ordering before grouping. - SolutionErrorDocumentationGenerator: deduplicate ErrorDocumentation by Code across assemblies, not just within each assembly. - ErrorCode: guard the implicit string conversion against a null ErrorCode (throws ArgumentNullException instead of NullReferenceException). Encapsulation / correctness: - DiagnosableException: remove unused private helpers (dead code) and make Error get-only; document that inner errors are surfaced via Error.InnerErrors. - ErrorContext: expose Values through a ReadOnlyDictionary so the read-only contract cannot be bypassed. - ErrorDocumentation: make the property setters internal (still populated by the builder and reader via InternalsVisibleTo). - ProvidesErrorsForAttribute: reject empty/whitespace source, like DocumentedByAttribute. - DocumentedByAttribute: restrict usage to methods via [AttributeUsage]. - PrimaryPortInnerErrors / SecondaryPortInnerErrors: make the two types symmetric (internal ToList, DebuggerDisplay + ToString on both). Documentation comments: - Fix mismatched XML docs (ErrorCode.ResetForTests, IErrorExamplesStage, ErrorDocumentationException messages, InfrastructureError.Transience, Error ctor params/ShortMessage, Outcome.Recover guarantees, OutcomeTaskExtensions exception tag), and remove stray '/// >' lines in ErrorContextKey. - Usage sample: fix TryOutcome->Outcome comments and the wrong <exception> cref. Tests: - Rename TryOutcomeTests->OutcomeTests and add coverage for Then/To/Recover/Finally/ ThrowIfFailure and the async OutcomeTaskExtensions. - Rename DiagnosableExceptionTests->ErrorTests to match its Error subject. - Give the GenDoc reader test a descriptive name and look documents up by Code. - Remove the orphaned, fully commented-out InfrastructureExceptionTests. - Sync the two builder-message assertions with the reworded messages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0193U5BYKQDbnJq7EbKMbBfw
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d73fc0aaaf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // An explicit opt-out (property present and set to a falsy value) is always honored, even when | ||
| // IncludeProjectsWithoutOptIn is true. | ||
| if (optedIn == false) { continue; } |
There was a problem hiding this comment.
Evaluate opt-out properties before skipping projects
When IncludeProjectsWithoutOptIn is true, this branch now turns any falsy occurrence of the opt-in property into a hard skip, but TryReadOptInFromProjectFile only scans the raw XML and does not evaluate MSBuild conditions or the selected options.Configuration. A project that has GenerateErrorDocumentation=false in a Release-only property group, for example, will be omitted during Debug solution-wide generation instead of falling back to the global include, so its errors silently disappear from the generated catalog. Please evaluate the property through MSBuild for the active configuration before treating false as an explicit opt-out.
Useful? React with 👍 / 👎.
Summary
This PR consolidates the error handling API by renaming
TryOutcome<T>toOutcome<T>and refactoring the error factory pattern from exception-based classes to static error factory classes. The changes align the library's public API with a more cohesive error modeling approach where errors are first-class values that can be transported without throwing.Key Changes
API Consolidation
TryOutcome<T>toOutcome<T>— The generic outcome type now has a clearer, more concise name that better reflects its dual purpose (success/failure transport)Outcome— Added a non-generic variant for operations that don't return a value, enabling consistent chaining patterns across both variantsTryOutcomeTests.csto newOutcomeTests.cswith expanded test cases covering both generic and non-generic variantsError Factory Pattern Refactoring
InvalidAmountOperationException.CurrencyMismatch()) with static error factory classes (e.g.,InvalidAmountOperationError.CurrencyMismatch())[ProvidesErrorsFor]attribute — Error factory classes are now annotated with[ProvidesErrorsFor(nameof(Model))]to declare ownership of errors for a given domain modelErrorinstances (or subtypes likeDomainError) rather than exceptions, with.ToException()available when throwing is neededDocumentation Pipeline Updates
AssemblyErrorDocumentationReadernow scans for classes annotated with[ProvidesErrorsFor]instead of exception typesDomainError,InfrastructureError,PrimaryPortError,SecondaryPortError)Supporting Changes
ErrorContextimprovements — AddedReadOnlyDictionarywrapper for safer exposure of context valuesErrorDocumentationrefinement — MadeCodeproperty internal setter to enforce immutabilityDiagnosableExceptionTeststoErrorTeststo reflect the shift from exception-centric to error-centric testingImplementation Details
Outcome<T>type maintains full backward compatibility in behavior while providing a clearer API surfaceThen(),To(),Recover(), andFinally()methods for fluent error handling pipelines[ProvidesErrorsFor]attribute serves as the primary anchor for the documentation model, replacing exception type scanning