Skip to content

Commit 5c7d5b2

Browse files
niemyjskiniemyjski
committed
PR Feedback
1 parent 4682aea commit 5c7d5b2

File tree

8 files changed

+81
-139
lines changed

8 files changed

+81
-139
lines changed

‎src/Platforms/Exceptionless.AspNetCore/ExceptionlessExceptionHandler.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@
66
using Microsoft.AspNetCore.Http;
77

88
namespace Exceptionless.AspNetCore {
9-
internal sealed class ExceptionlessExceptionHandler : IExceptionHandler {
9+
public sealed class ExceptionlessExceptionHandler : IExceptionHandler {
1010
private readonly ExceptionlessClient _client;
1111

1212
public ExceptionlessExceptionHandler(ExceptionlessClient client) {
1313
_client = client ?? ExceptionlessClient.Default;
1414
}
1515

1616
public ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception exception, CancellationToken cancellationToken) {
17-
if (httpContext.RequestAborted.IsCancellationRequested)
17+
if (cancellationToken.IsCancellationRequested)
1818
return ValueTask.FromResult(false);
1919

2020
var contextData = new ContextData();

‎src/Platforms/Exceptionless.AspNetCore/ExceptionlessExtensions.cs‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,26 @@
88
using Exceptionless.Models.Data;
99
using Exceptionless.Plugins.Default;
1010
using Microsoft.AspNetCore.Diagnostics;
11-
using Microsoft.Extensions.Configuration;
1211
using Microsoft.Extensions.DependencyInjection;
1312
using Microsoft.Extensions.DependencyInjection.Extensions;
1413
using Microsoft.Extensions.Hosting;
1514

1615
namespace Exceptionless {
1716
public static class ExceptionlessExtensions {
1817
/// <summary>
19-
/// Registers the Exceptionless <see cref="IExceptionHandler"/> for capturing unhandled exceptions
20-
/// in apps that use <c>UseExceptionHandler()</c>.
18+
/// Registers the Exceptionless <see cref="IExceptionHandler"/> for capturing unhandled exceptions.
19+
/// Call this in your service configuration alongside <c>app.UseExceptionHandler()</c>.
2120
/// </summary>
22-
public static IServiceCollection AddExceptionlessExceptionHandler(this IServiceCollection services) {
21+
public static IServiceCollection AddExceptionlessAspNetCore(this IServiceCollection services) {
22+
services.AddHttpContextAccessor();
2323
services.TryAddEnumerable(ServiceDescriptor.Singleton<IExceptionHandler, ExceptionlessExceptionHandler>());
2424
return services;
2525
}
2626

2727
/// <summary>
28-
/// Adds the Exceptionless middleware for capturing unhandled exceptions and ensures that the Exceptionless pending queue is processed before the host shuts down.
28+
/// Adds the Exceptionless middleware for 404 tracking and queue processing,
29+
/// subscribes to diagnostic events, and configures ASP.NET Core plugins.
2930
/// </summary>
30-
/// <param name="app">The target <see cref="IApplicationBuilder"/> to add Exceptionless to.</param>
31-
/// <param name="client">Optional pre-configured <see cref="ExceptionlessClient"/> instance to use. If not specified (recommended), the <see cref="ExceptionlessClient"/>
32-
/// instance registered in the services collection will be used.</param>
33-
/// <returns></returns>
3431
public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app, ExceptionlessClient client = null) {
3532
if (client == null)
3633
client = app.ApplicationServices.GetService<ExceptionlessClient>() ?? ExceptionlessClient.Default;
@@ -53,27 +50,6 @@ public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app,
5350
return app.UseMiddleware<ExceptionlessMiddleware>(client);
5451
}
5552

56-
[Obsolete("UseExceptionless should be called without an overload, ExceptionlessClient should be configured when adding to services collection using AddExceptionless")]
57-
public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app, Action<ExceptionlessConfiguration> configure) {
58-
var client = app.ApplicationServices.GetService<ExceptionlessClient>() ?? ExceptionlessClient.Default;
59-
configure?.Invoke(client.Configuration);
60-
return app.UseExceptionless(client);
61-
}
62-
63-
[Obsolete("UseExceptionless should be called without an overload, ExceptionlessClient should be configured when adding to services collection using AddExceptionless")]
64-
public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app, IConfiguration configuration) {
65-
var client = app.ApplicationServices.GetService<ExceptionlessClient>() ?? ExceptionlessClient.Default;
66-
client.Configuration.ReadFromConfiguration(configuration);
67-
return app.UseExceptionless(client);
68-
}
69-
70-
[Obsolete("UseExceptionless should be called without an overload, ExceptionlessClient should be configured when adding to services collection using AddExceptionless")]
71-
public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app, string apiKey) {
72-
var client = app.ApplicationServices.GetService<ExceptionlessClient>() ?? ExceptionlessClient.Default;
73-
client.Configuration.ApiKey = apiKey;
74-
return app.UseExceptionless(client);
75-
}
76-
7753
/// <summary>
7854
/// Adds the current request info.
7955
/// </summary>

‎src/Platforms/Exceptionless.AspNetCore/ExceptionlessMiddleware.cs‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
using System;
21
using System.Threading.Tasks;
32
using Microsoft.AspNetCore.Http;
4-
using Exceptionless.Plugins;
53

64
namespace Exceptionless.AspNetCore {
75
public class ExceptionlessMiddleware {
@@ -20,19 +18,7 @@ public async Task Invoke(HttpContext context) {
2018
});
2119
}
2220

23-
try {
24-
await _next(context);
25-
} catch (Exception ex) {
26-
if (context.RequestAborted.IsCancellationRequested)
27-
throw;
28-
29-
var contextData = new ContextData();
30-
contextData.MarkAsUnhandledError();
31-
contextData.SetSubmissionMethod(nameof(ExceptionlessMiddleware));
32-
33-
ex.ToExceptionless(contextData, _client).SetHttpContext(context).Submit();
34-
throw;
35-
}
21+
await _next(context);
3622

3723
if (context.Response?.StatusCode == 404) {
3824
string path = context.Request.Path.HasValue ? context.Request.Path.Value : "/";

‎src/Platforms/Exceptionless.Extensions.Hosting/ExceptionlessLifetimeService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public Task StartedAsync(CancellationToken cancellationToken) {
2929
}
3030

3131
public Task StoppingAsync(CancellationToken cancellationToken) {
32-
if (_started == 0)
32+
if (Volatile.Read(ref _started) == 0)
3333
return Task.CompletedTask;
3434

3535
return _exceptionlessClient.ProcessQueueAsync();

‎src/Platforms/Exceptionless.Extensions.Logging/ExceptionlessLoggerExtensions.cs‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
using System;
22
using Exceptionless.Extensions.Logging;
33
using ExceptionlessLogLevel = Exceptionless.Logging.LogLevel;
4-
using Microsoft.Extensions.Logging;
54
using Microsoft.Extensions.DependencyInjection;
5+
using Microsoft.Extensions.Logging;
66

77
namespace Exceptionless {
88
public static class ExceptionlessLoggerExtensions {
@@ -83,52 +83,5 @@ public static ILoggingBuilder AddExceptionless(this ILoggingBuilder builder, Act
8383
return builder;
8484
}
8585

86-
/// <summary>
87-
/// Adds Exceptionless to the logging pipeline using the <see cref="ExceptionlessClient.Default"/>.
88-
/// </summary>
89-
/// <param name="factory">The <see cref="ILoggerFactory"/>.</param>
90-
/// <param name="client">If a client is not specified then the <see cref="ExceptionlessClient.Default"/> will be used.</param>
91-
/// <returns>The <see cref="ILoggerFactory"/>.</returns>
92-
[Obsolete("Use ExceptionlessLoggerExtensions.AddExceptionless(ILoggingBuilder, ExceptionlessClient) instead.")]
93-
public static ILoggerFactory AddExceptionless(this ILoggerFactory factory, ExceptionlessClient client = null) {
94-
factory.AddProvider(new ExceptionlessLoggerProvider(client ?? ExceptionlessClient.Default));
95-
return factory;
96-
}
97-
98-
/// <summary>
99-
/// Adds Exceptionless to the logging pipeline using a new client with the provided api key.
100-
/// </summary>
101-
/// <param name="factory">The <see cref="ILoggerFactory"/>.</param>
102-
/// <param name="apiKey">The project api key.</param>
103-
/// <param name="serverUrl">The Server Url</param>
104-
/// <returns>The <see cref="ILoggerFactory"/>.</returns>
105-
[Obsolete("Use ExceptionlessLoggerExtensions.AddExceptionless(ILoggingBuilder, string, string) instead.")]
106-
public static ILoggerFactory AddExceptionless(this ILoggerFactory factory, string apiKey, string serverUrl = null) {
107-
if (String.IsNullOrEmpty(apiKey) && String.IsNullOrEmpty(serverUrl))
108-
return factory.AddExceptionless();
109-
110-
factory.AddProvider(new ExceptionlessLoggerProvider(config => {
111-
if (!String.IsNullOrEmpty(apiKey) && apiKey != "API_KEY_HERE")
112-
config.ApiKey = apiKey;
113-
if (!String.IsNullOrEmpty(serverUrl))
114-
config.ServerUrl = serverUrl;
115-
116-
config.UseInMemoryStorage();
117-
}));
118-
119-
return factory;
120-
}
121-
122-
/// <summary>
123-
/// Adds Exceptionless to the logging pipeline using a new client configured with the provided action.
124-
/// </summary>
125-
/// <param name="factory">The <see cref="ILoggerFactory"/>.</param>
126-
/// <param name="configure">An <see cref="Action{ExceptionlessConfiguration}"/> that applies additional settings and plugins. The project api key must be specified.</param>
127-
/// <returns>The <see cref="ILoggerFactory"/>.</returns>
128-
[Obsolete("Use ExceptionlessLoggerExtensions.AddExceptionless(ILoggingBuilder, Action<ExceptionlessConfiguration>) instead.")]
129-
public static ILoggerFactory AddExceptionless(this ILoggerFactory factory, Action<ExceptionlessConfiguration> configure) {
130-
factory.AddProvider(new ExceptionlessLoggerProvider(configure));
131-
return factory;
132-
}
13386
}
13487
}

‎test/Exceptionless.Tests/Platforms/AspNetCoreExceptionCaptureTests.cs‎

Lines changed: 59 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,84 +2,105 @@
22
using System;
33
using System.Collections.Generic;
44
using System.IO;
5+
using System.Threading;
56
using System.Threading.Tasks;
67
using Exceptionless.AspNetCore;
78
using Exceptionless.Models;
89
using Exceptionless.Models.Data;
9-
using Microsoft.AspNetCore.Diagnostics;
1010
using Microsoft.AspNetCore.Http;
1111
using Xunit;
1212

1313
namespace Exceptionless.Tests.Platforms {
1414
public class AspNetCoreExceptionCaptureTests {
1515
[Fact]
16-
public async Task Invoke_CapturesHandledExceptionsFromExceptionHandlerFeature() {
16+
public async Task TryHandleAsync_WhenRequestIsActive_ReturnsFalseAndCapturesUnhandledException() {
17+
// Arrange
1718
var submittingEvents = new List<EventSubmittingEventArgs>();
1819
var client = CreateClient(submittingEvents);
1920
var context = CreateHttpContext();
20-
var exception = new InvalidOperationException("handled");
21-
var middleware = new ExceptionlessMiddleware(currentContext => {
22-
currentContext.Features.Set<IExceptionHandlerFeature>(new ExceptionHandlerFeature {
23-
Error = exception
24-
});
25-
26-
return Task.CompletedTask;
27-
}, client);
21+
var exception = new InvalidOperationException("unhandled");
22+
var handler = new ExceptionlessExceptionHandler(client);
2823

29-
await middleware.Invoke(context);
24+
// Act
25+
var result = await handler.TryHandleAsync(context, exception, CancellationToken.None);
3026

27+
// Assert
28+
Assert.False(result);
3129
var submission = Assert.Single(submittingEvents);
32-
Assert.False(submission.IsUnhandledError);
33-
Assert.Equal(nameof(IExceptionHandlerFeature), submission.Event.Data[Event.KnownDataKeys.SubmissionMethod]);
34-
35-
var requestInfo = Assert.IsType<RequestInfo>(submission.Event.Data[Event.KnownDataKeys.RequestInfo]);
36-
Assert.Null(requestInfo.PostData);
30+
Assert.True(submission.IsUnhandledError);
31+
Assert.Equal(nameof(ExceptionlessExceptionHandler), submission.Event.Data[Event.KnownDataKeys.SubmissionMethod]);
3732
}
3833

3934
[Fact]
40-
public async Task Invoke_DoesNotDuplicateHandledExceptionsCapturedByDiagnostics() {
35+
public async Task TryHandleAsync_WhenCancellationIsRequested_ReturnsFalseWithoutCapturingException() {
36+
// Arrange
4137
var submittingEvents = new List<EventSubmittingEventArgs>();
4238
var client = CreateClient(submittingEvents);
39+
var cts = new CancellationTokenSource();
4340
var context = CreateHttpContext();
44-
var exception = new InvalidOperationException("handled");
45-
var listener = new ExceptionlessDiagnosticListener(client);
46-
var middleware = new ExceptionlessMiddleware(currentContext => {
47-
currentContext.Features.Set<IExceptionHandlerFeature>(new ExceptionHandlerFeature {
48-
Error = exception
49-
});
41+
cts.Cancel();
42+
var handler = new ExceptionlessExceptionHandler(client);
5043

51-
return Task.CompletedTask;
52-
}, client);
53-
54-
listener.OnNext(new KeyValuePair<string, object>("Microsoft.AspNetCore.Diagnostics.HandledException", new {
55-
httpContext = context,
56-
exception
57-
}));
44+
// Act
45+
var result = await handler.TryHandleAsync(context, new InvalidOperationException(), cts.Token);
5846

59-
await middleware.Invoke(context);
60-
61-
Assert.Single(submittingEvents);
47+
// Assert
48+
Assert.False(result);
49+
Assert.Empty(submittingEvents);
6250
}
6351

6452
[Fact]
65-
public async Task Invoke_DoesNotDuplicateUnhandledExceptionsCapturedByMiddleware() {
53+
public void OnNext_WhenUnhandledExceptionEventIsPublished_CapturesUnhandledException() {
54+
// Arrange
6655
var submittingEvents = new List<EventSubmittingEventArgs>();
6756
var client = CreateClient(submittingEvents);
6857
var context = CreateHttpContext();
6958
var exception = new InvalidOperationException("unhandled");
7059
var listener = new ExceptionlessDiagnosticListener(client);
71-
var middleware = new ExceptionlessMiddleware(_ => throw exception, client);
72-
73-
await Assert.ThrowsAsync<InvalidOperationException>(() => middleware.Invoke(context));
7460

61+
// Act
7562
listener.OnNext(new KeyValuePair<string, object>("Microsoft.AspNetCore.Hosting.UnhandledException", new {
7663
httpContext = context,
7764
exception
7865
}));
7966

67+
// Assert
8068
var submission = Assert.Single(submittingEvents);
8169
Assert.True(submission.IsUnhandledError);
82-
Assert.Equal(nameof(ExceptionlessMiddleware), submission.Event.Data[Event.KnownDataKeys.SubmissionMethod]);
70+
}
71+
72+
[Fact]
73+
public async Task Invoke_WhenResponseStatusIsNotFound_SubmitsNotFoundEvent() {
74+
// Arrange
75+
var submittingEvents = new List<EventSubmittingEventArgs>();
76+
var client = CreateClient(submittingEvents);
77+
var context = CreateHttpContext();
78+
var middleware = new ExceptionlessMiddleware(currentContext => {
79+
currentContext.Response.StatusCode = 404;
80+
return Task.CompletedTask;
81+
}, client);
82+
83+
// Act
84+
await middleware.Invoke(context);
85+
86+
// Assert
87+
var submission = Assert.Single(submittingEvents);
88+
Assert.Equal(Event.KnownTypes.NotFound, submission.Event.Type);
89+
}
90+
91+
[Fact]
92+
public async Task Invoke_WhenNextDelegateThrows_RethrowsExceptionWithoutSubmittingEvent() {
93+
// Arrange
94+
var submittingEvents = new List<EventSubmittingEventArgs>();
95+
var client = CreateClient(submittingEvents);
96+
var context = CreateHttpContext();
97+
var middleware = new ExceptionlessMiddleware(_ => throw new InvalidOperationException("boom"), client);
98+
99+
// Act
100+
await Assert.ThrowsAsync<InvalidOperationException>(() => middleware.Invoke(context));
101+
102+
// Assert
103+
Assert.Empty(submittingEvents);
83104
}
84105

85106
private static ExceptionlessClient CreateClient(ICollection<EventSubmittingEventArgs> submittingEvents) {

‎test/Exceptionless.Tests/Platforms/AspNetCoreRequestInfoTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
namespace Exceptionless.Tests.Platforms {
1212
public class AspNetCoreRequestInfoTests {
1313
[Fact]
14-
public void GetRequestInfo_DoesNotReadPostData_ForHandledErrors() {
14+
public void GetRequestInfo_WhenErrorIsHandled_DoesNotReadPostData() {
1515
// Arrange
1616
var context = CreateHttpContext("hello=world");
1717
var config = new ExceptionlessConfiguration(DependencyResolver.CreateDefault());
@@ -26,7 +26,7 @@ public void GetRequestInfo_DoesNotReadPostData_ForHandledErrors() {
2626
}
2727

2828
[Fact]
29-
public void GetRequestInfo_ReadsAndRestoresPostData_ForUnhandledErrors() {
29+
public void GetRequestInfo_WhenErrorIsUnhandled_ReadsAndRestoresPostData() {
3030
// Arrange
3131
const string body = "{\"hello\":\"world\"}";
3232
var context = CreateHttpContext(body);
@@ -44,7 +44,7 @@ public void GetRequestInfo_ReadsAndRestoresPostData_ForUnhandledErrors() {
4444
}
4545

4646
[Fact]
47-
public void GetRequestInfo_ReadsFormData_ForUnhandledErrors() {
47+
public void GetRequestInfo_WhenUnhandledRequestContainsFormData_ReadsFormData() {
4848
// Arrange
4949
var context = CreateFormHttpContext();
5050
var config = new ExceptionlessConfiguration(DependencyResolver.CreateDefault());

‎test/Exceptionless.Tests/Platforms/HostingExtensionsTests.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,30 @@
88
namespace Exceptionless.Tests.Platforms {
99
public class HostingExtensionsTests {
1010
[Fact]
11-
public void AddExceptionless_RegistersClientAndLifetimeService_OnHostApplicationBuilder() {
11+
public void AddExceptionless_WhenCalled_RegistersClientAndLifetimeService() {
12+
// Arrange
1213
var builder = Host.CreateApplicationBuilder();
1314

15+
// Act
1416
builder.AddExceptionless(configuration => configuration.ApiKey = "test-api-key");
1517

18+
// Assert
1619
Assert.Contains(builder.Services, descriptor => descriptor.ServiceType == typeof(ExceptionlessClient));
1720
Assert.Contains(builder.Services, descriptor =>
1821
descriptor.ServiceType == typeof(IHostedService) &&
1922
descriptor.ImplementationType == typeof(ExceptionlessLifetimeService));
2023
}
2124

2225
[Fact]
23-
public void UseExceptionless_DoesNotRegisterDuplicateLifetimeServices_OnHostApplicationBuilder() {
26+
public void UseExceptionless_WhenCalledTwice_DoesNotRegisterDuplicateLifetimeServices() {
27+
// Arrange
2428
var builder = Host.CreateApplicationBuilder();
2529

30+
// Act
2631
builder.UseExceptionless();
2732
builder.UseExceptionless();
2833

34+
// Assert
2935
Assert.Single(builder.Services, descriptor =>
3036
descriptor.ServiceType == typeof(IHostedService) &&
3137
descriptor.ImplementationType == typeof(ExceptionlessLifetimeService));

0 commit comments

Comments
 (0)