Skip to content

Commit d7b7b5d

Browse files
Merge pull request #101 from bartoszclapinski/sprint2/phase2.C.3-error-handling-#93
feat(error-handling): complete phase 2.c.3 cleanup (#93)
2 parents 53c4c0c + b6e6c4f commit d7b7b5d

30 files changed

+930
-481
lines changed

.ai/sprints/sprint2/cleanup-plan.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,14 @@ After completing Sprint 2 (GitHub Integration & Background Jobs), this cleanup p
6767
**Goal**: Improve error handling with user-friendly messages
6868

6969
#### Tasks:
70-
- [ ] Create custom exception types (NotFoundException, ValidationException, etc.)
71-
- [ ] Add global exception handler middleware
72-
- [ ] Return user-friendly error messages from API endpoints
73-
- [ ] Add validation to DTOs (FluentValidation)
74-
- [ ] Handle GitHub API rate limit errors gracefully
75-
- [ ] Add retry logic for transient failures (Polly)
76-
- [ ] Display error messages in Blazor components
77-
- [ ] Add error logging to Application Insights/Serilog
70+
- [x] Create custom exception types (NotFoundException, ValidationException, etc.)
71+
- [x] Add global exception handler middleware
72+
- [x] Return user-friendly error messages from API endpoints
73+
- [x] Add validation to DTOs (FluentValidation)
74+
- [x] Handle GitHub API rate limit errors gracefully
75+
- [x] Add retry logic for transient failures (Polly)
76+
- [x] Display error messages in Blazor components
77+
- [x] Add error logging to Application Insights/Serilog
7878

7979
**Time Estimate**: 1 day
8080

.ai/sprints/sprint2/sprint-log.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,6 +1276,67 @@ Integrate with GitHub to fetch and sync developer metrics with background proces
12761276

12771277
---
12781278

1279+
### Day 12 - November 16, 2025
1280+
**Focus**:
1281+
- Phase 2.C.3 task “Display error messages in Blazor components” (Issue #93)
1282+
1283+
**Highlights**:
1284+
- Created reusable `HttpResponseMessageExtensions.ReadProblemDetailsMessageAsync()` in Web layer to hydrate RFC 7807 payloads (reused across components and forms).
1285+
- Updated `Home.razor`, `Repositories.razor`, and `PullRequests.razor` to call the helper and surface API failures via `ISnackbar` and existing alert panels, instead of silent failures or generic strings.
1286+
- Refined `Login.razor` and `Register.razor` to show validation/business error details coming from the new global exception pipeline.
1287+
- Added `_Imports.razor` import so every component can access the helper without extra directives.
1288+
1289+
**Testing**:
1290+
- `dotnet build` (root solution) — succeeds with 0 warnings/errors.
1291+
1292+
**Notes / Next Steps**:
1293+
- Remaining 2.C.3 items are resiliency related (Polly retry + GitHub rate-limit handling) and structured logging.
1294+
- Once resiliency work lands we can close issue #93 and move to Phase 2.C.4 (Caching).
1295+
1296+
---
1297+
1298+
### Day 13 - November 17, 2025
1299+
**Focus**:
1300+
- Phase 2.C.3 tasks “Handle GitHub API rate limit errors gracefully” + “Add retry logic for transient failures (Polly)” (Issue #93)
1301+
1302+
**Highlights**:
1303+
- Added `Polly` dependency plus internal `GitHubResiliencePolicies` helper that gives every Octokit call a 3-attempt exponential backoff with jitter for `ApiException`, `HttpRequestException`, and `TaskCanceledException`.
1304+
- Centralized rate-limit messaging through `GitHubExceptionHelper`, mapping Octokit’s `RateLimitExceededException` to our `ExternalServiceException` with a human-readable retry ETA.
1305+
- Updated `GitHubRepositoryService`, `GitHubCommitsService`, and `GitHubPullRequestService` to:
1306+
- Execute through the shared retry policy (with cancellation support),
1307+
- Throw domain exceptions (`NotFoundException`, `UnauthorizedAccessException`, `ExternalServiceException`) so the global handler can respond consistently,
1308+
- Log transient retries vs terminal failures separately.
1309+
- Confirmed background job + controllers automatically benefit (they already bubble exceptions), so UI now gets friendly “rate limit” snackbars without special casing.
1310+
1311+
**Testing**:
1312+
- `dotnet build`
1313+
- `dotnet test tests/DevMetricsPro.Infrastructure.Tests/DevMetricsPro.Infrastructure.Tests.csproj`
1314+
1315+
**Notes / Next Steps**:
1316+
- Final open Phase 2.C.3 item is structured logging/Application Insights hook-up; once done we can close Issue #93 and move ahead to caching (2.C.4).
1317+
1318+
---
1319+
1320+
### Day 14 - November 17, 2025 (later)
1321+
**Focus**:
1322+
- Phase 2.C.3 task “Add error logging to Application Insights/Serilog” (Issue #93)
1323+
1324+
**Highlights**:
1325+
- Added `Microsoft.ApplicationInsights.AspNetCore` + `Serilog.Sinks.ApplicationInsights`, surfaced configuration knobs in `appsettings*.json`, and wired telemetry registration to only activate when a connection string is provided (so devs aren’t forced to provision AI locally).
1326+
- Updated `Program.cs` to:
1327+
- Bootstrap Serilog early, then rebuild the pipeline via `UseSerilog` with DI/Configuration.
1328+
- Forward console + rolling file logs, and conditionally fan out to Application Insights using `TelemetryConverter.Traces`.
1329+
- Enable `UseSerilogRequestLogging()` for per-request diagnostics.
1330+
- Confirmed solution builds cleanly with new telemetry references.
1331+
1332+
**Testing**:
1333+
- `dotnet build`
1334+
1335+
**Notes / Next Steps**:
1336+
- Phase 2.C.3 is now fully green (Issue #93 ready to close once branch is reviewed/merged). Next phase in the cleanup plan: 2.C.4 Caching Layer.
1337+
1338+
---
1339+
12791340
### Sprint 2 Completion Summary
12801341

12811342
---

src/DevMetricsPro.Application/DevMetricsPro.Application.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
<ProjectReference Include="..\DevMetricsPro.Core\DevMetricsPro.Core.csproj" />
55
</ItemGroup>
66

7+
<ItemGroup>
8+
<PackageReference Include="FluentValidation" Version="12.1.0" />
9+
</ItemGroup>
10+
711
<PropertyGroup>
812
<TargetFramework>net9.0</TargetFramework>
913
<ImplicitUsings>enable</ImplicitUsings>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using DevMetricsPro.Application.DTOs.GitHub;
2+
using FluentValidation;
3+
4+
namespace DevMetricsPro.Application.Validators;
5+
6+
public class GitHubCallbackRequestValidator : AbstractValidator<GitHubCallbackRequest>
7+
{
8+
public GitHubCallbackRequestValidator()
9+
{
10+
RuleFor(x => x.Code).NotEmpty();
11+
RuleFor(x => x.State).NotEmpty();
12+
}
13+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using DevMetricsPro.Application.DTOs.Auth;
2+
using FluentValidation;
3+
4+
namespace DevMetricsPro.Application.Validators;
5+
6+
public class LoginRequestValidator : AbstractValidator<LoginRequest>
7+
{
8+
public LoginRequestValidator()
9+
{
10+
RuleFor(x => x.Email)
11+
.NotEmpty().WithMessage("Email is required")
12+
.EmailAddress().WithMessage("Invalid email format");
13+
14+
RuleFor(x => x.Password)
15+
.NotEmpty().WithMessage("Password is required");
16+
}
17+
}
18+
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using DevMetricsPro.Application.DTOs.Auth;
2+
using FluentValidation;
3+
4+
namespace DevMetricsPro.Application.Validators;
5+
6+
public class RegisterRequestValidator : AbstractValidator<RegisterRequest>
7+
{
8+
public RegisterRequestValidator()
9+
{
10+
RuleFor(x => x.Email)
11+
.NotEmpty().WithMessage("Email is required")
12+
.EmailAddress().WithMessage("Invalid email format");
13+
14+
RuleFor(x => x.Password)
15+
.NotEmpty().WithMessage("Password is required")
16+
.MinimumLength(8).WithMessage("Password must be at least 8 characters long");
17+
18+
RuleFor(x => x.ConfirmPassword)
19+
.NotEmpty().WithMessage("Password confirmation is required")
20+
.Equal(x => x.Password).WithMessage("Passwords do not match");
21+
22+
RuleFor(x => x.DisplayName)
23+
.MaximumLength(100).WithMessage("Display name cannot exceed 100 characters")
24+
.When(x => !string.IsNullOrWhiteSpace(x.DisplayName));
25+
}
26+
}
27+
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace DevMetricsPro.Core.Exceptions;
2+
3+
public class BusinessRuleException : Exception
4+
{
5+
public BusinessRuleException(string message) : base(message) {}
6+
7+
public BusinessRuleException(string message, Exception? innerException) : base(message, innerException) {}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace DevMetricsPro.Core.Exceptions;
2+
3+
public class ExternalServiceException : Exception
4+
{
5+
public ExternalServiceException(string message) : base(message) {}
6+
7+
public ExternalServiceException(string message, Exception? innerException) : base(message, innerException) {}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace DevMetricsPro.Core.Exceptions;
2+
3+
public class NotFoundException : Exception
4+
{
5+
public NotFoundException(string message) : base(message) {}
6+
7+
public NotFoundException(string message, Exception? innerException) : base(message, innerException) {}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace DevMetricsPro.Core.Exceptions;
2+
3+
public class ValidationException : Exception
4+
{
5+
public ValidationException(string message) : base(message) {}
6+
7+
public ValidationException(string message, Exception? innerException) : base(message, innerException) {}
8+
}

0 commit comments

Comments
 (0)