feat: add Refit and Application Insights integrations - #17
Merged
Merged
Conversation
Cover the two edges the library was missing: the outbound call to a dependency, and the visibility of a domain failure that never became an exception. Offside.Refit maps a failed Refit call onto Error values. The status mapping mirrors what the dependency answered (404 -> NotFound, 502/503 -> ServiceUnavailable, 504 -> Timeout), the inverse of the kind-to-status mapping Offside applies on the way out. IExternalApiCaller removes the try/catch from every call site, converting ApiException, timeouts, and transport failures while rethrowing a cancellation the caller requested. When the dependency is itself an Offside service, its problem+json errors array is restored error for error; parsing never throws, so a malformed body degrades to the status mapping. OffsideRefitDiagnosticsHandler reports wire failures to an IExternalApiErrorObserver seam. Offside.ApplicationInsights records Error and Result failures as traces, with severity derived from ErrorKind and stable offside.* dimensions. Error.Arguments stay out of telemetry unless IncludeArguments is enabled: they carry whatever the domain put in them, and telemetry outlives the request. Offside.ApplicationInsights.MediatR is a thin bridge so neither main package depends on the other. Refit is floored at 8.0.0: 7.x carries a critical advisory, and 12+ drags ReactiveUI onto the netstandard2.0 target. The new test projects assert through Offside.Testing, matching the convention the assertions package introduced: ShouldHaveOnlyError(code) .WithKind(...).WithArgument(...) instead of indexing Errors[0]. Where the subject is a bare Error - an observed wire failure, a recorded notification - the spy exposes it as a failed Result so the same assertions apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_867a4e80-4ecd-49d9-88eb-7532be155c8f) |
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
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.
Cover the two edges the library was missing: the outbound call to a dependency, and the visibility of a domain failure that never became an exception.
Offside.Refit maps a failed Refit call onto Error values. The status mapping mirrors what the dependency answered (404 -> NotFound, 502/503 -> ServiceUnavailable, 504 -> Timeout), the inverse of the kind-to-status mapping Offside applies on the way out. IExternalApiCaller removes the try/catch from every call site, converting ApiException, timeouts, and transport failures while rethrowing a cancellation the caller requested. When the dependency is itself an Offside service, its problem+json errors array is restored error for error; parsing never throws, so a malformed body degrades to the status mapping. OffsideRefitDiagnosticsHandler reports wire failures to an IExternalApiErrorObserver seam.
Offside.ApplicationInsights records Error and Result failures as traces, with severity derived from ErrorKind and stable offside.* dimensions. Error.Arguments stay out of telemetry unless IncludeArguments is enabled: they carry whatever the domain put in them, and telemetry outlives the request.
Offside.ApplicationInsights.MediatR is a thin bridge so neither main package depends on the other.
Refit is floored at 8.0.0: 7.x carries a critical advisory, and 12+ drags ReactiveUI onto the netstandard2.0 target.
The new test projects assert through Offside.Testing, matching the convention the assertions package introduced: ShouldHaveOnlyError(code) .WithKind(...).WithArgument(...) instead of indexing Errors[0]. Where the subject is a bare Error - an observed wire failure, a recorded notification - the spy exposes it as a failed Result so the same assertions apply.
Summary
Describe the change and its motivation.
Validation
dotnet restore --locked-modedotnet build Offside.sln -c Release --no-restoredotnet test Offside.sln -c Release --no-build --no-restoredocs/+docs/pt-BR/) updated, or N/A (internal-only)Compatibility
Note
Medium Risk
New outbound error-mapping and telemetry paths affect how dependency failures surface to callers and what lands in long-lived logs; misconfiguration (e.g. enabling argument dimensions or passing through mirrored 404s) can leak context, though packages are optional and additive.
Overview
Adds three optional integration packages so outbound dependency failures and domain errors that never throw are first-class in the Offside model.
Offside.Refit turns Refit
ApiException(and timeouts /HttpRequestException) intoError/Result, with HTTP status mirrored ontoErrorKindand optionalapplication/problem+jsonparsing—including round-tripping another Offside service’serrorsarray.IExternalApiCallercentralizes thetry/catchat call sites;OffsideRefitDiagnosticsHandlerplusIExternalApiErrorObserverobserve wire failures without changing responses.Offside.ApplicationInsights writes failed
Errorvalues (andResult.RecordTo) as Application Insights traces withoffside.*dimensions and kind-based severity;Error.Argumentsare omitted unlessIncludeArgumentsis set. Offside.ApplicationInsights.MediatR records publishedDomainNotificationvalues via the same recorder, alongside the existing MediatR collector.The solution, README, CHANGELOG, API reference, and bilingual guides are updated; dedicated test projects cover mapping, caller behavior, diagnostics, and telemetry registration (including idempotent MediatR bridge registration).
Reviewed by Cursor Bugbot for commit f80e78e. Bugbot is set up for automated code reviews on this repo. Configure here.