Add NRTs to Sentry.AspNetCore

src/Sentry.AspNetCore/AspNetCoreEventProcessor.cs

@@ -12,7 +12,7 @@ internal class AspNetCoreEventProcessor : ISentryEventProcessor
         private readonly SentryAspNetCoreOptions _options;
 
         public AspNetCoreEventProcessor(IOptions<SentryAspNetCoreOptions> options)
-            => _options = options?.Value;
+            => _options = options.Value;
 
         public SentryEvent Process(SentryEvent @event)
         {

src/Sentry.AspNetCore/DefaultUserFactory.cs

@@ -7,7 +7,7 @@ namespace Sentry.AspNetCore
 {
     internal class DefaultUserFactory : IUserFactory
     {
-        public User Create(HttpContext context)
+        public User? Create(HttpContext context)
         {
             Debug.Assert(context != null);
 
@@ -17,9 +17,9 @@ public User Create(HttpContext context)
                 return null;
             }
 
-            string email = null;
-            string id = null;
-            string username = null;
+            string? email = null;
+            string? id = null;
+            string? username = null;
             foreach (var claim in principal.Claims)
             {
                 switch (claim.Type)

src/Sentry.AspNetCore/HttpRequestAdapter.cs

@@ -12,12 +12,12 @@ internal class HttpRequestAdapter : IHttpRequest
 
         public HttpRequestAdapter(HttpRequest request) => _request = request;
 
-        public long? ContentLength => _request?.ContentLength;
-        public string ContentType => _request?.ContentType;
-        public Stream Body => _request?.Body;
+        public long? ContentLength => _request.ContentLength;
+        public string? ContentType => _request.ContentType;
+        public Stream? Body => _request.Body;
 
         public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Form =>
             _request?.Form.Select(k => new KeyValuePair<string, IEnumerable<string>>(k.Key, k.Value))
             ?? Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
     }
-}
+}

src/Sentry.AspNetCore/IUserFactory.cs

@@ -13,6 +13,6 @@ public interface IUserFactory
         /// </summary>
         /// <param name="context">The HttpContext where the user resides</param>
         /// <returns>The protocol user</returns>
-        User Create(HttpContext context);
+        User? Create(HttpContext context);
     }
 }

src/Sentry.AspNetCore/ScopeExtensions.cs

@@ -26,19 +26,29 @@ public static class ScopeExtensions
         /// </remarks>
         public static void Populate(this Scope scope, HttpContext context, SentryAspNetCoreOptions options)
         {
+            // Not to throw on code that ignores nullability warnings.
+            // ReSharper disable ConditionIsAlwaysTrueOrFalse
+            if (scope is null || context is null || options is null)
+            {
+                return;
+            }
+            // ReSharper restore ConditionIsAlwaysTrueOrFalse
+
             // With the logger integration, a BeginScope call is made with RequestId. That ends up adding
             // two tags with the same value: RequestId and TraceIdentifier
             if (!scope.Tags.TryGetValue("RequestId", out var requestId) || requestId != context.TraceIdentifier)
             {
                 scope.SetTag(nameof(context.TraceIdentifier), context.TraceIdentifier);
             }
 
-            if (options?.SendDefaultPii == true && !scope.HasUser())
+            if (options.SendDefaultPii && !scope.HasUser())
             {
                 var userFactory = context.RequestServices?.GetService<IUserFactory>();
-                if (userFactory != null)
+                var user = userFactory?.Create(context);
+
+                if (user != null)
                 {
-                    scope.User = userFactory.Create(context);
+                    scope.User = user;
                 }
             }
 
@@ -48,8 +58,9 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
             }
             catch (Exception e)
             {
-                options?.DiagnosticLogger?.LogError("Failed to extract body.", e);
+                options.DiagnosticLogger?.LogError("Failed to extract body.", e);
             }
+
             SetEnv(scope, context, options);
 
             // Extract the route data
@@ -82,7 +93,7 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
             {
                 // Suppress the error here; we expect an ArgumentNullException if httpContext.Request.RouteValues is null from GetRouteData()
                 // TODO: Consider adding a bool to the Sentry options to make route data extraction optional in case they don't use a routing middleware?
-                options?.DiagnosticLogger?.LogDebug("Failed to extract route data.", e);
+                options.DiagnosticLogger?.LogDebug("Failed to extract route data.", e);
             }
 
             // TODO: Get context stuff into scope
@@ -108,7 +119,7 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt
             scope.Request.QueryString = context.Request.QueryString.ToString();
             foreach (var requestHeader in context.Request.Headers)
             {
-                if (options?.SendDefaultPii != true
+                if (!options.SendDefaultPii
                 // Don't add headers which might contain PII
                 && (requestHeader.Key == HeaderNames.Cookie
                     || requestHeader.Key == HeaderNames.Authorization))
@@ -121,7 +132,7 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt
 
             // TODO: Hide these 'Env' behind some extension method as
             // these might be reported in a non CGI, old-school way
-            if (options?.SendDefaultPii == true
+            if (options.SendDefaultPii
                 && context.Connection.RemoteIpAddress?.ToString() is { } ipAddress)
             {
                 scope.Request.Env["REMOTE_ADDR"] = ipAddress;
@@ -138,11 +149,6 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt
 
         private static void SetBody(BaseScope scope, HttpContext context, SentryAspNetCoreOptions options)
         {
-            if (context == null || scope == null || options == null)
-            {
-                return;
-            }
-
             var extractors = context.RequestServices.GetService<IEnumerable<IRequestPayloadExtractor>>();
             if (extractors == null)
             {
@@ -164,10 +170,13 @@ private static void SetBody(BaseScope scope, HttpContext context, SentryAspNetCo
         /// <param name="activity">The activity.</param>
         public static void Populate(this Scope scope, Activity activity)
         {
-            if (scope == null || activity == null)
+            // Not to throw on code that ignores nullability warnings.
+            // ReSharper disable ConditionIsAlwaysTrueOrFalse
+            if (scope is null || activity is null)
             {
                 return;
             }
+            // ReSharper restore ConditionIsAlwaysTrueOrFalse
 
             //scope.ActivityId = activity.Id;
 
@@ -177,10 +186,7 @@ public static void Populate(this Scope scope, Activity activity)
 
         internal static void SetWebRoot(this Scope scope, string webRoot)
         {
-            if (webRoot != null)
-            {
-                scope.Request.Env["DOCUMENT_ROOT"] = webRoot;
-            }
+            scope.Request.Env["DOCUMENT_ROOT"] = webRoot;
         }
     }
 }

src/Sentry.AspNetCore/Sentry.AspNetCore.csproj

@@ -7,6 +7,12 @@
     <AssemblyName>Sentry.AspNetCore</AssemblyName>
     <RootNamespace>Sentry.AspNetCore</RootNamespace>
     <Description>Official ASP.NET Core integration for Sentry - Open-source error tracking that helps developers monitor and fix crashes in real time.</Description>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <!-- Disable nullability warnings on older frameworks because there is no nullability info for BCL -->
+  <PropertyGroup Condition="'$(TargetFramework)' != 'netcoreapp3.0'">
+    <Nullable>annotations</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

src/Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs

@@ -25,10 +25,10 @@ public override void Configure(SentryAspNetCoreOptions options)
         {
             base.Configure(options);
 
-            options.Environment
-                = options.Environment // Don't override user defined value
-                  ?? EnvironmentLocator.Locate() // Sentry specific environment takes precedence #92
-                  ?? _hostingEnvironment?.EnvironmentName;
+            // Don't override user defined value
+            options.Environment ??=
+                EnvironmentLocator.Locate() // Sentry specific environment takes precedence #92
+                ?? _hostingEnvironment.EnvironmentName;
 
             options.AddLogEntryFilter((category, level, eventId, exception)
                 // https://github.com/aspnet/KestrelHttpServer/blob/0aff4a0440c2f393c0b98e9046a8e66e30a56cb0/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs#L33

src/Sentry.AspNetCore/SentryMiddleware.cs

@@ -55,7 +55,7 @@ public SentryMiddleware(
         {
             _next = next ?? throw new ArgumentNullException(nameof(next));
             _hubAccessor = hubAccessor ?? throw new ArgumentNullException(nameof(hubAccessor));
-            _options = options?.Value;
+            _options = options.Value;
             if (_options != null)
             {
                 var hub = _hubAccessor();
@@ -84,20 +84,17 @@ public async Task InvokeAsync(HttpContext context)
 
             using (hub.PushAndLockScope())
             {
-                if (_options != null)
+                if (_options.MaxRequestBodySize != RequestSize.None)
                 {
-                    if (_options.MaxRequestBodySize != RequestSize.None)
-                    {
-                        context.Request.EnableBuffering();
-                    }
-                    if (_options.FlushOnCompletedRequest)
+                    context.Request.EnableBuffering();
+                }
+                if (_options.FlushOnCompletedRequest)
+                {
+                    context.Response.OnCompleted(async () =>
                     {
-                        context.Response.OnCompleted(async () =>
-                        {
-                            // Serverless environments flush the queue at the end of each request
-                            await hub.FlushAsync(timeout: _options.FlushTimeout).ConfigureAwait(false);
-                        });
-                    }
+                        // Serverless environments flush the queue at the end of each request
+                        await hub.FlushAsync(timeout: _options.FlushTimeout).ConfigureAwait(false);
+                    });
                 }
 
                 hub.ConfigureScope(scope =>
@@ -133,29 +130,36 @@ void CaptureException(Exception e)
                 {
                     var evt = new SentryEvent(e);
 
-                    _logger?.LogTrace("Sending event '{SentryEvent}' to Sentry.", evt);
+                    _logger.LogTrace("Sending event '{SentryEvent}' to Sentry.", evt);
 
                     var id = hub.CaptureEvent(evt);
 
-                    _logger?.LogInformation("Event '{id}' queued.", id);
+                    _logger.LogInformation("Event '{id}' queued.", id);
                 }
             }
         }
 
         internal void PopulateScope(HttpContext context, Scope scope)
         {
-            scope.Sdk.Name = Constants.SdkName;
-            scope.Sdk.Version = NameAndVersion.Version;
-            scope.Sdk.AddPackage(ProtocolPackageName, NameAndVersion.Version);
+            if (scope.Sdk is { })
+            {
+                scope.Sdk.Name = Constants.SdkName;
+                scope.Sdk.Version = NameAndVersion.Version;
+
+                if (NameAndVersion.Version is { } version)
+                {
+                    scope.Sdk.AddPackage(ProtocolPackageName, version);
+                }
+            }
 
-            if (_hostingEnvironment != null)
+            if (_hostingEnvironment.WebRootPath is { } webRootPath)
             {
-                scope.SetWebRoot(_hostingEnvironment.WebRootPath);
+                scope.SetWebRoot(webRootPath);
             }
 
             scope.Populate(context, _options);
 
-            if (_options?.IncludeActivityData == true)
+            if (_options.IncludeActivityData)
             {
                 scope.Populate(Activity.Current);
             }

src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs

@@ -21,7 +21,7 @@ public static class SentryWebHostBuilderExtensions
         /// <param name="builder">The builder.</param>
         /// <returns></returns>
         public static IWebHostBuilder UseSentry(this IWebHostBuilder builder)
-            => UseSentry(builder, (Action<SentryAspNetCoreOptions>)null);
+            => UseSentry(builder, (Action<SentryAspNetCoreOptions>?)null);
 
         /// <summary>
         /// Uses Sentry integration.
@@ -40,7 +40,7 @@ public static IWebHostBuilder UseSentry(this IWebHostBuilder builder, string dsn
         /// <returns></returns>
         public static IWebHostBuilder UseSentry(
             this IWebHostBuilder builder,
-            Action<SentryAspNetCoreOptions> configureOptions)
+            Action<SentryAspNetCoreOptions>? configureOptions)
             => builder.UseSentry((context, options) => configureOptions?.Invoke(options));
 
         /// <summary>
@@ -51,7 +51,7 @@ public static IWebHostBuilder UseSentry(
         /// <returns></returns>
         public static IWebHostBuilder UseSentry(
             this IWebHostBuilder builder,
-            Action<WebHostBuilderContext, SentryAspNetCoreOptions> configureOptions)
+            Action<WebHostBuilderContext, SentryAspNetCoreOptions>? configureOptions)
         {
             // The earliest we can hook the SDK initialization code with the framework
             // Initialization happens at a later time depending if the default MEL backend is enabled or not.
@@ -81,10 +81,7 @@ public static IWebHostBuilder UseSentry(
                 _ = logging.Services.AddSentry();
             });
 
-            _ = builder.ConfigureServices(c =>
-            {
-                _ = c.AddTransient<IStartupFilter, SentryStartupFilter>();
-            });
+            _ = builder.ConfigureServices(c => _ = c.AddTransient<IStartupFilter, SentryStartupFilter>());
 
             return builder;
         }

test/Sentry.AspNetCore.Tests/MiddlewareLoggerIntegration.cs

@@ -31,7 +31,7 @@ private class Fixture : IDisposable
             public ISystemClock Clock { get; set; } = Substitute.For<ISystemClock>();
             public SentryAspNetCoreOptions Options { get; set; } = new SentryAspNetCoreOptions();
             public IHostingEnvironment HostingEnvironment { get; set; } = Substitute.For<IHostingEnvironment>();
-            public ILogger<SentryMiddleware> MiddlewareLogger { get; set; }
+            public ILogger<SentryMiddleware> MiddlewareLogger { get; set; } = Substitute.For<ILogger<SentryMiddleware>>();
             public ILogger SentryLogger { get; set; }
             public HttpContext HttpContext { get; set; } = Substitute.For<HttpContext>();
             public IFeatureCollection FeatureCollection { get; set; } = Substitute.For<IFeatureCollection>();